Re: workqueue thing

[Tejun Heo <tj@kernel.org>]

Hello,

On 12/23/2009 02:47 AM, Peter Zijlstra wrote:
> On Tue, 2009-12-22 at 09:20 -0800, Linus Torvalds wrote:
>>
>> So stop arguing about irrelevancies. Nobody uses workqueues for RT or for 
>> CPU-intensive crap. It's not what they were designed for, or used for.
> 
> RT crap maybe, but cpu intensive bits are used for sure, see the
> crypto/crypto_wq.c drivers/md/dm*.c.
>
> I've seen those consume significant amounts of cpu, now I'm not going to
> argue that workqueues are not the best way to consume lots of cpu, but
> the fact is they _are_ used for that.
>
> And since tejun's thing doesn't have wakeup parallelism covered these
> uses can turn into significant loads.

(cc'ing Herbert Xu and David Miller for crypt)

I wrote this before but if something is burning CPU cycles using MT
workqueues, this can be easily supported by marking such workqueue as
not concurrency-managed.  ie. Works queued for such workqueues
wouldn't contribute to the perceived workqueue concurrency and will be
left to be managed solely by the scheduler.  This will completely
cover the crypto_wq case which uses MT workqueue with local cpu
binding.

I don't think dm is currently using workqueue for checksumming but it
does use kcryptd (a ST workqueue) for dm-crypt.  My understanding of
crypt subsystem is limited but kcryptd simply passes the encryption
request to crypt subsystem which may then again pass the thing to
crypto workers (the MT workqueue described above) or process it
synchronously depending on how the encryption chain is setup.  In the
former case, cmwq with the above described modification wouldn't
change anything.  For the latter case, it would make the worker pinned
to a cpu while encrypting/decrypting but then again kcryptd spending
large amount of cpu cycles is simply wrong to begin with.  There's
only _single_ kcryptd which is shared by all dm-crypt instances.  It
should be issuing works to crypto workers instead of doing
encrypt/decrypt itself.

It seems that cryptd is building an async mechanism for CPU-intensive
tasks on top of workqueue mechanism, which can be well supported by
cmwq (not different from the current situation).  In the long run, the
right thing to do would to build a generic mechanism for this type of
workloads and share it among crypt, dm/btrfs block processing.  But,
at any rate, this doesn't really change anything for cmwq.  With
slight modification, it can support the current mechanisms just as
well.

>> People use workqueues for other things _today_, and they have annoying 
>> problems as they stand. It would be nice to get rid of the deadlock 
>> issue, for example - right now the tty driver literally does crazy things, 
>> and drops locks that it shouldn't drop due to the fact that it needs to 
>> wait for queued work - even if the queued work it is actually waiting for 
>> isn't the one that takes the lock!
> 
> Which in turn would imply we cannot carry fwd the current lockdep
> annotations, right?
>
> Which means we'll be stuck in a situation where A flushes B and B
> flushes A will go undetected until we actually hit it.

Lockdep annotations will keep working where it matters.  Why would it
change at all?

Thanks.

-- 
tejun