Re: [PATCH 07/30] ocfs2: don't use flush_scheduled_work()

Re: [PATCH 07/30] ocfs2: don't use flush_scheduled_work()

[Joel Becker]

On Sat, Dec 11, 2010 at 05:51:24PM +0100, Tejun Heo wrote:
> flush_scheduled_work() is deprecated and scheduled to be removed.
> 
> * cancel_delayed_work() + flush_schedule_work() ->
>   cancel_delayed_work_sync().
> 
> * flush qs->qs_work directly on exit instead.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Mark Fasheh <mfasheh@suse.com>
> Cc: Joel Becker <joel.becker@oracle.com>

Acked-by: Joel Becker <joel.becker@oracle.com>

-- 

#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Joel Becker
Senior Development Manager
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Takashi Iwai]

At Sat, 11 Dec 2010 17:51:26 +0100,
Tejun Heo wrote:
> 
> flush_scheduled_work() is deprecated and scheduled to be removed.
> 
> * cancel[_delayed]_work() + flush_scheduled_work() ->
>   cancel[_delayed]_work_sync().
> 
> * wm8350, wm8753 and soc-core use custom code to cancel a delayed
>   work, execute it immediately if it was pending and wait for its
>   completion.  This is equivalent to flush_delayed_work_sync().  Use
>   it instead.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Jaroslav Kysela <perex@perex.cz>
> ---
> This is part of a series to remove flush_scheduled_work() usage to
> prepare for deprecation of flush_scheduled_work().  Patches in this
> series are self contained and mostly straight-forward.
> 
> Please feel free to take it into the appropriate tree, or just ack it.
> In the latter case, I'll merge the patch through the workqueue tree
> during the next merge window.

The most of parts look OK to me, but I'm not sure about ASoC changes,
e.g. below one:

> diff --git a/sound/soc/codecs/wm8350.c b/sound/soc/codecs/wm8350.c
> index 7611add..b3e9fac 100644
> --- a/sound/soc/codecs/wm8350.c
> +++ b/sound/soc/codecs/wm8350.c
> @@ -1626,7 +1626,6 @@ static int  wm8350_codec_remove(struct snd_soc_codec *codec)
>  {
>  	struct wm8350_data *priv = snd_soc_codec_get_drvdata(codec);
>  	struct wm8350 *wm8350 = dev_get_platdata(codec->dev);
> -	int ret;
>  
>  	wm8350_clear_bits(wm8350, WM8350_JACK_DETECT,
>  			  WM8350_JDL_ENA | WM8350_JDR_ENA);
> @@ -1641,15 +1640,9 @@ static int  wm8350_codec_remove(struct snd_soc_codec *codec)
>  	priv->hpr.jack = NULL;
>  	priv->mic.jack = NULL;
>  
> -	/* cancel any work waiting to be queued. */
> -	ret = cancel_delayed_work(&codec->delayed_work);
> -
>  	/* if there was any work waiting then we run it now and
>  	 * wait for its completion */
> -	if (ret) {
> -		schedule_delayed_work(&codec->delayed_work, 0);
> -		flush_scheduled_work();
> -	}
> +	flush_delayed_work_sync(&codec->delayed_work);

I vaguely remember Liam introduced this kind of code by some reason.

Liam, isn't it better for cancel_delayed_work_sync(), or should it be
like the above?


thanks,

Takashi

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Tejun Heo]

Hello, Takashi.

On 12/12/2010 09:56 AM, Takashi Iwai wrote:
>> diff --git a/sound/soc/codecs/wm8350.c b/sound/soc/codecs/wm8350.c
>> index 7611add..b3e9fac 100644
>> --- a/sound/soc/codecs/wm8350.c
>> +++ b/sound/soc/codecs/wm8350.c
>> @@ -1626,7 +1626,6 @@ static int  wm8350_codec_remove(struct snd_soc_codec *codec)
>>  {
>>  	struct wm8350_data *priv = snd_soc_codec_get_drvdata(codec);
>>  	struct wm8350 *wm8350 = dev_get_platdata(codec->dev);
>> -	int ret;
>>  
>>  	wm8350_clear_bits(wm8350, WM8350_JACK_DETECT,
>>  			  WM8350_JDL_ENA | WM8350_JDR_ENA);
>> @@ -1641,15 +1640,9 @@ static int  wm8350_codec_remove(struct snd_soc_codec *codec)
>>  	priv->hpr.jack = NULL;
>>  	priv->mic.jack = NULL;
>>  
>> -	/* cancel any work waiting to be queued. */
>> -	ret = cancel_delayed_work(&codec->delayed_work);
>> -
>>  	/* if there was any work waiting then we run it now and
>>  	 * wait for its completion */
>> -	if (ret) {
>> -		schedule_delayed_work(&codec->delayed_work, 0);
>> -		flush_scheduled_work();
>> -	}
>> +	flush_delayed_work_sync(&codec->delayed_work);
> 
> I vaguely remember Liam introduced this kind of code by some reason.
> 
> Liam, isn't it better for cancel_delayed_work_sync(), or should it be
> like the above?

flush_delayed_work_sync() semantics has been recently changed such
that if a delayed work is pending it's queued immediately and then
completion is waited.  IOW, the behavior remains unchanged with the
above change.

Thanks.

-- 
tejun

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Takashi Iwai]

