All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 27/30] mtd: don't use flush_scheduled_work()
       [not found] <1292086307-19211-1-git-send-email-tj@kernel.org>
@ 2010-12-11 16:51 ` Tejun Heo
  2010-12-14 16:53     ` Artem Bityutskiy
       [not found] ` <1292086307-19211-8-git-send-email-tj@kernel.org>
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Tejun Heo @ 2010-12-11 16:51 UTC (permalink / raw)
  To: linux-kernel; +Cc: Tejun Heo, linux-mtd, David Woodhouse

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.

Thank you.

 drivers/mtd/mtdoops.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 1ee72f3..8b10273 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -396,7 +396,8 @@ static void mtdoops_notify_remove(struct mtd_info *mtd)
 		printk(KERN_WARNING "mtdoops: could not unregister kmsg_dumper\n");
 
 	cxt->mtd = NULL;
-	flush_scheduled_work();
+	flush_work_sync(&cxt->work_erase);
+	flush_work_sync(&cxt->work_write);
 }
 
 
-- 
1.7.1

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

* Re: [PATCH 07/30] ocfs2: don't use flush_scheduled_work()
       [not found] ` <1292086307-19211-8-git-send-email-tj@kernel.org>
@ 2010-12-11 21:33   ` Joel Becker
  0 siblings, 0 replies; 34+ messages in thread
From: Joel Becker @ 2010-12-11 21:33 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, Mark Fasheh

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

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
       [not found] ` <1292086307-19211-10-git-send-email-tj@kernel.org>
@ 2010-12-12  8:56   ` Takashi Iwai
  2010-12-12  9:15     ` Tejun Heo
  0 siblings, 1 reply; 34+ messages in thread
From: Takashi Iwai @ 2010-12-12  8:56 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, Jaroslav Kysela, Liam Girdwood, Mark Brown

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

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-12  8:56   ` [PATCH 09/30] sound: " Takashi Iwai
@ 2010-12-12  9:15     ` Tejun Heo
  2010-12-12 12:38       ` Takashi Iwai
  0 siblings, 1 reply; 34+ messages in thread
From: Tejun Heo @ 2010-12-12  9:15 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: linux-kernel, Jaroslav Kysela, Liam Girdwood, Mark Brown

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

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-12  9:15     ` Tejun Heo
@ 2010-12-12 12:38       ` Takashi Iwai
  2010-12-12 12:40         ` Mark Brown
  0 siblings, 1 reply; 34+ messages in thread
From: Takashi Iwai @ 2010-12-12 12:38 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, Jaroslav Kysela, Liam Girdwood, Mark Brown

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

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-12 12:38       ` Takashi Iwai
@ 2010-12-12 12:40         ` Mark Brown
  2010-12-12 16:02           ` Tejun Heo
  2010-12-13  8:34           ` Takashi Iwai
  0 siblings, 2 replies; 34+ messages in thread
From: Mark Brown @ 2010-12-12 12:40 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Tejun Heo, linux-kernel, Jaroslav Kysela, Liam Girdwood

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

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

* Re: [PATCH 20/30] macintosh/ams: don't use flush_scheduled_work()
       [not found]   ` <20101211201931.00414518@endymion.delvare>
@ 2010-12-12 13:37     ` Michael Hanselmann
  2010-12-13 10:16       ` Stelian Pop
  0 siblings, 1 reply; 34+ messages in thread
From: Michael Hanselmann @ 2010-12-12 13:37 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Tejun Heo, linux-kernel, Stelian Pop

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

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-12 12:40         ` Mark Brown
@ 2010-12-12 16:02           ` Tejun Heo
  2010-12-12 18:47             ` Mark Brown
  2010-12-13  8:34           ` Takashi Iwai
  1 sibling, 1 reply; 34+ messages in thread
From: Tejun Heo @ 2010-12-12 16:02 UTC (permalink / raw)
  To: Mark Brown; +Cc: Takashi Iwai, linux-kernel, Jaroslav Kysela, Liam Girdwood

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

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-12 16:02           ` Tejun Heo
@ 2010-12-12 18:47             ` Mark Brown
  2010-12-12 18:50               ` Tejun Heo
  0 siblings, 1 reply; 34+ messages in thread
