Re: [PATCH 1/3] PCI: ensure the PCI device is locked over ->reset_notify calls

From	Bjorn Helgaas <helgaas@kernel.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 1/3] PCI: ensure the PCI device is locked over ->reset_notify calls
Date	2017-06-13 16:10 +0200
Message-ID	<tRUG6-w3-11@gated-at.bofh.it> (permalink)
References	(3 earlier) <tPkdH-7xX-5@gated-at.bofh.it> <tPu3p-5KI-37@gated-at.bofh.it> <tPNSq-1Kk-9@gated-at.bofh.it> <tRGMO-cb-5@gated-at.bofh.it> <tRO7E-4US-19@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Tue, Jun 13, 2017 at 09:08:10AM +0200, Christoph Hellwig wrote:
> On Mon, Jun 12, 2017 at 06:14:23PM -0500, Bjorn Helgaas wrote:
> > My main concern is being able to verify the locking.  I think that is
> > much easier if the locking is adjacent to the method invocation.  But
> > if you just add a comment at the method invocation about where the
> > locking is, that should be sufficient.
> 
> Ok.  I can add comments for all the methods as a separate patch,
> similar to Documentation/vfs/Locking
> 
> > > Yes, I mentioned this earlier, and I also vaguely remember we got
> > > bug reports from IBM on power for this a while ago.  I just don't
> > > feel confident enough to touch all these without a good test plan.
> > 
> > Hmmm.  I see your point, but I hate leaving a known bug unfixed.  I
> > wonder if some enterprising soul could tickle this bug by injecting
> > errors while removing and rescanning devices below the bridge?
> 
> I'm completely loaded up at the moment, but this sounds like a good
> idea.
> 
> In the meantime how do you want to proceed with this patch?

Can you just add comments about the locking?  I'd prefer that in the
same patch that adds the locking because that's what I had a hard time
reviewing.  I'm not thinking of anything fancy like
Documentation/filesystems/Locking; I'm just thinking of something
along the lines of "caller must hold pci_dev_lock() to protect
err_handler->reset_notify from racing ->remove()".  And the changelog
should contain the general principle about the locking strategy.

Bjorn

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH 1/3] PCI: ensure the PCI device is locked over         ->reset_notify calls Christoph Hellwig <hch@lst.de> - 2017-06-07 20:30 +0200
  Re: [PATCH 1/3] PCI: ensure the PCI device is locked over  ->reset_notify calls Bjorn Helgaas <helgaas@kernel.org> - 2017-06-13 01:20 +0200
    Re: [PATCH 1/3] PCI: ensure the PCI device is locked over         ->reset_notify calls Christoph Hellwig <hch@lst.de> - 2017-06-13 09:10 +0200
      Re: [PATCH 1/3] PCI: ensure the PCI device is locked over  ->reset_notify calls Bjorn Helgaas <helgaas@kernel.org> - 2017-06-13 16:10 +0200

csiph-web