At Sun, 12 Dec 2010 10:15:55 +0100,
Tejun Heo wrote:
> 
> Hello, Takashi.
> 
> On 12/12/2010 09:56 AM, Takashi Iwai wrote:
> >> diff --git a/sound/soc/codecs/wm8350.c b/sound/soc/codecs/wm8350.c
> >> index 7611add..b3e9fac 100644
> >> --- a/sound/soc/codecs/wm8350.c
> >> +++ b/sound/soc/codecs/wm8350.c
> >> @@ -1626,7 +1626,6 @@ static int  wm8350_codec_remove(struct snd_soc_codec *codec)
> >>  {
> >>  	struct wm8350_data *priv = snd_soc_codec_get_drvdata(codec);
> >>  	struct wm8350 *wm8350 = dev_get_platdata(codec->dev);
> >> -	int ret;
> >>  
> >>  	wm8350_clear_bits(wm8350, WM8350_JACK_DETECT,
> >>  			  WM8350_JDL_ENA | WM8350_JDR_ENA);
> >> @@ -1641,15 +1640,9 @@ static int  wm8350_codec_remove(struct snd_soc_codec *codec)
> >>  	priv->hpr.jack = NULL;
> >>  	priv->mic.jack = NULL;
> >>  
> >> -	/* cancel any work waiting to be queued. */
> >> -	ret = cancel_delayed_work(&codec->delayed_work);
> >> -
> >>  	/* if there was any work waiting then we run it now and
> >>  	 * wait for its completion */
> >> -	if (ret) {
> >> -		schedule_delayed_work(&codec->delayed_work, 0);
> >> -		flush_scheduled_work();
> >> -	}
> >> +	flush_delayed_work_sync(&codec->delayed_work);
> > 
> > I vaguely remember Liam introduced this kind of code by some reason.
> > 
> > Liam, isn't it better for cancel_delayed_work_sync(), or should it be
> > like the above?
> 
> flush_delayed_work_sync() semantics has been recently changed such
> that if a delayed work is pending it's queued immediately and then
> completion is waited.  IOW, the behavior remains unchanged with the
> above change.

Yes, I noticed it while I was reviewing your patch.  So it's pretty
correct.

Meanwhile, I wondered whether it's the really wanted behavior for
that particular code path, thus the previous question to Liam.


thanks,

Takashi

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Mark Brown]

On Sun, Dec 12, 2010 at 01:38:36PM +0100, Takashi Iwai wrote:

> Meanwhile, I wondered whether it's the really wanted behavior for
> that particular code path, thus the previous question to Liam.

Yes, it's desired behaviour.  That's what the old code was trying to do.
I've not seen the original patch, could someone resend it to me please
(or send a pointer)?

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Tejun Heo]

On 12/12/2010 01:40 PM, Mark Brown wrote:
> On Sun, Dec 12, 2010 at 01:38:36PM +0100, Takashi Iwai wrote:
> 
>> Meanwhile, I wondered whether it's the really wanted behavior for
>> that particular code path, thus the previous question to Liam.
> 
> Yes, it's desired behaviour.  That's what the old code was trying to do.
> I've not seen the original patch, could someone resend it to me please
> (or send a pointer)?

Weirdly, lkml seemed to have eaten the original postings.  I can't
find them in any archive.  The following is the original patch.

  http://git.kernel.org/?p=linux/kernel/git/tj/wq.git;a=commitdiff;h=01f8126c2e14245cf0ba11a25fb9a291710066e1;hp=cd591981a8d06d6aaca6d7ef2412d12a7adf47ed

Thanks.

-- 
tejun

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Mark Brown]

On Sun, Dec 12, 2010 at 05:02:25PM +0100, Tejun Heo wrote:

> Weirdly, lkml seemed to have eaten the original postings.  I can't
> find them in any archive.  The following is the original patch.

Possibly too big or something?

>   http://git.kernel.org/?p=linux/kernel/git/tj/wq.git;a=commitdiff;h=01f8126c2e14245cf0ba11a25fb9a291710066e1;hp=cd591981a8d06d6aaca6d7ef2412d12a7adf47ed

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

For future reference please note that sound/soc is maintained as a
subtree of ALSA - if you could send patches to that separately to myself
and Liam as well as Takashi and Jaroslav that'd make life a little
easier.

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Tejun Heo]

Hello,

On 12/12/2010 07:47 PM, Mark Brown wrote:
> On Sun, Dec 12, 2010 at 05:02:25PM +0100, Tejun Heo wrote:
> 
>> Weirdly, lkml seemed to have eaten the original postings.  I can't
>> find them in any archive.  The following is the original patch.
> 
> Possibly too big or something?
> 
>>   http://git.kernel.org/?p=linux/kernel/git/tj/wq.git;a=commitdiff;h=01f8126c2e14245cf0ba11a25fb9a291710066e1;hp=cd591981a8d06d6aaca6d7ef2412d12a7adf47ed
> 
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> For future reference please note that sound/soc is maintained as a
> subtree of ALSA - if you could send patches to that separately to myself
> and Liam as well as Takashi and Jaroslav that'd make life a little
> easier.

I can resplit and resend if necessary.  Takashi, do you wanna take the
patch as-is or resplit?  Or shall I route it through wq tree?

Thanks.

-- 
tejun

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Mark Brown]

On Sun, Dec 12, 2010 at 07:50:02PM +0100, Tejun Heo wrote:

> > For future reference please note that sound/soc is maintained as a
> > subtree of ALSA - if you could send patches to that separately to myself
> > and Liam as well as Takashi and Jaroslav that'd make life a little
> > easier.

> I can resplit and resend if necessary.  Takashi, do you wanna take the
> patch as-is or resplit?  Or shall I route it through wq tree?

No need, just merge it along with the rest of the patch however that's
going.  I was mentioning this for future reference rather than for the
present patch.

Liam, will the TWL6040 updates Magi posted the other day also be
affected?  I seem to remember they added the same pattern as WM835x has.

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Liam Girdwood]