From: Mark Brown @ 2010-12-12 18:47 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Takashi Iwai, linux-kernel, Jaroslav Kysela, Liam Girdwood

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.

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-12 18:47             ` Mark Brown
@ 2010-12-12 18:50               ` Tejun Heo
  2010-12-12 19:00                 ` Mark Brown
  0 siblings, 1 reply; 34+ messages in thread
From: Tejun Heo @ 2010-12-12 18:50 UTC (permalink / raw)
  To: Mark Brown; +Cc: Takashi Iwai, linux-kernel, Jaroslav Kysela, Liam Girdwood

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

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-12 18:50               ` Tejun Heo
@ 2010-12-12 19:00                 ` Mark Brown
  2010-12-13 13:32                   ` Liam Girdwood
  0 siblings, 1 reply; 34+ messages in thread
From: Mark Brown @ 2010-12-12 19:00 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Takashi Iwai, linux-kernel, Jaroslav Kysela, Liam Girdwood

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.

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-12 12:40         ` Mark Brown
  2010-12-12 16:02           ` Tejun Heo
@ 2010-12-13  8:34           ` Takashi Iwai
  2010-12-13 13:29             ` Liam Girdwood
  2010-12-13 16:12             ` Mark Brown
  1 sibling, 2 replies; 34+ messages in thread
From: Takashi Iwai @ 2010-12-13  8:34 UTC (permalink / raw)
  To: Mark Brown; +Cc: Tejun Heo, linux-kernel, Jaroslav Kysela, Liam Girdwood

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

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

* Re: [PATCH 21/30] pcmcia/ipwireless: don't use flush_scheduled_work()
       [not found] ` <1292086307-19211-22-git-send-email-tj@kernel.org>
@ 2010-12-13  8:44   ` David Sterba
  0 siblings, 0 replies; 34+ messages in thread
From: David Sterba @ 2010-12-13  8:44 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, Jiri Kosina

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

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

* Re: [PATCH 20/30] macintosh/ams: don't use flush_scheduled_work()
  2010-12-12 13:37     ` [PATCH 20/30] macintosh/ams: " Michael Hanselmann
@ 2010-12-13 10:16       ` Stelian Pop
  2010-12-13 10:18         ` Jean Delvare
  0 siblings, 1 reply; 34+ messages in thread
From: Stelian Pop @ 2010-12-13 10:16 UTC (permalink / raw)
  To: Michael Hanselmann; +Cc: Jean Delvare, Tejun Heo, linux-kernel

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>

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

* Re: [PATCH 20/30] macintosh/ams: don't use flush_scheduled_work()
  2010-12-13 10:16       ` Stelian Pop
@ 2010-12-13 10:18         ` Jean Delvare
  2010-12-13 10:40           ` [PATCH] Remove myself from MAINTAINERS as I no longer have the hardware to test Stelian Pop
  0 siblings, 1 reply; 34+ messages in thread
From: Jean Delvare @ 2010-12-13 10:18 UTC (permalink / raw)
  To: Stelian Pop; +Cc: Michael Hanselmann, Tejun Heo, linux-kernel

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

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

* [PATCH] Remove myself from MAINTAINERS as I no longer have the hardware to test.
  2010-12-13 10:18         ` Jean Delvare
@ 2010-12-13 10:40           ` Stelian Pop
  2010-12-16  9:58             ` Jean Delvare
  0 siblings, 1 reply; 34+ messages in thread
From: Stelian Pop @ 2010-12-13 10:40 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Michael Hanselmann, Tejun Heo, linux-kernel

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

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

* Re: [PATCH 05/30] net/dsa: don't use flush_scheduled_work()
       [not found] ` <1292086307-19211-6-git-send-email-tj@kernel.org>
@ 2010-12-13 11:21   ` Lennert Buytenhek
  0 siblings, 0 replies; 34+ messages in thread
