Re: [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking

From: Kirill Smelkov <kirr@nexedi.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Logan Gunthorpe <logang@deltatee.com>,
	cocci@systeme.lip6.fr, linux-kernel@vger.kernel.org
Subject: Re: [Cocci] [PATCH 1/2] coccinelle: api/stream_open: treat all wait_.*() calls as blocking
Date: Sun, 23 Jun 2019 08:18:05 +0000	[thread overview]
Message-ID: <20190623081800.GA1889@deco.navytux.spb.ru> (raw)
In-Reply-To: <alpine.DEB.2.21.1906231001360.4961@hadrien> <alpine.DEB.2.21.1906231002020.4961@hadrien>

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