On Sun, 2010-12-12 at 19:00 +0000, Mark Brown wrote:
> On Sun, Dec 12, 2010 at 07:50:02PM +0100, Tejun Heo wrote:
> 
> > > For future reference please note that sound/soc is maintained as a
> > > subtree of ALSA - if you could send patches to that separately to myself
> > > and Liam as well as Takashi and Jaroslav that'd make life a little
> > > easier.
> 
> > I can resplit and resend if necessary.  Takashi, do you wanna take the
> > patch as-is or resplit?  Or shall I route it through wq tree?
> 
> No need, just merge it along with the rest of the patch however that's
> going.  I was mentioning this for future reference rather than for the
> present patch.
> 
> Liam, will the TWL6040 updates Magi posted the other day also be
> affected?  I seem to remember they added the same pattern as WM835x has.

Not sure myself.

Magi, could you check your patch queue for any use of
flush_scheduled_work() here ?

Thanks

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

RE: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Olaya, Margarita]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1683 bytes --]

Hi Liam, 

> -----Original Message-----
> From: Liam Girdwood [mailto:lrg@slimlogic.co.uk] 
> Sent: Monday, December 13, 2010 7:32 AM
> To: Mark Brown; Olaya, Margarita
> Cc: Tejun Heo; Takashi Iwai; linux-kernel@vger.kernel.org; 
> Jaroslav Kysela
> Subject: Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
> 
> On Sun, 2010-12-12 at 19:00 +0000, Mark Brown wrote:
> > On Sun, Dec 12, 2010 at 07:50:02PM +0100, Tejun Heo wrote:
> > 
> > > > For future reference please note that sound/soc is 
> maintained as a
> > > > subtree of ALSA - if you could send patches to that 
> separately to myself
> > > > and Liam as well as Takashi and Jaroslav that'd make 
> life a little
> > > > easier.
> > 
> > > I can resplit and resend if necessary.  Takashi, do you 
> wanna take the
> > > patch as-is or resplit?  Or shall I route it through wq tree?
> > 
> > No need, just merge it along with the rest of the patch 
> however that's
> > going.  I was mentioning this for future reference rather 
> than for the
> > present patch.
> > 
> > Liam, will the TWL6040 updates Magi posted the other day also be
> > affected?  I seem to remember they added the same pattern 
> as WM835x has.
> 
> Not sure myself.
> 
> Magi, could you check your patch queue for any use of
> flush_scheduled_work() here ?

flush_scheduled_work() is not being used in TWL6040 patches.

Regards,
Margarita

> 
> Thanks
> 
> Liam
> -- 
> Freelance Developer, SlimLogic Ltd
> ASoC and Voltage Regulator Maintainer.
> http://www.slimlogic.co.uk
> 
> ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Takashi Iwai]

At Sun, 12 Dec 2010 12:40:41 +0000,
Mark Brown wrote:
> 
> On Sun, Dec 12, 2010 at 01:38:36PM +0100, Takashi Iwai wrote:
> 
> > Meanwhile, I wondered whether it's the really wanted behavior for
> > that particular code path, thus the previous question to Liam.
> 
> Yes, it's desired behaviour.  That's what the old code was trying to do.

OK, now I merged to sound git tree.
Also it's merged back to topic/asoc branch with a conflict fix.
Please pull appropriately.

(I still don't remember why it had to be flush_work_sync() instead
 of cancel_work_sync() in the remove callback path for ASoC, though...
 Both aren't so much different nowadays and should work fine in such a
 case, though :)


thanks,

Takashi

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Liam Girdwood]

On Mon, 2010-12-13 at 09:34 +0100, Takashi Iwai wrote:
> At Sun, 12 Dec 2010 12:40:41 +0000,
> Mark Brown wrote:
> > 
> > On Sun, Dec 12, 2010 at 01:38:36PM +0100, Takashi Iwai wrote:
> > 
> > > Meanwhile, I wondered whether it's the really wanted behavior for
> > > that particular code path, thus the previous question to Liam.
> > 
> > Yes, it's desired behaviour.  That's what the old code was trying to do.
> 
> OK, now I merged to sound git tree.
> Also it's merged back to topic/asoc branch with a conflict fix.
> Please pull appropriately.
> 
> (I still don't remember why it had to be flush_work_sync() instead
>  of cancel_work_sync() in the remove callback path for ASoC, though...
>  Both aren't so much different nowadays and should work fine in such a
>  case, though :)

Fwiw, I can't remember either why it had to be flush_work_sync() here.
The initial soc-core stuff was 5 years ago and the reason is well and
truly forgotten ;-)

Thanks

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 09/30] sound: don't use flush_scheduled_work()

[Mark Brown]

On Mon, Dec 13, 2010 at 09:34:37AM +0100, Takashi Iwai wrote:

> OK, now I merged to sound git tree.
> Also it's merged back to topic/asoc branch with a conflict fix.
> Please pull appropriately.

Now merged up into my tree also.

Re: [PATCH 20/30] macintosh/ams: don't use flush_scheduled_work()

[Michael Hanselmann]

On Sat, Dec 11, 2010 at 8:19 PM, Jean Delvare <khali@linux-fr.org> wrote:
> On Sat, 11 Dec 2010 17:51:37 +0100, Tejun Heo wrote:
>> flush_scheduled_work() is deprecated and scheduled to be removed.
>> Directly flush ams_info.worker on detach instead.
>
> The AMS driver is maintained by Stelian Pop and Michael Hanselmann
> according to MAINTAINERS. I am Cc'ing them both.

Change looks good to me, but can't test due to lack of hardware.

Michael

Re: [PATCH 20/30] macintosh/ams: don't use flush_scheduled_work()

[Stelian Pop]