From: Lennert Buytenhek @ 2010-12-13 11:21 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel

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>

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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-13  8:34           ` Takashi Iwai
@ 2010-12-13 13:29             ` Liam Girdwood
  2010-12-13 16:12             ` Mark Brown
  1 sibling, 0 replies; 34+ messages in thread
From: Liam Girdwood @ 2010-12-13 13:29 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Mark Brown, Tejun Heo, linux-kernel, Jaroslav Kysela

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


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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-12 19:00                 ` Mark Brown
@ 2010-12-13 13:32                   ` Liam Girdwood
  2010-12-13 16:36                     ` Olaya, Margarita
  0 siblings, 1 reply; 34+ messages in thread
From: Liam Girdwood @ 2010-12-13 13:32 UTC (permalink / raw)
  To: Mark Brown, Olaya, Margarita
  Cc: Tejun Heo, Takashi Iwai, linux-kernel, Jaroslav Kysela

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


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

* Re: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-13  8:34           ` Takashi Iwai
  2010-12-13 13:29             ` Liam Girdwood
@ 2010-12-13 16:12             ` Mark Brown
  1 sibling, 0 replies; 34+ messages in thread
From: Mark Brown @ 2010-12-13 16:12 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Tejun Heo, linux-kernel, Jaroslav Kysela, Liam Girdwood

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.

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

* RE: [PATCH 09/30] sound: don't use flush_scheduled_work()
  2010-12-13 13:32                   ` Liam Girdwood
@ 2010-12-13 16:36                     ` Olaya, Margarita
  0 siblings, 0 replies; 34+ messages in thread
From: Olaya, Margarita @ 2010-12-13 16:36 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown
  Cc: Tejun Heo, Takashi Iwai, linux-kernel, Jaroslav Kysela

[-- 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¥

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

* Re: [PATCH 27/30] mtd: don't use flush_scheduled_work()
  2010-12-11 16:51 ` [PATCH 27/30] mtd: don't use flush_scheduled_work() Tejun Heo
@ 2010-12-14 16:53     ` Artem Bityutskiy
  0 siblings, 0 replies; 34+ messages in thread
From: Artem Bityutskiy @ 2010-12-14 16:53 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, linux-mtd, David Woodhouse

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 (Артём Битюцкий)


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

* Re: [PATCH 27/30] mtd: don't use flush_scheduled_work()
@ 2010-12-14 16:53     ` Artem Bityutskiy
  0 siblings, 0 replies; 34+ messages in thread
From: Artem Bityutskiy @ 2010-12-14 16:53 UTC (permalink / raw)
  To: Tejun Heo; +Cc: David Woodhouse, linux-mtd, linux-kernel

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 (Артём Битюцкий)

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

* Re: [PATCH 01/30] infiniband: update workqueue usage
       [not found] ` <1292086307-19211-2-git-send-email-tj@kernel.org>
@ 2010-12-15 18:33   ` Roland Dreier
  2010-12-16 16:50     ` Tejun Heo
  2011-01-17  5:21   ` Roland Dreier
  1 sibling, 1 reply; 34+ messages in thread
From: Roland Dreier @ 2010-12-15 18:33 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, Roland Dreier, Sean Hefty, Hal Rosenstock

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

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

* Re: [PATCH] Remove myself from MAINTAINERS as I no longer have the hardware to test.
  2010-12-13 10:40           ` [PATCH] Remove myself from MAINTAINERS as I no longer have the hardware to test Stelian Pop
@ 2010-12-16  9:58             ` Jean Delvare
  0 siblings, 0 replies; 34+ messages in thread
From: Jean Delvare @ 2010-12-16  9:58 UTC (permalink / raw)
  To: Stelian Pop; +Cc: Michael Hanselmann, Tejun Heo, linux-kernel

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

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

* Re: [PATCH 01/30] infiniband: update workqueue usage
  2010-12-15 18:33   ` [PATCH 01/30] infiniband: update workqueue usage Roland Dreier
@ 2010-12-16 16:50     ` Tejun Heo
  2010-12-23 21:47       ` David Dillow
  0 siblings, 1 reply; 34+ messages in thread
From: Tejun Heo @ 2010-12-16 16:50 UTC (permalink / raw)
  To: Roland Dreier; +Cc: linux-kernel, Roland Dreier, Sean Hefty, Hal Rosenstock

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

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

* Re: [PATCH 28/30] battery: don't use flush_scheduled_work()
       [not found] ` <1292086307-19211-29-git-send-email-tj@kernel.org>
