* [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) @ 2019-04-13 16:50 Julia Lawall 2019-04-13 16:56 ` Logan Gunthorpe ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Julia Lawall @ 2019-04-13 16:50 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: kbuild-all, Kurt Schwemmer, Logan Gunthorpe, Bjorn Helgaas, linux-pci, linux-kernel, Kirill Smelkov 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> --- 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); ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-13 16:50 [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) Julia Lawall @ 2019-04-13 16:56 ` Logan Gunthorpe 2019-04-13 17:01 ` Kirill Smelkov 2019-04-17 21:54 ` Bjorn Helgaas 2 siblings, 0 replies; 18+ messages in thread From: Logan Gunthorpe @ 2019-04-13 16:56 UTC (permalink / raw) To: Julia Lawall, Sebastian Andrzej Siewior Cc: kbuild-all, Kurt Schwemmer, Bjorn Helgaas, linux-pci, linux-kernel, Kirill Smelkov On 2019-04-13 10:50 a.m., Julia Lawall wrote: > Hello, > > Kirill will explain about this issue. I'm aware of this effort and Acked Kirill's original monolithic patch. So, for this patch: Acked-by: Logan Gunthorpe <logang@deltatee.com. Thanks, Logan > 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> > --- > > 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); > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-13 16:50 [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) Julia Lawall 2019-04-13 16:56 ` Logan Gunthorpe @ 2019-04-13 17:01 ` Kirill Smelkov 2019-04-15 14:38 ` Sebastian Andrzej Siewior 2019-04-17 21:54 ` Bjorn Helgaas 2 siblings, 1 reply; 18+ messages in thread From: Kirill Smelkov @ 2019-04-13 17:01 UTC (permalink / raw) To: Julia Lawall Cc: Sebastian Andrzej Siewior, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, Bjorn Helgaas, linux-pci, linux-kernel Hello everyone, On Sat, Apr 13, 2019 at 06:50:57PM +0200, Julia Lawall wrote: > Hello, > > Kirill will explain about this issue. pci/switchtec switching to stream_open is already queued to merge window and it was acked by Logan Gunthorpe: https://lore.kernel.org/lkml/CAHk-=wgqgN5j1ZWnyVLqqoyU=CCWTYOko3MDyU8L_5e21KvHAg@mail.gmail.com/ https://lab.nexedi.com/kirr/linux/commit/edaeb4101860 ( there are too many Cc's in that patch and email with it and reply-all to it did not get into mailing list probably due to being considered as spam ) stream_open.cocci was issuing only warning for pci/switchtec, but after 8a29a3bae2a2 ("pci/switchtec: Don't use completion's wait queue") they started to use wait_even_* inside read method and, since stream_open.cocci considers wait_event_* as blocking the warning became error. Previously it was completions there, but I added support for wait events only for simplicity. I can handle pci/switchtec switching via big nonseekable_open -> stream_open change at next merge window, or, alternatively please feel free to switch pci/switchtec now on its own. The change is correct - I was manually reviewing all changes that stream_open.cocci produces and pci/switchtec was there. Kirill > 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> > --- > > 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); ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-13 17:01 ` Kirill Smelkov @ 2019-04-15 14:38 ` Sebastian Andrzej Siewior 2019-04-15 14:55 ` Kirill Smelkov 0 siblings, 1 reply; 18+ messages in thread From: Sebastian Andrzej Siewior @ 2019-04-15 14:38 UTC (permalink / raw) To: Kirill Smelkov Cc: Julia Lawall, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, Bjorn Helgaas, linux-pci, linux-kernel On 2019-04-13 17:00:59 [+0000], Kirill Smelkov wrote: > Hello everyone, Hi, > On Sat, Apr 13, 2019 at 06:50:57PM +0200, Julia Lawall wrote: > > Hello, > > > > Kirill will explain about this issue. > > pci/switchtec switching to stream_open is already queued to merge > window and it was acked by Logan Gunthorpe: > > https://lore.kernel.org/lkml/CAHk-=wgqgN5j1ZWnyVLqqoyU=CCWTYOko3MDyU8L_5e21KvHAg@mail.gmail.com/ > https://lab.nexedi.com/kirr/linux/commit/edaeb4101860 > > ( there are too many Cc's in that patch and email with it and reply-all to > it did not get into mailing list probably due to being considered as spam ) > > stream_open.cocci was issuing only warning for pci/switchtec, but after > 8a29a3bae2a2 ("pci/switchtec: Don't use completion's wait queue") they > started to use wait_even_* inside read method and, since > stream_open.cocci considers wait_event_* as blocking the warning became > error. Previously it was completions there, but I added support for wait > events only for simplicity. why is wait_event_interruptible() treated differently compared to wait_for_completion_interruptible()? > Kirill Sebastian ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-15 14:38 ` Sebastian Andrzej Siewior @ 2019-04-15 14:55 ` Kirill Smelkov 2019-04-15 15:20 ` Sebastian Andrzej Siewior 0 siblings, 1 reply; 18+ messages in thread From: Kirill Smelkov @ 2019-04-15 14:55 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Julia Lawall, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, Bjorn Helgaas, linux-pci, linux-kernel Hi Sebastian, On Mon, Apr 15, 2019 at 04:38:57PM +0200, Sebastian Andrzej Siewior wrote: > On 2019-04-13 17:00:59 [+0000], Kirill Smelkov wrote: > > Hello everyone, > Hi, > > > On Sat, Apr 13, 2019 at 06:50:57PM +0200, Julia Lawall wrote: > > > Hello, > > > > > > Kirill will explain about this issue. > > > > pci/switchtec switching to stream_open is already queued to merge > > window and it was acked by Logan Gunthorpe: > > > > https://lore.kernel.org/lkml/CAHk-=wgqgN5j1ZWnyVLqqoyU=CCWTYOko3MDyU8L_5e21KvHAg@mail.gmail.com/ > > https://lab.nexedi.com/kirr/linux/commit/edaeb4101860 > > > > ( there are too many Cc's in that patch and email with it and reply-all to > > it did not get into mailing list probably due to being considered as spam ) > > > > stream_open.cocci was issuing only warning for pci/switchtec, but after > > 8a29a3bae2a2 ("pci/switchtec: Don't use completion's wait queue") they > > started to use wait_even_* inside read method and, since > > stream_open.cocci considers wait_event_* as blocking the warning became > > error. Previously it was completions there, but I added support for wait > > events only for simplicity. > > why is wait_event_interruptible() treated differently compared to > wait_for_completion_interruptible()? No particular reason. I just taught stream_open.cocci to consider only "wait_event_*" as blocking: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/api/stream_open.cocci?h=v5.1-rc5#n35 based on original /proc/xen/xenbus deadlock: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/xen/xenbus/xenbus_dev_frontend.c?h=v5.1-rc5#n135 https://git.kernel.org/linus/581d21a2d02a We can extend "a function that blocks" rule to cover other kernel primitives. For the reference: the deadlock scenario is described in https://git.kernel.org/linus/10dce8af3422 Kirill ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-15 14:55 ` Kirill Smelkov @ 2019-04-15 15:20 ` Sebastian Andrzej Siewior 2019-04-15 15:41 ` Kirill Smelkov 0 siblings, 1 reply; 18+ messages in thread From: Sebastian Andrzej Siewior @ 2019-04-15 15:20 UTC (permalink / raw) To: Kirill Smelkov Cc: Julia Lawall, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, Bjorn Helgaas, linux-pci, linux-kernel On 2019-04-15 14:55:02 [+0000], Kirill Smelkov wrote: > Hi Sebastian, Hi Kirill, > On Mon, Apr 15, 2019 at 04:38:57PM +0200, Sebastian Andrzej Siewior wrote: > > On 2019-04-13 17:00:59 [+0000], Kirill Smelkov wrote: > > > stream_open.cocci was issuing only warning for pci/switchtec, but after > > > 8a29a3bae2a2 ("pci/switchtec: Don't use completion's wait queue") they > > > started to use wait_even_* inside read method and, since > > > stream_open.cocci considers wait_event_* as blocking the warning became > > > error. Previously it was completions there, but I added support for wait > > > events only for simplicity. > > > > why is wait_event_interruptible() treated differently compared to > > wait_for_completion_interruptible()? > > No particular reason. I just taught stream_open.cocci to consider > only "wait_event_*" as blocking: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/api/stream_open.cocci?h=v5.1-rc5#n35 > > based on original /proc/xen/xenbus deadlock: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/xen/xenbus/xenbus_dev_frontend.c?h=v5.1-rc5#n135 > https://git.kernel.org/linus/581d21a2d02a > > We can extend "a function that blocks" rule to cover other kernel > primitives. > > For the reference: the deadlock scenario is described in > > https://git.kernel.org/linus/10dce8af3422 As far I understand the problem is when the ->read() callback waits for the ->write() callback. The locking isn't changed by patch you mentioned. So extended might make sense. But then wait_event_* by itself in ->read() isn't a problem as long as its counter part isn't in ->write(). But yes, nice finding. > Kirill Sebastian ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-15 15:20 ` Sebastian Andrzej Siewior @ 2019-04-15 15:41 ` Kirill Smelkov 0 siblings, 0 replies; 18+ messages in thread From: Kirill Smelkov @ 2019-04-15 15:41 UTC (permalink / raw) To: Sebastian Andrzej Siewior Cc: Julia Lawall, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, Bjorn Helgaas, linux-pci, linux-kernel Hi Sebastian, On Mon, Apr 15, 2019 at 05:20:22PM +0200, Sebastian Andrzej Siewior wrote: > On 2019-04-15 14:55:02 [+0000], Kirill Smelkov wrote: > > Hi Sebastian, > Hi Kirill, > > > On Mon, Apr 15, 2019 at 04:38:57PM +0200, Sebastian Andrzej Siewior wrote: > > > On 2019-04-13 17:00:59 [+0000], Kirill Smelkov wrote: > > > > stream_open.cocci was issuing only warning for pci/switchtec, but after > > > > 8a29a3bae2a2 ("pci/switchtec: Don't use completion's wait queue") they > > > > started to use wait_even_* inside read method and, since > > > > stream_open.cocci considers wait_event_* as blocking the warning became > > > > error. Previously it was completions there, but I added support for wait > > > > events only for simplicity. > > > > > > why is wait_event_interruptible() treated differently compared to > > > wait_for_completion_interruptible()? > > > > No particular reason. I just taught stream_open.cocci to consider > > only "wait_event_*" as blocking: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/api/stream_open.cocci?h=v5.1-rc5#n35 > > > > based on original /proc/xen/xenbus deadlock: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/xen/xenbus/xenbus_dev_frontend.c?h=v5.1-rc5#n135 > > https://git.kernel.org/linus/581d21a2d02a > > > > We can extend "a function that blocks" rule to cover other kernel > > primitives. > > > > For the reference: the deadlock scenario is described in > > > > https://git.kernel.org/linus/10dce8af3422 > > As far I understand the problem is when the ->read() callback waits for > the ->write() callback. The locking isn't changed by patch you > mentioned. Yes, correct. The patch that I mentioned only adds semantic patch which find places with such problem and can generate a regular patch to change locking. Here is that place for pci/switchtec: https://lab.nexedi.com/kirr/linux/commit/edaeb4101860?expand_all_diffs=1#ccc4baef911c8dad164d4ff29a8c0b287abed7c2_393_393 > So extended might make sense. But then wait_event_* by itself in > ->read() isn't a problem as long as its counter part isn't in ->write(). It is a problem either if its counterpart is in write _or_ if that wait_event depends on external source and waiting can be for potentially unbounded time, like e.g. waiting to receive a character from serial port or network. But you are right that even with wait_event used, cases are possible that there is no blocking that depend on external source and it could be just e.g. spawn kernel thread to do some limited amount of work and wait for it to complete. I did not taught stream_open.cocci about that because when something goes wrong with semantic patch and Coccinelle complains, it is hard to understand what is going on, and because generally it is better to convert files that do not depend on position, even if there is no deadlock at all, to stream_open - i.e. don't do any f_pos_lock locking at all. > But yes, nice finding. Thanks, Kirill ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-13 16:50 [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) Julia Lawall 2019-04-13 16:56 ` Logan Gunthorpe 2019-04-13 17:01 ` Kirill Smelkov @ 2019-04-17 21:54 ` Bjorn Helgaas 2019-04-18 5:31 ` Julia Lawall 2 siblings, 1 reply; 18+ messages in thread From: Bjorn Helgaas @ 2019-04-17 21:54 UTC (permalink / raw) To: Julia Lawall Cc: Sebastian Andrzej Siewior, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, linux-pci, linux-kernel, Kirill Smelkov 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. Bjorn > --- > > 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); > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-17 21:54 ` Bjorn Helgaas @ 2019-04-18 5:31 ` Julia Lawall 2019-04-18 10:38 ` Kirill Smelkov 0 siblings, 1 reply; 18+ messages in thread From: Julia Lawall @ 2019-04-18 5:31 UTC (permalink / raw) To: Bjorn Helgaas Cc: Sebastian Andrzej Siewior, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, linux-pci, linux-kernel, Kirill Smelkov 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. julia > > Bjorn > > > --- > > > > 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); > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-18 5:31 ` Julia Lawall @ 2019-04-18 10:38 ` Kirill Smelkov 2019-04-18 12:37 ` Bjorn Helgaas 0 siblings, 1 reply; 18+ messages in thread From: Kirill Smelkov @ 2019-04-18 10:38 UTC (permalink / raw) To: Julia Lawall Cc: Bjorn Helgaas, Sebastian Andrzej Siewior, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, linux-pci, linux-kernel 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); ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-18 10:38 ` Kirill Smelkov @ 2019-04-18 12:37 ` Bjorn Helgaas 2019-04-18 14:42 ` Kirill Smelkov 0 siblings, 1 reply; 18+ messages in thread From: Bjorn Helgaas @ 2019-04-18 12:37 UTC (permalink / raw) To: Kirill Smelkov Cc: Julia Lawall, Sebastian Andrzej Siewior, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, linux-pci, linux-kernel On Thu, Apr 18, 2019 at 10:38:02AM +0000, Kirill Smelkov wrote: > 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 Looks good, thanks. Feel free to add my Acked-by: Bjorn Helgaas <bhelgaas@google.com> [drivers/pci/switch/switchtec] to the https://lab.nexedi.com/kirr/linux/commit/edaeb4101860 patch. It looks like maybe the commit log could use s/and the reset were/and the rest were/ It also mentions "the previous patch" a couple times, which may lose some of its meaning depending on how things get merged into git. If that previous patch has already been merged, a SHA1 reference would be more specific. I would personally split that into two patches: one to avoid the potential deadlocks and a second to do the "safe to change to stream_open" changes. It seems like the first is more serious and urgent while the second is more of a cleanup. Then you could streamline the commit logs by including a single diagnostic and omitting the entire list of files. But that's all bike-shedding and I'm totally fine with this as-is. Bjorn ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-04-18 12:37 ` Bjorn Helgaas @ 2019-04-18 14:42 ` Kirill Smelkov 0 siblings, 0 replies; 18+ messages in thread From: Kirill Smelkov @ 2019-04-18 14:42 UTC (permalink / raw) To: Bjorn Helgaas Cc: Julia Lawall, Sebastian Andrzej Siewior, kbuild-all, Kurt Schwemmer, Logan Gunthorpe, linux-pci, linux-kernel On Thu, Apr 18, 2019 at 07:37:30AM -0500, Bjorn Helgaas wrote: > On Thu, Apr 18, 2019 at 10:38:02AM +0000, Kirill Smelkov wrote: > > 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 > > Looks good, thanks. Feel free to add my > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> [drivers/pci/switch/switchtec] > > to the https://lab.nexedi.com/kirr/linux/commit/edaeb4101860 patch. > > It looks like maybe the commit log could use > > s/and the reset were/and the rest were/ Bjorn, thanks for feedback. I've added your ack and fixed that typo. > It also mentions "the previous patch" a couple times, which may lose > some of its meaning depending on how things get merged into git. If > that previous patch has already been merged, a SHA1 reference would be > more specific. Good point. Initially those patches were coming together, but the first one landed to master while the conversion is only scheduled to be done. I've changed this reference to 10dce8af3422 ("fs: stream_open - opener for stream-like files so that read and write can run simultaneously without deadlock"). > I would personally split that into two patches: one to avoid the > potential deadlocks and a second to do the "safe to change to > stream_open" changes. It seems like the first is more serious and > urgent while the second is more of a cleanup. Then you could > streamline the commit logs by including a single diagnostic and > omitting the entire list of files. I was contemplating how to split this too. And one of the way was to go with separate patch for every subsystem. However I still hope to do the mass conversion all at one conversion, because otherwise it would be many patches and it will take my time to propagate them all / ping maintainers etc. About splitting deadlock / just safe: stream_open.cocci does not have complete coverage for detecting whether a .read() blocks, and as pci/switchtec case shows there are indeed other cases that might be deadlocking but are not currently detected as such. I would thus prefer not to split the conversion. I've added the following note to the patch: and the rest were just safe to convert to stream_open because their read and write do not use ppos at all and corresponding file_operations do not have methods that assume @offset file access(*): <long list> ... (*) This second group also contains cases with read/write deadlocks that stream_open.cocci don't yet detect, but which are still valid to convert to stream_open since ppos is not used. For example drivers/pci/switch/switchtec.c calls wait_for_completion_interruptible() in its .read, but stream_open.cocci currently detects only "wait_event*" as blocking. hope it is ok. > But that's all bike-shedding and I'm totally fine with this as-is. Thanks. Your input was useful. The updated patch is here: https://lab.nexedi.com/kirr/linux/commit/a34a8a9cb2d7 as well as related patches to tighten stream_open semantic and corresponding FUSE bits: https://lab.nexedi.com/kirr/linux/commit/47ee8df337a9 https://lab.nexedi.com/kirr/linux/commit/1b4636172835 Kirill ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <alpine.DEB.2.20.1906191227430.3726@hadrien>]
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) [not found] <alpine.DEB.2.20.1906191227430.3726@hadrien> @ 2019-06-19 16:27 ` Kirill Smelkov 2019-06-19 19:47 ` Logan Gunthorpe 2019-06-19 20:19 ` Bjorn Helgaas 0 siblings, 2 replies; 18+ messages in thread From: Kirill Smelkov @ 2019-06-19 16:27 UTC (permalink / raw) To: Julia Lawall, Sebastian Andrzej Siewior Cc: Kurt Schwemmer, Logan Gunthorpe, Bjorn Helgaas, linux-pci, linux-kernel, kbuild-all Hi Julia, everyone. On Wed, Jun 19, 2019 at 12:28:47PM +0200, Julia Lawall wrote: > Hi, > > Can you forward this patch to the people below if you think it is > appropriate? Yes, this patch is appropriate. It was actually part of git.kernel.org/linus/c5bf68fe0c86 . It should be safe, (and desirable as it removes a chance for deadlock) to apply it. Sebastian, Kurt, Logan, Bjorn, please consider picking it up. > Could I tell the kbuild people to add you to the CC list for > this semantic patch? Yes, sure. Please feel free to add me to CC list for stream_open.cocci related patches. Kirill > thanks, > julia > > ---------- Forwarded message ---------- > Date: Wed, 19 Jun 2019 18:23:18 +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: a3a1e895d4fa ("pci/switchtec: Don't use completion's wait queue") > Signed-off-by: kbuild test robot <lkp@intel.com> > --- > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git linux-5.0.y-rt-rebase > head: 31cc76d5590f5e60c2f26f029e40bc7d0441d93f > commit: a3a1e895d4fa0508e11ac9107ace883a5b2a4d3b [171/305] pci/switchtec: Don't use completion's wait queue > :::::: branch date: 6 days ago > :::::: commit date: 6 days 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); > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-06-19 16:27 ` Kirill Smelkov @ 2019-06-19 19:47 ` Logan Gunthorpe 2019-06-19 20:19 ` Bjorn Helgaas 1 sibling, 0 replies; 18+ messages in thread From: Logan Gunthorpe @ 2019-06-19 19:47 UTC (permalink / raw) To: Kirill Smelkov, Julia Lawall, Sebastian Andrzej Siewior Cc: Kurt Schwemmer, Bjorn Helgaas, linux-pci, linux-kernel, kbuild-all On 2019-06-19 10:27 a.m., Kirill Smelkov wrote: > Hi Julia, everyone. > > On Wed, Jun 19, 2019 at 12:28:47PM +0200, Julia Lawall wrote: >> Hi, >> >> Can you forward this patch to the people below if you think it is >> appropriate? >> 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: a3a1e895d4fa ("pci/switchtec: Don't use completion's wait queue") >> Signed-off-by: kbuild test robot <lkp@intel.com> >> --- >> >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git linux-5.0.y-rt-rebase This is for the RT tree? The patch in the fixes tag isn't in upstream and I don't understand how this is related to that patch at all. It just looks like the RT tree hasn't picked up the patch which made this change in upstream. I feel like I've seen the change in this patch a bunch of times already and it appears to be correct in rc5 at least... Logan >> head: 31cc76d5590f5e60c2f26f029e40bc7d0441d93f >> commit: a3a1e895d4fa0508e11ac9107ace883a5b2a4d3b [171/305] pci/switchtec: Don't use completion's wait queue >> :::::: branch date: 6 days ago >> :::::: commit date: 6 days 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); >> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-06-19 16:27 ` Kirill Smelkov 2019-06-19 19:47 ` Logan Gunthorpe @ 2019-06-19 20:19 ` Bjorn Helgaas 2019-06-19 20:21 ` Julia Lawall 2019-06-20 7:01 ` kirr 1 sibling, 2 replies; 18+ messages in thread From: Bjorn Helgaas @ 2019-06-19 20:19 UTC (permalink / raw) To: Kirill Smelkov Cc: Julia Lawall, Sebastian Andrzej Siewior, Kurt Schwemmer, Logan Gunthorpe, linux-pci, linux-kernel, kbuild-all On Wed, Jun 19, 2019 at 04:27:52PM +0000, Kirill Smelkov wrote: > Hi Julia, everyone. > > On Wed, Jun 19, 2019 at 12:28:47PM +0200, Julia Lawall wrote: > > Hi, > > > > Can you forward this patch to the people below if you think it is > > appropriate? > > Yes, this patch is appropriate. It was actually part of > git.kernel.org/linus/c5bf68fe0c86 . It should be safe, (and desirable as > it removes a chance for deadlock) to apply it. > > Sebastian, Kurt, Logan, Bjorn, please consider picking it up. I don't get it. This appeared in v5.2-rc1 as c5bf68fe0c86 ("*: convert stream-like files from nonseekable_open -> stream_open"), so it looks like this is already done. What would you like me to do with it? > > Could I tell the kbuild people to add you to the CC list for > > this semantic patch? > > Yes, sure. Please feel free to add me to CC list for stream_open.cocci > related patches. > > Kirill > > > > thanks, > > julia > > > > ---------- Forwarded message ---------- > > Date: Wed, 19 Jun 2019 18:23:18 +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: a3a1e895d4fa ("pci/switchtec: Don't use completion's wait queue") > > Signed-off-by: kbuild test robot <lkp@intel.com> > > --- > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git linux-5.0.y-rt-rebase > > head: 31cc76d5590f5e60c2f26f029e40bc7d0441d93f > > commit: a3a1e895d4fa0508e11ac9107ace883a5b2a4d3b [171/305] pci/switchtec: Don't use completion's wait queue > > :::::: branch date: 6 days ago > > :::::: commit date: 6 days 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); > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-06-19 20:19 ` Bjorn Helgaas @ 2019-06-19 20:21 ` Julia Lawall 2019-06-20 7:01 ` kirr 1 sibling, 0 replies; 18+ messages in thread From: Julia Lawall @ 2019-06-19 20:21 UTC (permalink / raw) To: Bjorn Helgaas Cc: Kirill Smelkov, Sebastian Andrzej Siewior, Kurt Schwemmer, Logan Gunthorpe, linux-pci, linux-kernel, kbuild-all On Wed, 19 Jun 2019, Bjorn Helgaas wrote: > On Wed, Jun 19, 2019 at 04:27:52PM +0000, Kirill Smelkov wrote: > > Hi Julia, everyone. > > > > On Wed, Jun 19, 2019 at 12:28:47PM +0200, Julia Lawall wrote: > > > Hi, > > > > > > Can you forward this patch to the people below if you think it is > > > appropriate? > > > > Yes, this patch is appropriate. It was actually part of > > git.kernel.org/linus/c5bf68fe0c86 . It should be safe, (and desirable as > > it removes a chance for deadlock) to apply it. > > > > Sebastian, Kurt, Logan, Bjorn, please consider picking it up. > > I don't get it. This appeared in v5.2-rc1 as c5bf68fe0c86 ("*: convert > stream-like files from nonseekable_open -> stream_open"), so it looks like > this is already done. What would you like me to do with it? Somehow 0-day got a hold of it... If there is nothing to do, just ignore it. thanks, julia > > > > Could I tell the kbuild people to add you to the CC list for > > > this semantic patch? > > > > Yes, sure. Please feel free to add me to CC list for stream_open.cocci > > related patches. > > > > Kirill > > > > > > > thanks, > > > julia > > > > > > ---------- Forwarded message ---------- > > > Date: Wed, 19 Jun 2019 18:23:18 +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: a3a1e895d4fa ("pci/switchtec: Don't use completion's wait queue") > > > Signed-off-by: kbuild test robot <lkp@intel.com> > > > --- > > > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git linux-5.0.y-rt-rebase > > > head: 31cc76d5590f5e60c2f26f029e40bc7d0441d93f > > > commit: a3a1e895d4fa0508e11ac9107ace883a5b2a4d3b [171/305] pci/switchtec: Don't use completion's wait queue > > > :::::: branch date: 6 days ago > > > :::::: commit date: 6 days 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); > > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-06-19 20:19 ` Bjorn Helgaas 2019-06-19 20:21 ` Julia Lawall @ 2019-06-20 7:01 ` kirr 2019-06-21 16:42 ` Sebastian Andrzej Siewior 1 sibling, 1 reply; 18+ messages in thread From: kirr @ 2019-06-20 7:01 UTC (permalink / raw) To: Bjorn Helgaas Cc: Julia Lawall, Sebastian Andrzej Siewior, Kurt Schwemmer, Logan Gunthorpe, linux-pci, linux-kernel, kbuild-all Bjorn Helgaas писал 2019-06-19 23:19: > On Wed, Jun 19, 2019 at 04:27:52PM +0000, Kirill Smelkov wrote: >> Hi Julia, everyone. >> >> On Wed, Jun 19, 2019 at 12:28:47PM +0200, Julia Lawall wrote: >> > Hi, >> > >> > Can you forward this patch to the people below if you think it is >> > appropriate? >> >> Yes, this patch is appropriate. It was actually part of >> git.kernel.org/linus/c5bf68fe0c86 . It should be safe, (and desirable >> as >> it removes a chance for deadlock) to apply it. >> >> Sebastian, Kurt, Logan, Bjorn, please consider picking it up. > > I don't get it. This appeared in v5.2-rc1 as c5bf68fe0c86 ("*: convert > stream-like files from nonseekable_open -> stream_open"), so it looks > like > this is already done. What would you like me to do with it? I meant just that it was ok to pick this change into 5.0-RT tree as kbuild robot was suggesting. Sorry for not being clear. Kirill >> > Could I tell the kbuild people to add you to the CC list for >> > this semantic patch? >> >> Yes, sure. Please feel free to add me to CC list for stream_open.cocci >> related patches. >> >> Kirill >> >> >> > thanks, >> > julia >> > >> > ---------- Forwarded message ---------- >> > Date: Wed, 19 Jun 2019 18:23:18 +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: a3a1e895d4fa ("pci/switchtec: Don't use completion's wait queue") >> > Signed-off-by: kbuild test robot <lkp@intel.com> >> > --- >> > >> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git linux-5.0.y-rt-rebase >> > head: 31cc76d5590f5e60c2f26f029e40bc7d0441d93f >> > commit: a3a1e895d4fa0508e11ac9107ace883a5b2a4d3b [171/305] pci/switchtec: Don't use completion's wait queue >> > :::::: branch date: 6 days ago >> > :::::: commit date: 6 days 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); >> > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) 2019-06-20 7:01 ` kirr @ 2019-06-21 16:42 ` Sebastian Andrzej Siewior 0 siblings, 0 replies; 18+ messages in thread From: Sebastian Andrzej Siewior @ 2019-06-21 16:42 UTC (permalink / raw) To: kirr Cc: Bjorn Helgaas, Julia Lawall, Kurt Schwemmer, Logan Gunthorpe, linux-pci, linux-kernel, kbuild-all On 2019-06-20 07:01:43 [+0000], kirr@nexedi.com wrote: > I meant just that it was ok to pick this change into 5.0-RT tree as > kbuild robot was suggesting. Sorry for not being clear. Okay, I was under the impression that this affect non-RT as well. But if this is RT-only then let me pick it up… > Kirill Sebastian ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-06-21 16:42 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-13 16:50 [PATCH] pci/switchtec: fix stream_open.cocci warnings (fwd) Julia Lawall 2019-04-13 16:56 ` Logan Gunthorpe 2019-04-13 17:01 ` Kirill Smelkov 2019-04-15 14:38 ` Sebastian Andrzej Siewior 2019-04-15 14:55 ` Kirill Smelkov 2019-04-15 15:20 ` Sebastian Andrzej Siewior 2019-04-15 15:41 ` Kirill Smelkov 2019-04-17 21:54 ` Bjorn Helgaas 2019-04-18 5:31 ` Julia Lawall 2019-04-18 10:38 ` Kirill Smelkov 2019-04-18 12:37 ` Bjorn Helgaas 2019-04-18 14:42 ` Kirill Smelkov [not found] <alpine.DEB.2.20.1906191227430.3726@hadrien> 2019-06-19 16:27 ` Kirill Smelkov 2019-06-19 19:47 ` Logan Gunthorpe 2019-06-19 20:19 ` Bjorn Helgaas 2019-06-19 20:21 ` Julia Lawall 2019-06-20 7:01 ` kirr 2019-06-21 16:42 ` Sebastian Andrzej Siewior
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).