On Sun, Dec 12, 2010 at 02:37:28PM +0100, Michael Hanselmann wrote:

> On Sat, Dec 11, 2010 at 8:19 PM, Jean Delvare <khali@linux-fr.org> wrote:
> > On Sat, 11 Dec 2010 17:51:37 +0100, Tejun Heo wrote:
> >> flush_scheduled_work() is deprecated and scheduled to be removed.
> >> Directly flush ams_info.worker on detach instead.
> >
> > The AMS driver is maintained by Stelian Pop and Michael Hanselmann
> > according to MAINTAINERS. I am Cc'ing them both.
> 
> Change looks good to me, but can't test due to lack of hardware.

Same here. Looks like this driver can be orphaned...

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [PATCH 20/30] macintosh/ams: don't use flush_scheduled_work()

[Jean Delvare]

On Mon, 13 Dec 2010 11:16:38 +0100, Stelian Pop wrote:
> On Sun, Dec 12, 2010 at 02:37:28PM +0100, Michael Hanselmann wrote:
> 
> > On Sat, Dec 11, 2010 at 8:19 PM, Jean Delvare <khali@linux-fr.org> wrote:
> > > On Sat, 11 Dec 2010 17:51:37 +0100, Tejun Heo wrote:
> > >> flush_scheduled_work() is deprecated and scheduled to be removed.
> > >> Directly flush ams_info.worker on detach instead.
> > >
> > > The AMS driver is maintained by Stelian Pop and Michael Hanselmann
> > > according to MAINTAINERS. I am Cc'ing them both.
> > 
> > Change looks good to me, but can't test due to lack of hardware.
> 
> Same here. Looks like this driver can be orphaned...

If so, please submit a patch updating MAINTAINERS. Otherwise you'll
keep getting patches asking for your acknowledgment.

-- 
Jean Delvare

[PATCH] Remove myself from MAINTAINERS as I no longer have the hardware to test.

[Stelian Pop]

This driver is PPC only, and I can no longer test it.

Subject: [PATCH] Remove myself from MAINTAINERS as I no longer have the hardware to test.

Signed-off-by: Stelian Pop <stelian@popies.net>
---
 MAINTAINERS |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

On Mon, Dec 13, 2010 at 11:18:17AM +0100, Jean Delvare wrote:

> > > > The AMS driver is maintained by Stelian Pop and Michael Hanselmann
> > > > according to MAINTAINERS. I am Cc'ing them both.
> > > 
> > > Change looks good to me, but can't test due to lack of hardware.
> > 
> > Same here. Looks like this driver can be orphaned...
> 
> If so, please submit a patch updating MAINTAINERS. Otherwise you'll
> keep getting patches asking for your acknowledgment.

Here it is.

I'll let Michael remove itself from MAINTAINERS and orphan the
driver if he no longer has any related hardware.

Thanks,

Stelian.

diff --git a/MAINTAINERS b/MAINTAINERS
index 1a1c27b..61d8c41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -429,7 +429,6 @@ S:	Supported
 F:	arch/x86/kernel/microcode_amd.c
 
 AMS (Apple Motion Sensor) DRIVER
-M:	Stelian Pop <stelian@popies.net>
 M:	Michael Hanselmann <linux-kernel@hansmi.ch>
 S:	Supported
 F:	drivers/macintosh/ams/
-- 
1.7.1

Re: [PATCH] Remove myself from MAINTAINERS as I no longer have the hardware to test.

[Jean Delvare]

Hi Stelian,

On Mon, 13 Dec 2010 11:40:55 +0100, Stelian Pop wrote:
> This driver is PPC only, and I can no longer test it.
> 
> Subject: [PATCH] Remove myself from MAINTAINERS as I no longer have the hardware to test.
> 
> Signed-off-by: Stelian Pop <stelian@popies.net>
> ---
>  MAINTAINERS |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> (...)
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1a1c27b..61d8c41 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -429,7 +429,6 @@ S:	Supported
>  F:	arch/x86/kernel/microcode_amd.c
>  
>  AMS (Apple Motion Sensor) DRIVER
> -M:	Stelian Pop <stelian@popies.net>
>  M:	Michael Hanselmann <linux-kernel@hansmi.ch>
>  S:	Supported
>  F:	drivers/macintosh/ams/

I suggest that you send this patch to Andrew Morton or Benjamin
Herrenschmidt. It doesn't fit in any of my trees so I can't pick it.

Thanks,
-- 
Jean Delvare

Re: [PATCH 21/30] pcmcia/ipwireless: don't use flush_scheduled_work()

[David Sterba]

On Saturday 11 December 2010 17:51:38 Tejun Heo wrote:
> flush_scheduled_work() is deprecated and scheduled to be removed.
> Directly flush the used works instead.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: David Sterba <dsterba@suse.cz>

Acked-by: David Sterba <dsterba@suse.cz>


Dave

Re: [PATCH 05/30] net/dsa: don't use flush_scheduled_work()

[Lennert Buytenhek]

On Sat, Dec 11, 2010 at 05:51:22PM +0100, Tejun Heo wrote:

> flush_scheduled_work() is deprecated and scheduled to be removed.
> Directly flush dst->link_poll_work on remove instead.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Lennert Buytenhek <buytenh@wantstofly.org>

Acked-by: Lennert Buytenhek <buytenh@wantstofly.org>

Re: [PATCH 27/30] mtd: don't use flush_scheduled_work()

[Artem Bityutskiy]

