RE: [PATCHv1 7/7] vfio/mdev: Fix race conditions with mdev device life cycle APIs

[Parav Pandit]

Hi Alex,


> -----Original Message-----
> From: Alex Williamson <alex.williamson@xxxxxxxxxx>
> Sent: Friday, April 26, 2019 11:09 AM
> To: Parav Pandit <parav@xxxxxxxxxxxx>
> Cc: kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> kwankhede@xxxxxxxxxx; cjia@xxxxxxxxxx
> Subject: Re: [PATCHv1 7/7] vfio/mdev: Fix race conditions with mdev device
> life cycle APIs

[..]

> > > >
> > > > > Patch 6 looks ok, except I'd rather see the sanitizing loop stay
> > > > > until we can otherwise fix the race above.
> > > > I can put back the sanitizing look, once it looks valid. Will wait
> > > > for your response.
> > >
> > > Yep, I think patch 6 is good w/o the removal of the sanitizing loop.
> > > Will you repost it?
> > >
> > Just the patch-6 or 1 to 6?
> 
> Your choice, please roll in reviews/acks if you repost the rest.
> 
> > > > > Patch 7 needed more work, iirc.  Thanks,
> > > > For a moment if we assume sanitizing loop exists, than patch-7
> > > > looks good?
> > >
> > > Patch 7 is a bit less trivial, so I think as we're close to the
> > > merge window for v5.2, I'd rather push it out to be included with
> > > the later re-works.  Thanks,
> > >
> > I agree it little less trivial, I tried to place as much comment as possible,
> but it is an important fix.
> > Let me repost 6-7 and decide if it can be included or not?
> 
> Sounds good.  Thanks,
> 
I am dropping patch-7 for today and reworking the patch-6 for now.

Even after keeping that that crazy loop, I am easily able to create this below [1] call trace on adding file when mdev_remove() fails with the thread sequence we discussed above.

I think this is high time, we fix the sequence to match the linux bus sequence.
I have some cycles this week.
Post these 6 patches,
I like to get total 3 patches done.
1. fix the bus sequence
2. race with parent device removal
3. do not try to add sysfs file on remove() failure

Is there any possibility above 3 patches can make to 5.2, given that merge window closes in June?
If yes, I will get them in 2-3 days. I will test with sample drivers and mlx5 driver.
Can we get some tests also done from Kirti also done on their hw?

kernel: WARNING: CPU: 2 PID: 9348 at fs/sysfs/file.c:327 sysfs_create_file_ns+0x7f/0x90
kernel: CPU: 2 PID: 9348 Comm: bash Kdump: loaded Not tainted 5.1.0-rc6-vdevbus+ #6
kernel: Hardware name: Supermicro SYS-6028U-TR4+/X10DRU-i+, BIOS 2.0b 08/09/2016
kernel: RIP: 0010:sysfs_create_file_ns+0x7f/0x90
kernel: Call Trace:
kernel: remove_store+0xdc/0x100 [mdev]
kernel: kernfs_fop_write+0x113/0x1a0
kernel: vfs_write+0xad/0x1b0
kernel: ksys_write+0x5a/0xe0
kernel: do_syscall_64+0x5a/0x210
kernel: entry_SYSCALL_64_after_hwframe+0x49/0xbe