Re: workqueue thing

[Tejun Heo <tj@kernel.org>]

Hello,

On 12/21/2009 11:26 PM, Peter Zijlstra wrote:
>> I think you're primarily concerned with the scheduler modifications
> 
> No I think the whole wq redesign sucks chunks, because it:
> 
>  1) looses the queue property

Restored with max_active based ST workqueue implementation.

>  2) doesn't deal with cpu heavy tasks/wakeup parallelism

workqueue was never suited for this.  MT workqueues have strong CPU
affinity which doesn't make sense for CPU-heavy workloads.  ST
workqueues can't take advantage of parallelism.  We can easily mark
some workqueues as not contributing to concurrency accounting such
that works queued for such wqs are always considered blocked for MT
CPU-heavy workloads.  But I don't think that's the right use of
workqueues.  For CPU-heavy workloads, let's use something which is
more suited to the job.  Something which can take advantage of
parallelism.

>  3) gets fragile at memory-pressure/reclaim

Shared dynamic pool is going to be affected by memory pressure no
matter how you implement it.  cmwq tries to maintain stable level of
workers and has forward progress guarantee.  If you're gonna do shared
pool, it can't get much better.

>  4) didn't consider the RT needs

Can you be more specific?  What RT needs?  It's pretty difficult to
tell when there's no in-kernel user and any shared worker pool would
have pretty similar properties as cmwq.

> Also, I think that whole move tasks back on online stuff is utter crazy,
> just move then to another cpu and leave them there.

Can you be more specific?  Why is it crazy when moving to online but
!active cpu is necessary anyway?

> Also, I don't think it can properly warn of simple AB-BA flush
> deadlocks, where work A flushes B and B flushes A.

During transformation, I lost per-work lockdep annoation in flush_work
path which used to go through wait_on_work().  It can easily be
restored.  Nothing changed at per-work level.  The same lockdep
annoations will work.

> (I also don't much like the colour coding flush implementation, but I
> haven't spend a lot of time considering alternatives)

When you can come up with much better one, let me know.  Will be happy
to replace the current one.

>> and think that the choose-between-two-masks on migration is ugly.  I
>> agree it's not the prettiest thing in this world but then again it's
>> not a lot of code.  The reason why it looks ugly is because the way
>> migration is implemented and parameter is passed in.  API-wise, I
>> think making kthread_bind() synchronized against cpu onliness should
>> be pretty clean.
> 
> Assuming you only migrate blocked tasks the current kthread_bind()
> should suit your needs -- I recently reworked most of the migration
> logic.

Okay, that requires another thread which would watch whether those
rescuers which need to be migrated are scheduled out and a mechanism
to make sure they stay scheduled out.  After the "migration" thread
makes that sure, it would call kthread_bind() and wake up the migrated
rescuers, right?  That's basically what the scheduler "migration"
mechanism does.  Why implement it again?

> But as it stands I don't think its wise to replace the current workqueue
> implementation with this, esp since there are known heavy CPU users
> using it, nor have you addressed the queueing issue (or is that the
> restoration of the single-queue workqueue?)

The queueing one is addressed and which CPU-heavy users are you
talking about?  Workqueues have always been CPU-affine.  Not much
changes for its users by cmwq.

Thanks.

-- 
tejun