On Sat, 2010-12-11 at 17:51 +0100, Tejun Heo wrote:
> flush_scheduled_work() is deprecated and scheduled to be removed.
> Directly flush cxt->work_{erase|write} on removal instead.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: linux-mtd@lists.infradead.org
> ---
> This is part of a series to remove flush_scheduled_work() usage to
> prepare for deprecation of flush_scheduled_work().  Patches in this
> series are self contained and mostly straight-forward.
> 
> Please feel free to take it into the appropriate tree, or just ack it.
> In the latter case, I'll merge the patch through the workqueue tree
> during the next merge window.
> 
> If you're seeing this patch for the second time, it's because the
> commit hasn't showed up in mainline yet.  Please let me know what
> should be done.

Pushed to l2-mtd-2.6.git, thanks. This means this will be merged to the
mtd tree a bit later and will go upstream.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH 28/30] battery: don't use flush_scheduled_work()

[Anton Vorontsov]

On Sat, Dec 11, 2010 at 05:51:45PM +0100, Tejun Heo wrote:
> flush_scheduled_work() is deprecated and scheduled to be removed.
> 
> In battery drivers, the work can be canceled on probe failure and
> removal and should be flushed on suspend.  Replace
> flush_scheduled_work() usages with direct cancels and flushes.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Anton Vorontsov <cbouatmailru@gmail.com>
> ---
> This is part of a series to remove flush_scheduled_work() usage to
> prepare for deprecation of flush_scheduled_work().  Patches in this
> series are self contained and mostly straight-forward.
> 
> Please feel free to take it into the appropriate tree, or just ack it.
> In the latter case, I'll merge the patch through the workqueue tree
> during the next merge window.
> 
> If you're seeing this patch for the second time, it's because the
> commit hasn't showed up in mainline yet.  Please let me know what
> should be done.
> 
> Thank you.

I applied this to battery-2.6.git, thanks!

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCHSET] workqueue: remove flush_scheduled_work() usage

[Tejun Heo]

On Sat, Dec 11, 2010 at 05:51:17PM +0100, Tejun Heo wrote:
> Hello,
> 
> This patchset removes flush_scheduled_work() usage to prepare for
> deprecation of the interface.  Patches in this series are self
> contained and mostly straight-forward.  More complex conversions will
> be done in separate patchsets.
> 
> This patchset contains the following 30 patches.
> 
>  0001-infiniband-update-workqueue-usage.patch
>  0002-isdn-capi-unregister-capictr-notifier-after-init-fai.patch
>  0003-isdn-capi-make-kcapi-use-a-separate-workqueue.patch
>  0004-drm-ttm-use-cancel_delayed_work_sync-in-ttm_bo.patch
>  0005-net-dsa-don-t-use-flush_scheduled_work.patch
>  0006-ncpfs-don-t-use-flush_scheduled_work.patch
>  0007-ocfs2-don-t-use-flush_scheduled_work.patch
>  0008-afs-add-afs_wq-and-use-it-instead-of-the-system-work.patch
>  0009-sound-don-t-use-flush_scheduled_work.patch
>  0010-arm-sharpsl-don-t-use-flush_scheduled_work.patch
>  0011-sh-don-t-use-flush_scheduled_work.patch
>  0012-floppy-don-t-use-flush_scheduled_work.patch
>  0013-gdrom-don-t-use-flush_scheduled_work.patch
>  0014-xen-don-t-use-flush_scheduled_work.patch
>  0015-hvsi-don-t-use-flush_scheduled_work.patch
>  0016-sonypi-don-t-use-flush_scheduled_work.patch
>  0017-tpm-don-t-use-flush_scheduled_work.patch
>  0018-vmwgfx-don-t-use-flush_scheduled_work.patch
>  0019-hid-picolcd-don-t-use-flush_scheduled_work.patch
>  0020-macintosh-ams-don-t-use-flush_scheduled_work.patch
>  0021-pcmcia-ipwireless-don-t-use-flush_scheduled_work.patch
>  0022-mISDN-don-t-use-flush_scheduled_work.patch
>  0023-leds-wm8350-don-t-use-flush_scheduled_work.patch
>  0024-dvb-don-t-use-flush_scheduled_work.patch
>  0025-mfd-update-workqueue-usages.patch
>  0026-mmc-update-workqueue-usages.patch
>  0027-mtd-don-t-use-flush_scheduled_work.patch
>  0028-battery-don-t-use-flush_scheduled_work.patch
>  0029-rtc-don-t-use-flush_scheduled_work.patch
>  0030-s390-don-t-use-flush_scheduled_work.patch

Patches other than 0001 and 0008 are merged into wq#for-next.

Thanks.

-- 
tejun

Re: [PATCH 01/30] infiniband: update workqueue usage

[Roland Dreier]

Thanks Tejun.  A couple questions:

 > * ib_wq is added, which is used as the common workqueue for infiniband
 >   instead of the system workqueue.  All system workqueue usages
 >   including flush_scheduled_work() callers are converted to use and
 >   flush ib_wq.  This is to prepare for deprecation of
 >   flush_scheduled_work().

Why do we want to move to a subsystem-specific workqueue?  Can we just
replace flush_scheduled_work() by cancel_delayed_work_sync() as
appropriate and not create yet another work queue?

 > * qib_wq is removed and ib_wq is used instead.

You obviously looked at the comment

-	/*
-	 * We create our own workqueue mainly because we want to be
-	 * able to flush it when devices are being removed.  We can't
-	 * use schedule_work()/flush_scheduled_work() because both
-	 * unregister_netdev() and linkwatch_event take the rtnl lock,
-	 * so flush_scheduled_work() can deadlock during device
-	 * removal.
-	 */
-	qib_wq = create_workqueue("qib");

and know that with the new workqueue stuff, this issue no longer
exists.  But for both my education and also the clarity of the changelog
for this patch, perhaps you could expand on why ib_wq is safe here.

 > * create[_singlethread]_workqueue() usages are replaced with the new
 >   alloc[_ordered]_workqueue().  This removes rescuers from all
 >   infiniband workqueues.

