From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Samu Onkalo <samu.p.onkalo@nokia.com>, Tejun Heo <tj@kernel.org>,
linux-kernel@vger.kernel.org
Subject: [PATCH 1/3] workqueues: change cancel_work_sync() to clear work->data
Date: Wed, 24 Feb 2010 21:20:31 +0100 [thread overview]
Message-ID: <20100224202031.GA26987@redhat.com> (raw)
In short: change cancel_work_sync(work) to mark this work as "never
queued" upon return.
When cancel_work_sync(work) succeeds, we know that this work can't be
queued or running, and since we own WORK_STRUCT_PENDING nobody can change
the bits in work->data under us. This means we can also clear the "cwq"
part along with _PENDING bit lockless before return, unless the work is
queued nobody can assume get_wq_data() is stable even under cwq->lock.
This change can speedup the subsequent cancel/flush requests, and as
Dmitry pointed out this simplifies the usage of work_struct's which
can be queued on different workqueues. Consider this pseudo code from
the input subsystem:
struct workqueue_struct *WQ;
struct work_struct *WORK;
for (;;) {
WQ = create_workqueue();
...
if (condition())
queue_work(WQ, WORK);
...
cancel_work_sync(WORK);
destroy_workqueue(WQ);
}
If condition() returns T and then F, cancel_work_sync() will crash the
kernel because WORK->data still points to the already destroyed workqueue.
With this patch the code like above becomes correct.
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/workqueue.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
--- wq/kernel/workqueue.c~1_CANCEL_CLEAR_WQ 2010-02-24 20:43:32.000000000 +0100
+++ wq/kernel/workqueue.c 2010-02-24 20:55:53.000000000 +0100
@@ -229,6 +229,16 @@ static inline void set_wq_data(struct wo
atomic_long_set(&work->data, new);
}
+/*
+ * Clear WORK_STRUCT_PENDING and the workqueue on which it was queued.
+ */
+static inline void clear_wq_data(struct work_struct *work)
+{
+ unsigned long flags = *work_data_bits(work) &
+ (1UL << WORK_STRUCT_STATIC);
+ atomic_long_set(&work->data, flags);
+}
+
static inline
struct cpu_workqueue_struct *get_wq_data(struct work_struct *work)
{
@@ -671,7 +681,7 @@ static int __cancel_work_timer(struct wo
wait_on_work(work);
} while (unlikely(ret < 0));
- work_clear_pending(work);
+ clear_wq_data(work);
return ret;
}
next reply other threads:[~2010-02-24 20:22 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-24 20:20 Oleg Nesterov [this message]
2010-02-25 3:13 ` [PATCH 1/3] workqueues: change cancel_work_sync() to clear work->data Tejun Heo
2010-02-25 9:53 ` Dmitry Torokhov
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=20100224202031.GA26987@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=dmitry.torokhov@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=samu.p.onkalo@nokia.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.