Hi,

On Fri 2016-06-24 11:54:47, Tejun Heo wrote:

On Fri, Jun 24, 2016 at 09:05:15AM +0200, Peter Zijlstra wrote:

quoted

quoted

Given how rare that is 

Could you then not remove/rework these few cases for workqueue as well
and make that 'better' too?

Usage of draining is rare for workqueue but that still means several
legitimate users.  With draining there, it's logical to use it during
shutdown.  I don't think it makes sense to change it on workqueue
side.

quoted

quoted

and the extra
complexity of identifying self-requeueing cases, let's forget about
draining and on destruction clear the worker pointer to block further
queueing and then flush whatever is in flight.

You're talking about regular workqueues here?

No, kthread worker.  It's unlikely that kthread worker is gonna need
chained draining especially given that most of its usages are gonna be
conversions from raw kthread usages.  We won't lose much if anything
by just ignoring draining and making the code simpler.

OK, so you suggest to do the following:

  1. Add a flag into struct kthread_worker that will prevent
     from further queuing.

  2. kthread_create_worker()/kthread_destroy_worker() will
     not longer dynamically allocate struct kthread_worker.
     They will just start/stop the kthread.


The result will be:

  a. User will not need the strict synchronization between
     the queue and create/destroy operations.

  b. We could get rid of drain_kthread_worker() because
     flush_kthread_worker() will be enough.


IMHO, the 1st change does not make sense without the 2nd one.
Otherwise, users could do an out-of-memory access when testing
the freed kthread_worker flag.

Do I get this correctly please?

Best Regards,
Petr