What are rescuers?

Can we replace some of these driver-specific work queues by the ib_wq?

Are all these things just possibilities for future cleanup?

Thanks,
  Roland

Re: [PATCH 01/30] infiniband: update workqueue usage

[Tejun Heo]

Hello, Roland.  Sorry about the delay.

On 12/15/2010 07:33 PM, Roland Dreier wrote:
> Thanks Tejun.  A couple questions:
> 
>  > * ib_wq is added, which is used as the common workqueue for infiniband
>  >   instead of the system workqueue.  All system workqueue usages
>  >   including flush_scheduled_work() callers are converted to use and
>  >   flush ib_wq.  This is to prepare for deprecation of
>  >   flush_scheduled_work().
> 
> Why do we want to move to a subsystem-specific workqueue?  Can we just
> replace flush_scheduled_work() by cancel_delayed_work_sync() as
> appropriate and not create yet another work queue?

Because there are places where work is used to free the containing
structure.  Before a module is unloaded, all works which uses
functions in the module should be flushed; however, if a work is used
to free the containing structure, such work can't be flushed
explicitly, so the workqueue which processes such works should be
flushed.

So, in this case, ib_wq is added primarily to serve as a flush domain.
For driver midlayers, this seems often necessary.  Also, the workqueue
doesn't have any dedicated worker and is quite cheap.

> 
>  > * qib_wq is removed and ib_wq is used instead.
> 
> You obviously looked at the comment
> 
> -	/*
> -	 * We create our own workqueue mainly because we want to be
> -	 * able to flush it when devices are being removed.  We can't
> -	 * use schedule_work()/flush_scheduled_work() because both
> -	 * unregister_netdev() and linkwatch_event take the rtnl lock,
> -	 * so flush_scheduled_work() can deadlock during device
> -	 * removal.
> -	 */
> -	qib_wq = create_workqueue("qib");
> 
> and know that with the new workqueue stuff, this issue no longer
> exists.  But for both my education and also the clarity of the changelog
> for this patch, perhaps you could expand on why ib_wq is safe here.

I think I got confused.  I thought the comment was indicating the
separation between qib_wq and qib_cq_wq.  It's between system_wq and
qib_wq, right?  I'll drop this part from the series, but then again
what's the difference from ib_srp, which flushes the common workqueue?
Why doesn't ib_srp have the same problem?

>  > * create[_singlethread]_workqueue() usages are replaced with the new
>  >   alloc[_ordered]_workqueue().  This removes rescuers from all
>  >   infiniband workqueues.
> 
> What are rescuers?

Normally, all workqueues share global per-cpu worker pool, but certain
workqueues needs forward progress guarantee under memory pressure (the
ones which are used to free memory).  In this case, the workqueues are
created with WQ_MEM_RECLAIM and has a single rescuer worker reserved.
So, any workqueue which is in memory reclaim path needs to have the
flag set to avoid the unlikely but still possible deadlock under
memory pressure.

> Can we replace some of these driver-specific work queues by the ib_wq?
>
> Are all these things just possibilities for future cleanup?

Hmm... Yeah, sure, they can be.  With the new implementation, separate
workqueues are used for the following purposes.

* As a forward progress guarantee domain as decribed above.

* As a flushing domain.

* As a property domain.  Different workqueues have different execution
  and queueing properties set.

Unless one of the above is necessary, work items can be queued
together into the same workqueue.  Concurrency-wise, it wouldn't make
any difference.  They all use the same set of workers anyway, but I
don't know the code well enough to make the changes myself.  If you're
interested in doing it, I'll be happy to help.

Thanks.

-- 
tejun

Re: [PATCH 01/30] infiniband: update workqueue usage

[David Dillow]

On Thu, 2010-12-16 at 17:50 +0100, Tejun Heo wrote:
> On 12/15/2010 07:33 PM, Roland Dreier wrote:
> > 
> >  > * qib_wq is removed and ib_wq is used instead.
> > 
> > You obviously looked at the comment
> > 
> > -	/*
> > -	 * We create our own workqueue mainly because we want to be
> > -	 * able to flush it when devices are being removed.  We can't
> > -	 * use schedule_work()/flush_scheduled_work() because both
> > -	 * unregister_netdev() and linkwatch_event take the rtnl lock,
> > -	 * so flush_scheduled_work() can deadlock during device
> > -	 * removal.
> > -	 */
> > -	qib_wq = create_workqueue("qib");
> > 
> > and know that with the new workqueue stuff, this issue no longer
> > exists.  But for both my education and also the clarity of the changelog
> > for this patch, perhaps you could expand on why ib_wq is safe here.
> 
> I think I got confused.  I thought the comment was indicating the
> separation between qib_wq and qib_cq_wq.  It's between system_wq and
> qib_wq, right?  I'll drop this part from the series, but then again
> what's the difference from ib_srp, which flushes the common workqueue?
> Why doesn't ib_srp have the same problem?

Looking at qib, I'm not sure the comment isn't confused -- the only
place I see where qib_wq or qib_cq_wq get flushed is by
destroy_workqueue() when the module is being unloaded. And we shouldn't
be there with rtnl_lock held by the caller.

Roland, please let me know how plan to proceed -- I need to update
ib_srp to get rid of *scheduled_work(), and I can either use the IB
core's queue, or define my own. Since it's cheap, I don't suppose it
matters much, but I think I'd prefer to share if possible.

Thanks,
Dave

Re: [PATCH 01/30] infiniband: update workqueue usage

[Roland Dreier]

