From patchwork Wed Jan 6 23:05:12 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 42364 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 593EBB6EF0 for ; Thu, 7 Jan 2010 10:06:53 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932848Ab0AFXGW (ORCPT ); Wed, 6 Jan 2010 18:06:22 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S932789Ab0AFXGU (ORCPT ); Wed, 6 Jan 2010 18:06:20 -0500 Received: from mga09.intel.com ([134.134.136.24]:51734 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933023Ab0AFXGP (ORCPT ); Wed, 6 Jan 2010 18:06:15 -0500 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 06 Jan 2010 15:05:54 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.49,232,1262592000"; d="scan'208";a="584944625" Received: from unknown (HELO localhost.localdomain) ([10.23.209.129]) by orsmga001.jf.intel.com with ESMTP; 06 Jan 2010 15:05:47 -0800 From: John Fastabend Subject: [RFC PATCH] workqueue.c: should schedule work serialize work->func To: netdev@vger.kernel.org, jesse.brandeburg@ingel.com, tj@kernel.org, tglx@linuxtronix.de Date: Wed, 06 Jan 2010 23:05:12 +0000 Message-ID: <20100106230512.17900.47310.stgit@localhost.localdomain> User-Agent: StGIT/0.14.2 MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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);