dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Milan Broz <mbroz@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: device-mapper development <dm-devel@redhat.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	just.for.lkml@googlemail.com, hch@infradead.org,
	herbert@gondor.hengli.com.au
Subject: [PATCH wq#for-next] workqueue: fix HIGHPRI handling in keep_working()
Date: Mon, 11 Oct 2010 12:09:18 +0200	[thread overview]
Message-ID: <4CB2E24E.3020209@kernel.org> (raw)
In-Reply-To: <4CAF4E93.1040101@kernel.org>

The policy function keep_working() didn't check GCWQ_HIGHPRI_PENDING
and could return %false with highpri work pending.  This could lead to
late execution of a highpri work which was delayed due to @max_active
throttling if other works are actively consuming CPU cycles.

For example, the following could happen.

1. Work W0 which burns CPU cycles.

2. Two works W1 and W2 are queued to a highpri wq w/ @max_active of 1.

3. W1 starts executing and W2 is put to delayed queue.  W0 and W1 are
   both runnable.

4. W1 finishes which puts W2 to pending queue but keep_working()
   incorrectly returns %false and the worker goes to sleep.

5. W0 finishes and W2 starts execution.

With this patch applied, W2 starts execution as soon as W1 finishes.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
This is the workqueue bug I've found while trying to debug the dm/raid
hang.  Although the bug may introduce unexpected delay in scheduling a
highpri work, the delay can only be as long as the combined length of
CPU cycle burns of the already running works.  Given that HIGHPRI is
currently only used by xfs and its usage, I don't think it's likely to
cause an actual issue.  I'll queue it for #for-next.

Thank you.

 kernel/workqueue.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index f77afd9..d355278 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -604,7 +604,9 @@ static bool keep_working(struct global_cwq *gcwq)
 {
 	atomic_t *nr_running = get_gcwq_nr_running(gcwq->cpu);

-	return !list_empty(&gcwq->worklist) && atomic_read(nr_running) <= 1;
+	return !list_empty(&gcwq->worklist) &&
+		(atomic_read(nr_running) <= 1 ||
+		 gcwq->flags & GCWQ_HIGHPRI_PENDING);
 }

 /* Do we need a new worker?  Called from manager. */
-- 
1.7.1

      parent reply	other threads:[~2010-10-11 10:09 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <AANLkTi=LsBNU+O2hqZUcM2nYM_ze6qPq3thwSZBMtY_v@mail.gmail.com>
2010-10-07 19:28 ` Linux 2.6.36-rc7 Tejun Heo
2010-10-07 20:13   ` Milan Broz
2010-10-08 17:02     ` [dm-devel] " Tejun Heo
2010-10-10 11:56       ` Torsten Kaiser
2010-10-11 10:09       ` Tejun Heo [this message]

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=4CB2E24E.3020209@kernel.org \
    --to=tj@kernel.org \
    --cc=dm-devel@redhat.com \
    --cc=hch@infradead.org \
    --cc=herbert@gondor.hengli.com.au \
    --cc=just.for.lkml@googlemail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbroz@redhat.com \
    --cc=torvalds@linux-foundation.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;
as well as URLs for NNTP newsgroup(s).