cocci.inria.fr archive mirror
 help / color / mirror / Atom feed
* [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking
@ 2019-06-23  7:28 Kirill Smelkov
  2019-06-23  7:29 ` [Cocci] [PATCH 2/2] *: convert stream-like files -> stream_open, even if they use noop_llseek Kirill Smelkov
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Kirill Smelkov @ 2019-06-23  7:28 UTC (permalink / raw)
  To: cocci, linux-kernel
  Cc: Sebastian Andrzej Siewior, Bjorn Helgaas, Logan Gunthorpe,
	Kirill Smelkov

Previously steam_open.cocci was treating only wait_event_.* - e.g.
wait_event_interruptible - as a blocking operation. However e.g.
wait_for_completion_interruptible is also blocking, and so from this
point of view it would be more logical to treat all wait_.* as a
blocking point.

The logic of this change actually came up for real when
drivers/pci/switch/switchtec.c changed from using
wait_event_interruptible to wait_for_completion_interruptible:

	https://lore.kernel.org/linux-pci/20190413170056.GA11293@deco.navytux.spb.ru/
	https://lore.kernel.org/linux-pci/20190415145456.GA15280@deco.navytux.spb.ru/
	https://lore.kernel.org/linux-pci/20190415154102.GB17661@deco.navytux.spb.ru/

For a driver that uses nonseekable_open with read/write having stream
semantic and read also calling e.g. wait_for_completion_interruptible,
running stream_open.cocci before this patch would produce:

	WARNING: <driver>_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.

while after this patch it will report:

	ERROR: <driver>_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.

Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
---
 scripts/coccinelle/api/stream_open.cocci | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/coccinelle/api/stream_open.cocci b/scripts/coccinelle/api/stream_open.cocci
index 350145da7669..12ce18fa6b74 100644
--- a/scripts/coccinelle/api/stream_open.cocci
+++ b/scripts/coccinelle/api/stream_open.cocci
@@ -35,11 +35,11 @@ type loff_t;
 // a function that blocks
 @ blocks @
 identifier block_f;
-identifier wait_event =~ "^wait_event_.*";
+identifier wait =~ "^wait_.*";
 @@
   block_f(...) {
     ... when exists
-    wait_event(...)
+    wait(...)
     ... when exists
   }
 
@@ -49,12 +49,12 @@ identifier wait_event =~ "^wait_event_.*";
 // XXX currently reader_blocks supports only direct and 1-level indirect cases.
 @ reader_blocks_direct @
 identifier stream_reader.readstream;
-identifier wait_event =~ "^wait_event_.*";
+identifier wait =~ "^wait_.*";
 @@
   readstream(...)
   {
     ... when exists
-    wait_event(...)
+    wait(...)
     ... when exists
   }
 
-- 
2.20.1
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Cocci] [PATCH 2/2] *: convert stream-like files -> stream_open, even if they use noop_llseek
  2019-06-23  7:28 [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking Kirill Smelkov
@ 2019-06-23  7:29 ` Kirill Smelkov
  2019-06-23  7:35 ` [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking Julia Lawall
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Kirill Smelkov @ 2019-06-23  7:29 UTC (permalink / raw)
  To: cocci, linux-input, linux-iio, linux-kernel
  Cc: Dmitry Torokhov, Arnd Bergmann, Jiri Kosina, Benjamin Tissoires,
	Srinivas Pandruvada, Jan Blunck, Kirill Smelkov,
	Jonathan Cameron

This patch continues 10dce8af3422 (fs: stream_open - opener for
stream-like files so that read and write can run simultaneously without
deadlock) and c5bf68fe0c86 (*: convert stream-like files from
nonseekable_open -> stream_open) and teaches steam_open.cocci to
consider files as being stream-like not only if they have
.llseek=no_llseek, but also if they have .llseek=noop_llseek.

This is safe to do: the comment about noop_llseek says

	This is an implementation of ->llseek useable for the rare special case when
	userspace expects the seek to succeed but the (device) file is actually not
	able to perform the seek. In this case you use noop_llseek() instead of
	falling back to the default implementation of ->llseek.

and in general noop_llseek was massively added to drivers in 6038f373a3dc
(llseek: automatically add .llseek fop) when changing default for NULL .llseek
from NOP to no_llseek with the idea to avoid breaking compatibility, if
maybe some user-space program was using lseek on a device without caring
about the result, but caring if it was an error or not.

Amended semantic patch produces two changes when applied tree-wide:

        drivers/hid/hid-sensor-custom.c:690:8-24: WARNING: hid_sensor_custom_fops: .read() has stream semantic; safe to change nonseekable_open -> stream_open.
        drivers/input/mousedev.c:564:1-17: ERROR: mousedev_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.

Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Jan Blunck <jblunck@suse.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
---
 drivers/hid/hid-sensor-custom.c          | 2 +-
 drivers/input/mousedev.c                 | 2 +-
 scripts/coccinelle/api/stream_open.cocci | 9 ++++++++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index c60f82673cf2..fb827c295842 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -687,7 +687,7 @@ static int hid_sensor_custom_open(struct inode *inode, struct file *file)
 	if (test_and_set_bit(0, &sensor_inst->misc_opened))
 		return -EBUSY;
 
-	return nonseekable_open(inode, file);
+	return stream_open(inode, file);
 }
 
 static __poll_t hid_sensor_custom_poll(struct file *file,
diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
index 412fa71245af..58afd5253485 100644
--- a/drivers/input/mousedev.c
+++ b/drivers/input/mousedev.c
@@ -561,7 +561,7 @@ static int mousedev_open(struct inode *inode, struct file *file)
 		goto err_free_client;
 
 	file->private_data = client;
-	nonseekable_open(inode, file);
+	stream_open(inode, file);
 
 	return 0;
 
diff --git a/scripts/coccinelle/api/stream_open.cocci b/scripts/coccinelle/api/stream_open.cocci
index 12ce18fa6b74..df00d6619b06 100644
--- a/scripts/coccinelle/api/stream_open.cocci
+++ b/scripts/coccinelle/api/stream_open.cocci
@@ -134,6 +134,13 @@ identifier fops0.fops;
     .llseek = no_llseek,
   };
 
+@ has_noop_llseek @
+identifier fops0.fops;
+@@
+  struct file_operations fops = {
+    .llseek = noop_llseek,
+  };
+
 @ has_mmap @
 identifier fops0.fops;
 identifier mmap_f;
@@ -180,7 +187,7 @@ identifier splice_write_f;
 //
 // XXX for simplicity require no .{read/write}_iter and no .splice_{read/write} for now.
 // XXX maybe_steam.fops cannot be used in other rules - it gives "bad rule maybe_stream or bad variable fops".
-@ maybe_stream depends on (!has_llseek || has_no_llseek) && !has_mmap && !has_copy_file_range && !has_remap_file_range && !has_read_iter && !has_write_iter && !has_splice_read && !has_splice_write @
+@ maybe_stream depends on (!has_llseek || has_no_llseek || has_noop_llseek) && !has_mmap && !has_copy_file_range && !has_remap_file_range && !has_read_iter && !has_write_iter && !has_splice_read && !has_splice_write @
 identifier fops0.fops;
 @@
   struct file_operations fops = {
-- 
2.20.1
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking
  2019-06-23  7:28 [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking Kirill Smelkov
  2019-06-23  7:29 ` [Cocci] [PATCH 2/2] *: convert stream-like files -> stream_open, even if they use noop_llseek Kirill Smelkov
@ 2019-06-23  7:35 ` Julia Lawall
  2019-06-23  8:01 ` Julia Lawall
  2019-07-06 13:17 ` Masahiro Yamada
  3 siblings, 0 replies; 7+ messages in thread
From: Julia Lawall @ 2019-06-23  7:35 UTC (permalink / raw)
  To: Kirill Smelkov
  Cc: Sebastian Andrzej Siewior, linux-kernel, Bjorn Helgaas,
	Logan Gunthorpe, cocci



On Sun, 23 Jun 2019, Kirill Smelkov wrote:

> Previously steam_open.cocci was treating only wait_event_.* - e.g.
> wait_event_interruptible - as a blocking operation. However e.g.
> wait_for_completion_interruptible is also blocking, and so from this
> point of view it would be more logical to treat all wait_.* as a
> blocking point.
>
> The logic of this change actually came up for real when
> drivers/pci/switch/switchtec.c changed from using
> wait_event_interruptible to wait_for_completion_interruptible:
>
> 	https://lore.kernel.org/linux-pci/20190413170056.GA11293@deco.navytux.spb.ru/
> 	https://lore.kernel.org/linux-pci/20190415145456.GA15280@deco.navytux.spb.ru/
> 	https://lore.kernel.org/linux-pci/20190415154102.GB17661@deco.navytux.spb.ru/
>
> For a driver that uses nonseekable_open with read/write having stream
> semantic and read also calling e.g. wait_for_completion_interruptible,
> running stream_open.cocci before this patch would produce:
>
> 	WARNING: <driver>_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
>
> while after this patch it will report:
>
> 	ERROR: <driver>_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.

Are you really sure that every word that starts with wait_ in the Linux
kernel has the property you want?  How many of them are there?  Would it
be reasonable to put the names in the semantic patch explicitly?

julia

>
> Cc: Julia Lawall <Julia.Lawall@lip6.fr>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Bjorn Helgaas <helgaas@kernel.org>
> Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
> ---
>  scripts/coccinelle/api/stream_open.cocci | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/coccinelle/api/stream_open.cocci b/scripts/coccinelle/api/stream_open.cocci
> index 350145da7669..12ce18fa6b74 100644
> --- a/scripts/coccinelle/api/stream_open.cocci
> +++ b/scripts/coccinelle/api/stream_open.cocci
> @@ -35,11 +35,11 @@ type loff_t;
>  // a function that blocks
>  @ blocks @
>  identifier block_f;
> -identifier wait_event =~ "^wait_event_.*";
> +identifier wait =~ "^wait_.*";
>  @@
>    block_f(...) {
>      ... when exists
> -    wait_event(...)
> +    wait(...)
>      ... when exists
>    }
>
> @@ -49,12 +49,12 @@ identifier wait_event =~ "^wait_event_.*";
>  // XXX currently reader_blocks supports only direct and 1-level indirect cases.
>  @ reader_blocks_direct @
>  identifier stream_reader.readstream;
> -identifier wait_event =~ "^wait_event_.*";
> +identifier wait =~ "^wait_.*";
>  @@
>    readstream(...)
>    {
>      ... when exists
> -    wait_event(...)
> +    wait(...)
>      ... when exists
>    }
>
> --
> 2.20.1
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking
  2019-06-23  7:28 [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking Kirill Smelkov
  2019-06-23  7:29 ` [Cocci] [PATCH 2/2] *: convert stream-like files -> stream_open, even if they use noop_llseek Kirill Smelkov
  2019-06-23  7:35 ` [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking Julia Lawall
@ 2019-06-23  8:01 ` Julia Lawall
       [not found]   ` <alpine.DEB.2.21.1906231002020.4961@hadrien>
  2019-07-06 13:17 ` Masahiro Yamada
  3 siblings, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2019-06-23  8:01 UTC (permalink / raw)
  To: Kirill Smelkov
  Cc: Sebastian Andrzej Siewior, linux-kernel, Bjorn Helgaas,
	Logan Gunthorpe, cocci



On Sun, 23 Jun 2019, Kirill Smelkov wrote:

> Previously steam_open.cocci was treating only wait_event_.* - e.g.
> wait_event_interruptible - as a blocking operation. However e.g.
> wait_for_completion_interruptible is also blocking, and so from this
> point of view it would be more logical to treat all wait_.* as a
> blocking point.
>
> The logic of this change actually came up for real when
> drivers/pci/switch/switchtec.c changed from using
> wait_event_interruptible to wait_for_completion_interruptible:
>
> 	https://lore.kernel.org/linux-pci/20190413170056.GA11293@deco.navytux.spb.ru/
> 	https://lore.kernel.org/linux-pci/20190415145456.GA15280@deco.navytux.spb.ru/
> 	https://lore.kernel.org/linux-pci/20190415154102.GB17661@deco.navytux.spb.ru/
>
> For a driver that uses nonseekable_open with read/write having stream
> semantic and read also calling e.g. wait_for_completion_interruptible,
> running stream_open.cocci before this patch would produce:
>
> 	WARNING: <driver>_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
>
> while after this patch it will report:
>
> 	ERROR: <driver>_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
>
> Cc: Julia Lawall <Julia.Lawall@lip6.fr>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Bjorn Helgaas <helgaas@kernel.org>
> Signed-off-by: Kirill Smelkov <kirr@nexedi.com>

Acked-by: Julia Lawall <julia.lawall@lip6.fr>


> ---
>  scripts/coccinelle/api/stream_open.cocci | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/coccinelle/api/stream_open.cocci b/scripts/coccinelle/api/stream_open.cocci
> index 350145da7669..12ce18fa6b74 100644
> --- a/scripts/coccinelle/api/stream_open.cocci
> +++ b/scripts/coccinelle/api/stream_open.cocci
> @@ -35,11 +35,11 @@ type loff_t;
>  // a function that blocks
>  @ blocks @
>  identifier block_f;
> -identifier wait_event =~ "^wait_event_.*";
> +identifier wait =~ "^wait_.*";
>  @@
>    block_f(...) {
>      ... when exists
> -    wait_event(...)
> +    wait(...)
>      ... when exists
>    }
>
> @@ -49,12 +49,12 @@ identifier wait_event =~ "^wait_event_.*";
>  // XXX currently reader_blocks supports only direct and 1-level indirect cases.
>  @ reader_blocks_direct @
>  identifier stream_reader.readstream;
> -identifier wait_event =~ "^wait_event_.*";
> +identifier wait =~ "^wait_.*";
>  @@
>    readstream(...)
>    {
>      ... when exists
> -    wait_event(...)
> +    wait(...)
>      ... when exists
>    }
>
> --
> 2.20.1
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking
       [not found]   ` <alpine.DEB.2.21.1906231002020.4961@hadrien>
@ 2019-06-23  8:18     ` Kirill Smelkov
  0 siblings, 0 replies; 7+ messages in thread
From: Kirill Smelkov @ 2019-06-23  8:18 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Bjorn Helgaas, Sebastian Andrzej Siewior, Logan Gunthorpe, cocci,
	linux-kernel

On Sun, Jun 23, 2019 at 10:02:17AM +0200, Julia Lawall wrote:
> On Sun, 23 Jun 2019, Kirill Smelkov wrote:
> > On Sun, Jun 23, 2019 at 09:35:17AM +0200, Julia Lawall wrote:
> > > On Sun, 23 Jun 2019, Kirill Smelkov wrote:
> > >
> > > > Previously steam_open.cocci was treating only wait_event_.* - e.g.
> > > > wait_event_interruptible - as a blocking operation. However e.g.
> > > > wait_for_completion_interruptible is also blocking, and so from this
> > > > point of view it would be more logical to treat all wait_.* as a
> > > > blocking point.
> > > >
> > > > The logic of this change actually came up for real when
> > > > drivers/pci/switch/switchtec.c changed from using
> > > > wait_event_interruptible to wait_for_completion_interruptible:
> > > >
> > > > 	https://lore.kernel.org/linux-pci/20190413170056.GA11293@deco.navytux.spb.ru/
> > > > 	https://lore.kernel.org/linux-pci/20190415145456.GA15280@deco.navytux.spb.ru/
> > > > 	https://lore.kernel.org/linux-pci/20190415154102.GB17661@deco.navytux.spb.ru/
> > > >
> > > > For a driver that uses nonseekable_open with read/write having stream
> > > > semantic and read also calling e.g. wait_for_completion_interruptible,
> > > > running stream_open.cocci before this patch would produce:
> > > >
> > > > 	WARNING: <driver>_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
> > > >
> > > > while after this patch it will report:
> > > >
> > > > 	ERROR: <driver>_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
> > >
> > > Are you really sure that every word that starts with wait_ in the Linux
> > > kernel has the property you want?  How many of them are there?  Would it
> > > be reasonable to put the names in the semantic patch explicitly?
> >
> > We are talking about function calls or macros here only. I think that
> > by default every "wait.*()" has blocking semantic, and if there are
> > something that does not block, it should be instead white-listed with
> > the default still being treated as "all wait.*() blocks".
> >
> > Here are the list of wait functions & macros (if I read tags format
> > correctly). They were generated with `grep -w d tags |grep '^wait'` and
> > `grep -w f tags |grep '^wait'`. Offhand they all seem to be of blocking
> > kind to me.
> 
> OK, thanks for the list.
> 
> julia

On Sun, Jun 23, 2019 at 10:01:52AM +0200, Julia Lawall wrote:
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>

Julia, you are welcome. Thanks for ACK.

Kirill
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking
  2019-06-23  7:28 [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking Kirill Smelkov
                   ` (2 preceding siblings ...)
  2019-06-23  8:01 ` Julia Lawall
@ 2019-07-06 13:17 ` Masahiro Yamada
  2019-07-07 18:20   ` Kirill Smelkov
  3 siblings, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2019-07-06 13:17 UTC (permalink / raw)
  To: Kirill Smelkov
  Cc: Logan Gunthorpe, Sebastian Andrzej Siewior, Bjorn Helgaas, cocci,
	Linux Kernel Mailing List

On Sun, Jun 23, 2019 at 4:29 PM Kirill Smelkov <kirr@nexedi.com> wrote:
>
> Previously steam_open.cocci was treating only wait_event_.* - e.g.
> wait_event_interruptible - as a blocking operation. However e.g.
> wait_for_completion_interruptible is also blocking, and so from this
> point of view it would be more logical to treat all wait_.* as a
> blocking point.
>
> The logic of this change actually came up for real when
> drivers/pci/switch/switchtec.c changed from using
> wait_event_interruptible to wait_for_completion_interruptible:
>
>         https://lore.kernel.org/linux-pci/20190413170056.GA11293@deco.navytux.spb.ru/
>         https://lore.kernel.org/linux-pci/20190415145456.GA15280@deco.navytux.spb.ru/
>         https://lore.kernel.org/linux-pci/20190415154102.GB17661@deco.navytux.spb.ru/
>
> For a driver that uses nonseekable_open with read/write having stream
> semantic and read also calling e.g. wait_for_completion_interruptible,
> running stream_open.cocci before this patch would produce:
>
>         WARNING: <driver>_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
>
> while after this patch it will report:
>
>         ERROR: <driver>_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
>
> Cc: Julia Lawall <Julia.Lawall@lip6.fr>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Bjorn Helgaas <helgaas@kernel.org>
> Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
> ---

Applied to linux-kbuild. Thanks.

>  scripts/coccinelle/api/stream_open.cocci | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/coccinelle/api/stream_open.cocci b/scripts/coccinelle/api/stream_open.cocci
> index 350145da7669..12ce18fa6b74 100644
> --- a/scripts/coccinelle/api/stream_open.cocci
> +++ b/scripts/coccinelle/api/stream_open.cocci
> @@ -35,11 +35,11 @@ type loff_t;
>  // a function that blocks
>  @ blocks @
>  identifier block_f;
> -identifier wait_event =~ "^wait_event_.*";
> +identifier wait =~ "^wait_.*";
>  @@
>    block_f(...) {
>      ... when exists
> -    wait_event(...)
> +    wait(...)
>      ... when exists
>    }
>
> @@ -49,12 +49,12 @@ identifier wait_event =~ "^wait_event_.*";
>  // XXX currently reader_blocks supports only direct and 1-level indirect cases.
>  @ reader_blocks_direct @
>  identifier stream_reader.readstream;
> -identifier wait_event =~ "^wait_event_.*";
> +identifier wait =~ "^wait_.*";
>  @@
>    readstream(...)
>    {
>      ... when exists
> -    wait_event(...)
> +    wait(...)
>      ... when exists
>    }
>
> --
> 2.20.1
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci



-- 
Best Regards
Masahiro Yamada
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking
  2019-07-06 13:17 ` Masahiro Yamada
@ 2019-07-07 18:20   ` Kirill Smelkov
  0 siblings, 0 replies; 7+ messages in thread
From: Kirill Smelkov @ 2019-07-07 18:20 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Logan Gunthorpe, Sebastian Andrzej Siewior, Bjorn Helgaas, cocci,
	Linux Kernel Mailing List

On Sat, Jul 06, 2019 at 10:17:18PM +0900, Masahiro Yamada wrote:
> On Sun, Jun 23, 2019 at 4:29 PM Kirill Smelkov <kirr@nexedi.com> wrote:
> >
> > Previously steam_open.cocci was treating only wait_event_.* - e.g.
> > wait_event_interruptible - as a blocking operation. However e.g.
> > wait_for_completion_interruptible is also blocking, and so from this
> > point of view it would be more logical to treat all wait_.* as a
> > blocking point.
> >
> > The logic of this change actually came up for real when
> > drivers/pci/switch/switchtec.c changed from using
> > wait_event_interruptible to wait_for_completion_interruptible:
> >
> >         https://lore.kernel.org/linux-pci/20190413170056.GA11293@deco.navytux.spb.ru/
> >         https://lore.kernel.org/linux-pci/20190415145456.GA15280@deco.navytux.spb.ru/
> >         https://lore.kernel.org/linux-pci/20190415154102.GB17661@deco.navytux.spb.ru/
> >
> > For a driver that uses nonseekable_open with read/write having stream
> > semantic and read also calling e.g. wait_for_completion_interruptible,
> > running stream_open.cocci before this patch would produce:
> >
> >         WARNING: <driver>_fops: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open.
> >
> > while after this patch it will report:
> >
> >         ERROR: <driver>_fops: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix.
> >
> > Cc: Julia Lawall <Julia.Lawall@lip6.fr>
> > Cc: Logan Gunthorpe <logang@deltatee.com>
> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Cc: Bjorn Helgaas <helgaas@kernel.org>
> > Signed-off-by: Kirill Smelkov <kirr@nexedi.com>
> > ---
> 
> Applied to linux-kbuild. Thanks.

Thanks.


> >  scripts/coccinelle/api/stream_open.cocci | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/coccinelle/api/stream_open.cocci b/scripts/coccinelle/api/stream_open.cocci
> > index 350145da7669..12ce18fa6b74 100644
> > --- a/scripts/coccinelle/api/stream_open.cocci
> > +++ b/scripts/coccinelle/api/stream_open.cocci
> > @@ -35,11 +35,11 @@ type loff_t;
> >  // a function that blocks
> >  @ blocks @
> >  identifier block_f;
> > -identifier wait_event =~ "^wait_event_.*";
> > +identifier wait =~ "^wait_.*";
> >  @@
> >    block_f(...) {
> >      ... when exists
> > -    wait_event(...)
> > +    wait(...)
> >      ... when exists
> >    }
> >
> > @@ -49,12 +49,12 @@ identifier wait_event =~ "^wait_event_.*";
> >  // XXX currently reader_blocks supports only direct and 1-level indirect cases.
> >  @ reader_blocks_direct @
> >  identifier stream_reader.readstream;
> > -identifier wait_event =~ "^wait_event_.*";
> > +identifier wait =~ "^wait_.*";
> >  @@
> >    readstream(...)
> >    {
> >      ... when exists
> > -    wait_event(...)
> > +    wait(...)
> >      ... when exists
> >    }
> >
> > --
> > 2.20.1
> > _______________________________________________
> > Cocci mailing list
> > Cocci@systeme.lip6.fr
> > https://systeme.lip6.fr/mailman/listinfo/cocci
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-07-07 18:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-23  7:28 [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking Kirill Smelkov
2019-06-23  7:29 ` [Cocci] [PATCH 2/2] *: convert stream-like files -> stream_open, even if they use noop_llseek Kirill Smelkov
2019-06-23  7:35 ` [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking Julia Lawall
2019-06-23  8:01 ` Julia Lawall
     [not found]   ` <alpine.DEB.2.21.1906231002020.4961@hadrien>
2019-06-23  8:18     ` Kirill Smelkov
2019-07-06 13:17 ` Masahiro Yamada
2019-07-07 18:20   ` Kirill Smelkov

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).