Re: [PATCH] workqueue: not allow recursion run_workqueue

[Peter Zijlstra]

On Thu, 2009-01-22 at 14:03 +0800, Lai Jiangshan wrote:
> 
> void do_some_cleanup(void)
> {
>         find_all_queued_work_struct_and_mark_it_old();
>         flush_workqueue(workqueue);
>         /* we can destroy old work_struct for we have flushed them */
>         destroy_old_work_structs();
> }
> 
> if work->func() called do_some_cleanup(), it's very probably a bug.

Of course it is, if only because calling flush on the same workqueue is
pretty dumb.

But I'm still not getting it, flush_workqueue() provides the guarantee
that all work enqueued previous to the call will be finished thereafter.

The self-flush stuff you propose to rip out doesn't violate that
guarantee afaict.

Suppose we have a workqueue Q, with pending work W1..Wn.

Suppose W5 will have the nested flush, it will then recursively complete
W6..Wn+i, where i accounts for any concurrent worklet additions.

Therefore it will have completed (at least) those worklets that were
enqueued at the time flush got called.

So, to get back at your changelog.

 1) yes lockdep will complain -- for good reasons, and I'm all for
getting rid of this mis-feature.

 2) I've no clue what you're on about

 3) more mystery.



--
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/