All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Parav Pandit <parav@mellanox.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kwankhede@nvidia.com" <kwankhede@nvidia.com>,
	"cjia@nvidia.com" <cjia@nvidia.com>
Subject: Re: [PATCHv1 7/7] vfio/mdev: Fix race conditions with mdev device life cycle APIs
Date: Fri, 26 Apr 2019 15:58:48 -0600	[thread overview]
Message-ID: <20190426155848.5173e896@x1.home> (raw)
In-Reply-To: <AM4PR0501MB22602E3F3480FAC5DC297D4DD13E0@AM4PR0501MB2260.eurprd05.prod.outlook.com>

On Fri, 26 Apr 2019 19:02:40 +0000
Parav Pandit <parav@mellanox.com> wrote:

> Hi Alex,
> 
> 
> > -----Original Message-----
> > From: Alex Williamson <alex.williamson@redhat.com>
> > Sent: Friday, April 26, 2019 11:09 AM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > kwankhede@nvidia.com; cjia@nvidia.com
> > 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?

It depends how soon they stabilize and how invasive they are.  These
are bug fixes, so we can consider them for after the merge window
(which will close before the end of May), but the longer they take to
stabilize and the more significant the change, the more likely I'd be
to wait for 5.3.  Thanks,

Alex

      reply	other threads:[~2019-04-26 21:58 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-27  3:45 [PATCHv1 0/7] vfio/mdev: Improve vfio/mdev core module Parav Pandit
2019-03-27  3:45 ` [PATCHv1 1/7] vfio/mdev: Avoid release parent reference during error path Parav Pandit
2019-04-01 16:58   ` Cornelia Huck
2019-03-27  3:45 ` [PATCHv1 2/7] vfio/mdev: Removed unused kref Parav Pandit
2019-04-01 17:01   ` Cornelia Huck
2019-03-27  3:45 ` [PATCHv1 3/7] vfio/mdev: Drop redundant extern for exported symbols Parav Pandit
2019-04-01 17:02   ` Cornelia Huck
2019-03-27  3:45 ` [PATCHv1 4/7] vfio/mdev: Avoid masking error code to EBUSY Parav Pandit
2019-04-01 17:21   ` Cornelia Huck
2019-03-27  3:45 ` [PATCHv1 5/7] vfio/mdev: Follow correct remove sequence Parav Pandit
2019-04-01 17:24   ` Cornelia Huck
2019-03-27  3:45 ` [PATCHv1 6/7] vfio/mdev: Fix aborting mdev child device removal if one fails Parav Pandit
2019-04-01 17:39   ` Cornelia Huck
2019-04-02 19:59     ` Parav Pandit
2019-04-02 22:33       ` Alex Williamson
2019-04-03  6:34         ` Parav Pandit
2019-03-27  3:45 ` [PATCHv1 7/7] vfio/mdev: Fix race conditions with mdev device life cycle APIs Parav Pandit
2019-04-03 21:27   ` Alex Williamson
2019-04-04  0:02     ` Parav Pandit
2019-04-04 20:44       ` Alex Williamson
2019-04-04 23:05         ` Parav Pandit
2019-04-23 19:21           ` Alex Williamson
2019-04-25 23:29             ` Parav Pandit
2019-04-26 15:33               ` Alex Williamson
2019-04-26 15:55                 ` Parav Pandit
2019-04-26 16:09                   ` Alex Williamson
2019-04-26 19:02                     ` Parav Pandit
2019-04-26 21:58                       ` Alex Williamson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190426155848.5173e896@x1.home \
    --to=alex.williamson@redhat.com \
    --cc=cjia@nvidia.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=parav@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.