Messages in this thread Patch in this message | | | Date | Wed, 19 Sep 2012 18:13:24 +0800 | From | Lai Jiangshan <> | Subject | Re: [PATCH 2/2] workqueue: Keep activate-order equals to queue_work()-order |
| |
On 09/19/2012 01:08 AM, Tejun Heo wrote: > On Tue, Sep 18, 2012 at 10:05:19AM -0700, Tejun Heo wrote: >> On Tue, Sep 18, 2012 at 04:36:53PM +0800, Lai Jiangshan wrote: >>> The whole workqueue.c keeps activate-order equals to queue_work()-order >>> in any given cwq except workqueue_set_max_active(). >>> >>> If this order is not kept, something may be not good: >>> >>> first_work_fn() { release some resource; } >>> second_work_fn() { wait and request the resource; use resource; } >>> >>> 1. user queues the first work. # ->max_active is low, is queued on ->delayed_works. >>> 2. someone increases the >max_active via workqueue_set_max_active() >>> 3. user queues the second work. # queued on cwq->pool. >>> >>> When the second work is launched to execute, it waits the first work >>> to release the resource. But the first work is still in ->delayed_works, >>> it waits the first work to finish and them it can be activated. >>> >>> It is bad. we fix it by activating the first work in the step 2. >>> >>> I can't fully determine that it is workqueue's responsibility >>> or the user's responsibility. >>> If it is workqueue's responsibility, the patch needs go to -stable. >>> If it is user's responsibility. it is a nice cleanup, it can go to for-next. >>> I prefer it is workqueue's responsibility. >> >> Unless max_active == 1, workqueue doesn't give any guarantee on >> execution order. I don't think we need to care about this. > > That said, I kinda like the patches. Can you please update the > description on the second patch to something along the line of "use > common set_max_active logic which immediately makes use of the newly > increased max_mactive if there are delayed work items and also happens > to keep activation ordering"? > > Thanks. >
Updated. Thanks, Lai
From 314d43f087c85b11a29be0555f32deeb742bf18e Mon Sep 17 00:00:00 2001 From: Lai Jiangshan <laijs@cn.fujitsu.com> Date: Tue, 18 Sep 2012 16:26:30 +0800 Subject: [PATCH] workqueue: use common cwq_set_max_active() for workqueue_set_max_active()
workqueue_set_max_active() may increase ->max_active without activating delayed works. And it may cause the activation order doesn't equal to to queue_work()-order.
To make things consist, we use common cwq_set_max_active() logic which immediately makes use of the newly increased max_mactive if there are delayed work items and also keep activation ordering.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- kernel/workqueue.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c index d0ca063..8783414 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -3458,7 +3458,7 @@ void workqueue_set_max_active(struct workqueue_struct *wq, int max_active) if (!(wq->flags & WQ_FREEZABLE) || !(gcwq->flags & GCWQ_FREEZING)) - get_cwq(gcwq->cpu, wq)->max_active = max_active; + cwq_set_max_active(get_cwq(gcwq->cpu, wq), max_active); spin_unlock_irq(&gcwq->lock); } -- 1.7.4.4
| |