From: John Fastabend <john.r.fastabend@intel.com>
To: netdev@vger.kernel.org, jesse.brandeburg@ingel.com,
tj@kernel.org, tglx@linuxtronix.de
Subject: [RFC PATCH] workqueue.c: should schedule work serialize work->func
Date: Wed, 06 Jan 2010 23:05:12 +0000 [thread overview]
Message-ID: <20100106230512.17900.47310.stgit@localhost.localdomain> (raw)
codepath
schedule_work(); ... ; run_workqueue() -> work_clear_pending() -> f(work)
Although schedule_work() will only schedule the task if it is not already
on the work queue the WORK_STRUCT_PENDING bits are cleared just before
calling the work function. If work is scheduled after this occurs and
before f(work) completes it is possible to have multiple calls into f().
Should the kernel protect us from this? With something like the patch
below.
Thanks
---
include/linux/workqueue.h | 3 ++-
kernel/workqueue.c | 3 +++
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 9466e86..fa6ffea 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -26,7 +26,8 @@ struct work_struct {
atomic_long_t data;
#define WORK_STRUCT_PENDING 0 /* T if work item pending execution */
#define WORK_STRUCT_STATIC 1 /* static initializer (debugobjects) */
-#define WORK_STRUCT_FLAG_MASK (3UL)
+#define WORK_STRUCT_RUNNING 2
+#define WORK_STRUCT_FLAG_MASK (7UL) /* 0111 */
#define WORK_STRUCT_WQ_DATA_MASK (~WORK_STRUCT_FLAG_MASK)
struct list_head entry;
work_func_t func;
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index dee4865..b867125 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -397,10 +397,13 @@ static void run_workqueue(struct cpu_workqueue_struct *cwq)
spin_unlock_irq(&cwq->lock);
BUG_ON(get_wq_data(work) != cwq);
+ while (test_and_set_bit(WORK_STRUCT_RUNNING, work_data_bits(work)))
+ cpu_relax();
work_clear_pending(work);
lock_map_acquire(&cwq->wq->lockdep_map);
lock_map_acquire(&lockdep_map);
f(work);
+ clear_bit(WORK_STRUCT_RUNNING, work_data_bits(work));
lock_map_release(&lockdep_map);
lock_map_release(&cwq->wq->lockdep_map);
next reply other threads:[~2010-01-06 23:06 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-06 23:05 John Fastabend [this message]
2010-01-07 0:42 ` [RFC PATCH] workqueue.c: should schedule work serialize work->func 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=20100106230512.17900.47310.stgit@localhost.localdomain \
--to=john.r.fastabend@intel.com \
--cc=jesse.brandeburg@ingel.com \
--cc=netdev@vger.kernel.org \
--cc=tglx@linuxtronix.de \
--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;
as well as URLs for NNTP newsgroup(s).