Re: [PATCH v2.6.36-rc7] init: don't call flush_scheduled_work() fromdo_initcalls()

[Tejun Heo]

Hello,

On 10/22/2010 02:12 AM, Andrew Morton wrote:
>> Index: work/init/main.c
>> ===================================================================
>> --- work.orig/init/main.c
>> +++ work/init/main.c
>> @@ -778,9 +778,6 @@ static void __init do_initcalls(void)
>>
>>  	for (fn = __early_initcall_end; fn < __initcall_end; fn++)
>>  		do_one_initcall(*fn);
>> -
>> -	/* Make sure there is no pending stuff from the initcall sequence */
>> -	flush_scheduled_work();
>>  }
>>
> 
> hm, that predates the initial 2002 BK tree.
> 
> If some initcall function leaves a work scheduled and doesn't flush it
> then free_initmem() can come along and pull the rug out from under its
> feet.  Then you will own both pieces :)

Ah, okay, so that's the rationale behind it.  That at least explains
why it made sense back then.  :-)

I don't think it makes much sense anymore tho for the following
reasons.

* As I wrote in the patch description, there are many other workqueues
  and, after auditing many workqueue users in kernel, my impression is
  that switching to different or dedicated workqueues is done quite
  casually and I've never seen any caller which notes explicit
  dependency on the auto flush behvaior.

* I don't think using workqueue that way is common and we already have
  a specialized mechanism for parallel probing - async - which has
  well defined flushing/ordering behavior during booting.

* System workqueue now can happily host long running works.  We can
  end up sitting there for undetermined amount of time.

> If you really don't like people sending you angry emails then I suppose
> you could add some warning here if a scheduled work is pending, and
> that the scheduled work's callback existed in init.text memory.  Which
> would be a bit of a pain to implement.
> 
> Oh well.  The oops traces will make it fairly clear what happened.

I haven't pushed the patch to Linus yet.  I'll remove it for now and
try to implement something which at least checks the text section of
pending and running works.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/