@ 2010-12-22  0:07   ` Anton Vorontsov
  0 siblings, 0 replies; 34+ messages in thread
From: Anton Vorontsov @ 2010-12-22  0:07 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel

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

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

* Re: [PATCH 01/30] infiniband: update workqueue usage
  2010-12-16 16:50     ` Tejun Heo
@ 2010-12-23 21:47       ` David Dillow
  0 siblings, 0 replies; 34+ messages in thread
From: David Dillow @ 2010-12-23 21:47 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Roland Dreier, linux-kernel, Roland Dreier, Sean Hefty, Hal Rosenstock

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



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

* Re: [PATCHSET] workqueue: remove flush_scheduled_work() usage
       [not found] <1292086307-19211-1-git-send-email-tj@kernel.org>
                   ` (6 preceding siblings ...)
       [not found] ` <1292086307-19211-29-git-send-email-tj@kernel.org>
@ 2010-12-24 15:02 ` Tejun Heo
       [not found] ` <1292086307-19211-2-git-send-email-tj@kernel.org>
  8 siblings, 0 replies; 34+ messages in thread
From: Tejun Heo @ 2010-12-24 15:02 UTC (permalink / raw)
  To: linux-kernel; +Cc: David Howells, Roland Dreier, Sean Hefty, Hal Rosenstock

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

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

* Re: [PATCH 01/30] infiniband: update workqueue usage
       [not found] ` <1292086307-19211-2-git-send-email-tj@kernel.org>
  2010-12-15 18:33   ` [PATCH 01/30] infiniband: update workqueue usage Roland Dreier
@ 2011-01-17  5:21   ` Roland Dreier
  2011-01-24 11:06     ` [PATCH] RDMA: update missed converion of flush_scheduled_work() Tejun Heo
  1 sibling, 1 reply; 34+ messages in thread
From: Roland Dreier @ 2011-01-17  5:21 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, Roland Dreier, Sean Hefty, Hal Rosenstock

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.

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

* [PATCH] RDMA: update missed converion of flush_scheduled_work()
  2011-01-17  5:21   ` Roland Dreier
@ 2011-01-24 11:06     ` Tejun Heo
  2011-01-24 11:12       ` [PATCH RFC] RDMA: use alloc[_ordered]_workqueue() Tejun Heo
  2011-01-29  0:40       ` [PATCH] RDMA: update missed converion of flush_scheduled_work() Roland Dreier
  0 siblings, 2 replies; 34+ messages in thread
From: Tejun Heo @ 2011-01-24 11:06 UTC (permalink / raw)
  To: Roland Dreier; +Cc: linux-kernel, Roland Dreier, Sean Hefty, Hal Rosenstock

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

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

* [PATCH RFC] RDMA: use alloc[_ordered]_workqueue()
  2011-01-24 11:06     ` [PATCH] RDMA: update missed converion of flush_scheduled_work() Tejun Heo
@ 2011-01-24 11:12       ` Tejun Heo
  2011-01-29  0:40       ` [PATCH] RDMA: update missed converion of flush_scheduled_work() Roland Dreier
  1 sibling, 0 replies; 34+ messages in thread
From: Tejun Heo @ 2011-01-24 11:12 UTC (permalink / raw)
  To: Roland Dreier; +Cc: linux-kernel, Roland Dreier, Sean Hefty, Hal Rosenstock

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;

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

* Re: [PATCH] RDMA: update missed converion of flush_scheduled_work()
  2011-01-24 11:06     ` [PATCH] RDMA: update missed converion of flush_scheduled_work() Tejun Heo
  2011-01-24 11:12       ` [PATCH RFC] RDMA: use alloc[_ordered]_workqueue() Tejun Heo
@ 2011-01-29  0:40       ` Roland Dreier
  2011-01-31 10:36         ` Tejun Heo
  1 sibling, 1 reply; 34+ messages in thread
