public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Lai Jiangshan <laijs@cn.fujitsu.com>
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] workqueue: Keep activate-order equals to queue_work()-order
Date: Wed, 19 Sep 2012 17:57:12 +0800	[thread overview]
Message-ID: <505996F8.1020506@cn.fujitsu.com> (raw)
In-Reply-To: <20120918170519.GB8474@google.com>

On 09/19/2012 01:05 AM, 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.
> 
> 

(disorder of execution is OK for current WQ. because we can launch new worker
to execute the next work if the previous one is waiting something)

But I concern activate-order, not execution order. A non-delayed work
may delay a delayed work for ever, and if a non-delayed work needs something
which will be produced by delayed one, the two work may wait each other.

{
a subsystem queues a work to release resource.
and them
a subsystem queues a work to use the resource.
}
Is this behavior acceptable?

If this is acceptable, and if the first queued work go to ->delayed_works,
and a userspace user issues workqueue_set_max_active() via sysctl
before the second queuing. thus the second queued work will be activated
immediately. since the second work is activated,
the first work can't be activated before the first work finished.
thus the two work may wait each other.


All of this is abnormal thought. and it is not a real problem, merging them
as cleanup is enough.

Thanks,
Lai

  parent reply	other threads:[~2012-09-19  9:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-18  8:36 [PATCH 1/2] workqueue: intruduce cwq_set_max_active() helper for thaw_workqueues() Lai Jiangshan
2012-09-18  8:36 ` [PATCH 2/2] workqueue: Keep activate-order equals to queue_work()-order Lai Jiangshan
2012-09-18 17:05   ` Tejun Heo
2012-09-18 17:08     ` Tejun Heo
2012-09-19 10:13       ` Lai Jiangshan
2012-09-19 17:42         ` Tejun Heo
2012-09-19  9:57     ` Lai Jiangshan [this message]
2012-09-19 17:31       ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=505996F8.1020506@cn.fujitsu.com \
    --to=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox