Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd)

[Kirill Smelkov <kirr@nexedi.com>]

On Thu, Apr 18, 2019 at 07:31:02AM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 17 Apr 2019, Bjorn Helgaas wrote:
> 
> > On Sat, Apr 13, 2019 at 06:50:57PM +0200, Julia Lawall wrote:
> > > Hello,
> > >
> > > Kirill will explain about this issue.
> > >
> > > julia
> > >
> > > ---------- Forwarded message ----------
> > > Date: Sat, 13 Apr 2019 11:22:51 +0800
> > > From: kbuild test robot <lkp@intel.com>
> > > To: kbuild@01.org
> > > Cc: Julia Lawall <julia.lawall@lip6.fr>
> > > Subject: [PATCH] pci/switchtec: fix stream_open.cocci warnings
> > >
> > > CC: kbuild-all@01.org
> > > TO: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > CC: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
> > > CC: Logan Gunthorpe <logang@deltatee.com>
> > > CC: Bjorn Helgaas <helgaas@kernel.org>
> > > CC: linux-pci@vger.kernel.org
> > > CC: linux-kernel@vger.kernel.org
> > >
> > > From: kbuild test robot <lkp@intel.com>
> > >
> > > drivers/pci/switch/switchtec.c:395:1-17: ERROR: switchtec_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
> > >
> > > Generated by: scripts/coccinelle/api/stream_open.cocci
> > >
> > > Fixes: 8a29a3bae2a2 ("pci/switchtec: Don't use completion's wait queue")
> > > Signed-off-by: kbuild test robot <lkp@intel.com>
> >
> > Based on Kirill's subsequent email saying this is already queued to
> > the merge window, I assume I need to do nothing here.
> >
> > I think a signed-off-by from a robot, i.e., not from a real person, is
> > meaningless, and I don't think I would personally accept it.  It's
> > certainly OK to indicate that a patch was auto-generated, but I think
> > a real person still needs to take responsibility for it.
> >
> > Documentation/process/submitting-patches.rst says it must contain a
> > real name (no pseudonyms or anonymous contributions), and I don't
> > think a robot fits in the spirit of that.
> >
> > I see that
> > https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git/commit/?id=8a29a3bae2a2
> > (mentioned below) does have a good signed-off-by from Sebastian, but
> > that's not *this* patch, so I don't know what's what.
> 
> Normally, for these robot generated patches, when I approve them, I put my
> own sign off, but under the robot one, since the robot has put a From
> line.  In this case, I handed the problem off to Kirill, so I didn't do
> that.  I agree that it would be good for Kirill to sign off on it.

Just for the reference: I have put my signature on the mass converstion
patch as well as ack's that were received:

https://lab.nexedi.com/kirr/linux/commit/edaeb4101860

I plan to refresh that patch and post it to mainline when merge-window opens.

Appologize if I missed something. I'm not a kernel developer and don't
have much know-how in this land.

Kirill


> > > ---
> > >
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git linux-5.0.y-rt-rebase
> > > head:   794c294ae4483c240429c25a0d18e272e92c94de
> > > commit: 8a29a3bae2a2dfb0116cd8791d9700515d6e765e [154/311] pci/switchtec: Don't use completion's wait queue
> > > :::::: branch date: 7 hours ago
> > > :::::: commit date: 7 hours ago
> > >
> > > Please take the patch only if it's a positive warning. Thanks!
> > >
> > >  switchtec.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > --- a/drivers/pci/switch/switchtec.c
> > > +++ b/drivers/pci/switch/switchtec.c
> > > @@ -392,7 +392,7 @@ static int switchtec_dev_open(struct ino
> > >  		return PTR_ERR(stuser);
> > >
> > >  	filp->private_data = stuser;
> > > -	nonseekable_open(inode, filp);
> > > +	stream_open(inode, filp);
> > >
> > >  	dev_dbg(&stdev->dev, "%s: %p\n", __func__, stuser);