So I went ahead and applied Tejun's patch, since I think it should be no
worse than what we have now... I agree the qib stuff looks fine.

 - R.

[PATCH] RDMA: update missed converion of flush_scheduled_work()

[Tejun Heo]

Commit f0626710 (d9112f11586830d22501d0e26245ea) introduced ib_wq and
removed the use of flush_scheduled_work(); however, during merge
process one chunk was lost in ib_sa_remove_one().  Fix it.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Roland Dreier <rolandd@cisco.com>
---
 drivers/infiniband/core/sa_query.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: work/drivers/infiniband/core/sa_query.c
===================================================================
--- work.orig/drivers/infiniband/core/sa_query.c
+++ work/drivers/infiniband/core/sa_query.c
@@ -1079,7 +1079,7 @@ static void ib_sa_remove_one(struct ib_d
 
 	ib_unregister_event_handler(&sa_dev->event_handler);
 
-	flush_scheduled_work();
+	flush_workqueue(ib_wq);
 
 	for (i = 0; i <= sa_dev->end_port - sa_dev->start_port; ++i) {
 		if (rdma_port_get_link_layer(device, i + 1) == IB_LINK_LAYER_INFINIBAND) {

[PATCH RFC] RDMA: use alloc[_ordered]_workqueue()

[Tejun Heo]

Replace create[_singlethread]_workqueue() usages with the new
alloc[_ordered]_workqueue().  This removes rescuers from all
infiniband workqueues.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Roland Dreier <rolandd@cisco.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
---
Hello,

So, all the create* -> alloc* conversions were dropped (one
flush_scheduled_work() conversion was incorrectly dropped in the
process, fixed by the other patch).  Was this because of forward
progress under memory pressure guarantee?

It doesn't look like infiniband can be safely used in memory reclaim
path anyway (ie. it doesn't seem safe to host swap space) but I could
definitely be mistaken.  If forward progress is necessary, the right
thing to do is adding WQ_MEM_RECLAIM flag to workqueue allocation
calls.

Thank you.

 drivers/infiniband/core/addr.c            |    2 +-
 drivers/infiniband/core/cm.c              |    2 +-
 drivers/infiniband/core/cma.c             |    2 +-
 drivers/infiniband/core/iwcm.c            |    2 +-
 drivers/infiniband/core/mad.c             |    2 +-
 drivers/infiniband/core/multicast.c       |    2 +-
 drivers/infiniband/hw/cxgb3/iwch_cm.c     |    2 +-
 drivers/infiniband/hw/cxgb4/cm.c          |    2 +-
 drivers/infiniband/hw/mlx4/main.c         |    2 +-
 drivers/infiniband/hw/mthca/mthca_catas.c |    2 +-
 drivers/infiniband/hw/nes/nes_cm.c        |    4 ++--
 drivers/infiniband/ulp/ipoib/ipoib_main.c |    2 +-
 12 files changed, 13 insertions(+), 13 deletions(-)

Index: work/drivers/infiniband/core/addr.c
===================================================================
--- work.orig/drivers/infiniband/core/addr.c
+++ work/drivers/infiniband/core/addr.c
@@ -438,7 +438,7 @@ static struct notifier_block nb = {
 
 static int __init addr_init(void)
 {
-	addr_wq = create_singlethread_workqueue("ib_addr");
+	addr_wq = alloc_ordered_workqueue("ib_addr", 0);
 	if (!addr_wq)
 		return -ENOMEM;
 
Index: work/drivers/infiniband/core/cma.c
===================================================================
--- work.orig/drivers/infiniband/core/cma.c
+++ work/drivers/infiniband/core/cma.c
@@ -3257,7 +3257,7 @@ static int __init cma_init(void)
 {
 	int ret;
 
-	cma_wq = create_singlethread_workqueue("rdma_cm");
+	cma_wq = alloc_ordered_workqueue("rdma_cm", 0);
 	if (!cma_wq)
 		return -ENOMEM;
 
Index: work/drivers/infiniband/core/iwcm.c
===================================================================
--- work.orig/drivers/infiniband/core/iwcm.c
+++ work/drivers/infiniband/core/iwcm.c
@@ -1015,7 +1015,7 @@ EXPORT_SYMBOL(iw_cm_init_qp_attr);
 
 static int __init iw_cm_init(void)
 {
-	iwcm_wq = create_singlethread_workqueue("iw_cm_wq");
+	iwcm_wq = alloc_ordered_workqueue("iw_cm_wq", 0);
 	if (!iwcm_wq)
 		return -ENOMEM;
 
Index: work/drivers/infiniband/core/mad.c
===================================================================
--- work.orig/drivers/infiniband/core/mad.c
+++ work/drivers/infiniband/core/mad.c
@@ -2839,7 +2839,7 @@ static int ib_mad_port_open(struct ib_de
 		goto error7;
 
 	snprintf(name, sizeof name, "ib_mad%d", port_num);
-	port_priv->wq = create_singlethread_workqueue(name);
+	port_priv->wq = alloc_ordered_workqueue(name, 0);
 	if (!port_priv->wq) {
 		ret = -ENOMEM;
 		goto error8;
Index: work/drivers/infiniband/core/multicast.c
===================================================================
--- work.orig/drivers/infiniband/core/multicast.c
+++ work/drivers/infiniband/core/multicast.c
@@ -872,7 +872,7 @@ int mcast_init(void)
 {
 	int ret;
 
-	mcast_wq = create_singlethread_workqueue("ib_mcast");
+	mcast_wq = alloc_ordered_workqueue("ib_mcast", 0);
 	if (!mcast_wq)
 		return -ENOMEM;
 
Index: work/drivers/infiniband/hw/cxgb3/iwch_cm.c
===================================================================
--- work.orig/drivers/infiniband/hw/cxgb3/iwch_cm.c
+++ work/drivers/infiniband/hw/cxgb3/iwch_cm.c
@@ -2236,7 +2236,7 @@ int __init iwch_cm_init(void)
 {
 	skb_queue_head_init(&rxq);
 
-	workq = create_singlethread_workqueue("iw_cxgb3");
+	workq = alloc_ordered_workqueue("iw_cxgb3", 0);
 	if (!workq)
 		return -ENOMEM;
 
Index: work/drivers/infiniband/hw/cxgb4/cm.c
===================================================================
--- work.orig/drivers/infiniband/hw/cxgb4/cm.c
+++ work/drivers/infiniband/hw/cxgb4/cm.c
@@ -2356,7 +2356,7 @@ int __init c4iw_cm_init(void)
 	spin_lock_init(&timeout_lock);
 	skb_queue_head_init(&rxq);
 
-	workq = create_singlethread_workqueue("iw_cxgb4");
+	workq = alloc_ordered_workqueue("iw_cxgb4", 0);
 	if (!workq)
 		return -ENOMEM;
 
Index: work/drivers/infiniband/hw/mlx4/main.c
===================================================================
--- work.orig/drivers/infiniband/hw/mlx4/main.c
+++ work/drivers/infiniband/hw/mlx4/main.c
@@ -1214,7 +1214,7 @@ static int __init mlx4_ib_init(void)
 {
 	int err;
 
-	wq = create_singlethread_workqueue("mlx4_ib");
+	wq = alloc_ordered_workqueue("mlx4_ib", 0);
 	if (!wq)
 		return -ENOMEM;
 
Index: work/drivers/infiniband/hw/mthca/mthca_catas.c
===================================================================
--- work.orig/drivers/infiniband/hw/mthca/mthca_catas.c
+++ work/drivers/infiniband/hw/mthca/mthca_catas.c
@@ -186,7 +186,7 @@ int __init mthca_catas_init(void)
 {
 	INIT_WORK(&catas_work, catas_reset);
 
-	catas_wq = create_singlethread_workqueue("mthca_catas");
+	catas_wq = alloc_ordered_workqueue("mthca_catas", 0);
 	if (!catas_wq)
 		return -ENOMEM;
 
Index: work/drivers/infiniband/hw/nes/nes_cm.c
===================================================================
--- work.orig/drivers/infiniband/hw/nes/nes_cm.c
+++ work/drivers/infiniband/hw/nes/nes_cm.c
@@ -2381,10 +2381,10 @@ static struct nes_cm_core *nes_cm_alloc_
 	nes_debug(NES_DBG_CM, "Init CM Core completed -- cm_core=%p\n", cm_core);
 
 	nes_debug(NES_DBG_CM, "Enable QUEUE EVENTS\n");
-	cm_core->event_wq = create_singlethread_workqueue("nesewq");
+	cm_core->event_wq = alloc_ordered_workqueue("nesewq", 0);
 	cm_core->post_event = nes_cm_post_event;
 	nes_debug(NES_DBG_CM, "Enable QUEUE DISCONNECTS\n");
-	cm_core->disconn_wq = create_singlethread_workqueue("nesdwq");
+	cm_core->disconn_wq = alloc_ordered_workqueue("nesdwq", 0);
 
 	print_core(cm_core);
 	return cm_core;
Index: work/drivers/infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- work.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ work/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -1374,7 +1374,7 @@ static int __init ipoib_init_module(void
 	 * so flush_scheduled_work() can deadlock during device
 	 * removal.
 	 */
-	ipoib_workqueue = create_singlethread_workqueue("ipoib");
+	ipoib_workqueue = alloc_ordered_workqueue("ipoib", 0);
 	if (!ipoib_workqueue) {
 		ret = -ENOMEM;
 		goto err_fs;
Index: work/drivers/infiniband/core/cm.c
===================================================================
--- work.orig/drivers/infiniband/core/cm.c
+++ work/drivers/infiniband/core/cm.c
@@ -3804,7 +3804,7 @@ static int __init ib_cm_init(void)
 	if (ret)
 		return -ENOMEM;
 
-	cm.wq = create_workqueue("ib_cm");
+	cm.wq = alloc_workqueue("ib_cm", 0, 0);
 	if (!cm.wq) {
 		ret = -ENOMEM;
 		goto error1;

Re: [PATCH] RDMA: update missed converion of flush_scheduled_work()

[Roland Dreier]

> Commit f0626710 (d9112f11586830d22501d0e26245ea) introduced ib_wq and
> removed the use of flush_scheduled_work(); however, during merge
> process one chunk was lost in ib_sa_remove_one().  Fix it.

Thanks, applied.

BTW in the future if you can cc the linux-rdma@vger mailing list on RDMA patches
that helps me out, since the patch goes into the linux-rdma patchwork
and I'm much
less likely to lose it in my inbox.

Thanks,
  Roland

Re: [PATCH] RDMA: update missed converion of flush_scheduled_work()

[Tejun Heo]

Hello,

On Fri, Jan 28, 2011 at 04:40:55PM -0800, Roland Dreier wrote:
> > Commit f0626710 (d9112f11586830d22501d0e26245ea) introduced ib_wq and
> > removed the use of flush_scheduled_work(); however, during merge
> > process one chunk was lost in ib_sa_remove_one().  Fix it.
> 
> Thanks, applied.
> 
> BTW in the future if you can cc the linux-rdma@vger mailing list on RDMA patches
> that helps me out, since the patch goes into the linux-rdma patchwork
> and I'm much
> less likely to lose it in my inbox.

Oh, I see.  I'll try to remember that.

Thank you.

-- 
tejun