From: Roland Dreier @ 2011-01-29  0:40 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, Sean Hefty, Hal Rosenstock

> 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

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

* Re: [PATCH] RDMA: update missed converion of flush_scheduled_work()
  2011-01-29  0:40       ` [PATCH] RDMA: update missed converion of flush_scheduled_work() Roland Dreier
@ 2011-01-31 10:36         ` Tejun Heo
  0 siblings, 0 replies; 34+ messages in thread
From: Tejun Heo @ 2011-01-31 10:36 UTC (permalink / raw)
  To: Roland Dreier; +Cc: linux-kernel, Sean Hefty, Hal Rosenstock

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

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

end of thread, other threads:[~2011-01-31 10:37 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1292086307-19211-1-git-send-email-tj@kernel.org>
2010-12-11 16:51 ` [PATCH 27/30] mtd: don't use flush_scheduled_work() Tejun Heo
2010-12-14 16:53   ` Artem Bityutskiy
2010-12-14 16:53     ` Artem Bityutskiy
     [not found] ` <1292086307-19211-8-git-send-email-tj@kernel.org>
2010-12-11 21:33   ` [PATCH 07/30] ocfs2: " Joel Becker
     [not found] ` <1292086307-19211-10-git-send-email-tj@kernel.org>
2010-12-12  8:56   ` [PATCH 09/30] sound: " Takashi Iwai
2010-12-12  9:15     ` Tejun Heo
2010-12-12 12:38       ` Takashi Iwai
2010-12-12 12:40         ` Mark Brown
2010-12-12 16:02           ` Tejun Heo
2010-12-12 18:47             ` Mark Brown
2010-12-12 18:50               ` Tejun Heo
2010-12-12 19:00                 ` Mark Brown
2010-12-13 13:32                   ` Liam Girdwood
2010-12-13 16:36                     ` Olaya, Margarita
2010-12-13  8:34           ` Takashi Iwai
2010-12-13 13:29             ` Liam Girdwood
2010-12-13 16:12             ` Mark Brown
     [not found] ` <1292086307-19211-21-git-send-email-tj@kernel.org>
     [not found]   ` <20101211201931.00414518@endymion.delvare>
2010-12-12 13:37     ` [PATCH 20/30] macintosh/ams: " Michael Hanselmann
2010-12-13 10:16       ` Stelian Pop
2010-12-13 10:18         ` Jean Delvare
2010-12-13 10:40           ` [PATCH] Remove myself from MAINTAINERS as I no longer have the hardware to test Stelian Pop
2010-12-16  9:58             ` Jean Delvare
     [not found] ` <1292086307-19211-22-git-send-email-tj@kernel.org>
2010-12-13  8:44   ` [PATCH 21/30] pcmcia/ipwireless: don't use flush_scheduled_work() David Sterba
     [not found] ` <1292086307-19211-6-git-send-email-tj@kernel.org>
2010-12-13 11:21   ` [PATCH 05/30] net/dsa: " Lennert Buytenhek
     [not found] ` <1292086307-19211-29-git-send-email-tj@kernel.org>
2010-12-22  0:07   ` [PATCH 28/30] battery: " Anton Vorontsov
2010-12-24 15:02 ` [PATCHSET] workqueue: remove flush_scheduled_work() usage Tejun Heo
     [not found] ` <1292086307-19211-2-git-send-email-tj@kernel.org>
2010-12-15 18:33   ` [PATCH 01/30] infiniband: update workqueue usage Roland Dreier
2010-12-16 16:50     ` Tejun Heo
2010-12-23 21:47       ` David Dillow
2011-01-17  5:21   ` Roland Dreier
2011-01-24 11:06     ` [PATCH] RDMA: update missed converion of flush_scheduled_work() Tejun Heo
2011-01-24 11:12       ` [PATCH RFC] RDMA: use alloc[_ordered]_workqueue() Tejun Heo
2011-01-29  0:40       ` [PATCH] RDMA: update missed converion of flush_scheduled_work() Roland Dreier
2011-01-31 10:36         ` Tejun Heo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.