From patchwork Fri May 14 21:45:03 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Inaky Perez-Gonzalez X-Patchwork-Id: 52688 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 33617B7DBC for ; Sat, 15 May 2010 07:51:04 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760026Ab0ENVu4 (ORCPT ); Fri, 14 May 2010 17:50:56 -0400 Received: from mga05.intel.com ([192.55.52.89]:16639 "EHLO fmsmga101.fm.intel.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932488Ab0ENVtG (ORCPT ); Fri, 14 May 2010 17:49:06 -0400 Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 14 May 2010 14:46:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.53,233,1272870000"; d="scan'208";a="798591973" Received: from unknown (HELO localhost.localdomain) ([10.255.13.4]) by fmsmga001.fm.intel.com with ESMTP; 14 May 2010 14:48:52 -0700 From: Inaky Perez-Gonzalez To: netdev@vger.kernel.org, wimax@linuxwimax.org Cc: Cindy H Kao Subject: [patch 2.6.35 04/25] wimax/i2400m: fix the race condition for accessing TX queue Date: Fri, 14 May 2010 14:45:03 -0700 Message-Id: X-Mailer: git-send-email 1.6.6.1 In-Reply-To: References: Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Cindy H Kao The race condition happens when the TX queue is accessed by the TX work while the same TX queue is being destroyed because a bus reset is triggered either by debugfs entry or simply by failing waking up the device from WiMAX IDLE mode. This fix is to prevent the TX queue from being accessed by multiple threads Signed-off-by: Cindy H Kao --- drivers/net/wimax/i2400m/i2400m-sdio.h | 5 ++++- drivers/net/wimax/i2400m/sdio-tx.c | 31 ++++++++++++++++++++++++------- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/drivers/net/wimax/i2400m/i2400m-sdio.h b/drivers/net/wimax/i2400m/i2400m-sdio.h index b9c4bed..360d4fb 100644 --- a/drivers/net/wimax/i2400m/i2400m-sdio.h +++ b/drivers/net/wimax/i2400m/i2400m-sdio.h @@ -99,7 +99,10 @@ enum { * * @tx_workqueue: workqeueue used for data TX; we don't use the * system's workqueue as that might cause deadlocks with code in - * the bus-generic driver. + * the bus-generic driver. The read/write operation to the queue + * is protected with spinlock (tx_lock in struct i2400m) to avoid + * the queue being destroyed in the middle of a the queue read/write + * operation. * * @debugfs_dentry: dentry for the SDIO specific debugfs files * diff --git a/drivers/net/wimax/i2400m/sdio-tx.c b/drivers/net/wimax/i2400m/sdio-tx.c index de66d06..412b6a8 100644 --- a/drivers/net/wimax/i2400m/sdio-tx.c +++ b/drivers/net/wimax/i2400m/sdio-tx.c @@ -114,13 +114,17 @@ void i2400ms_bus_tx_kick(struct i2400m *i2400m) { struct i2400ms *i2400ms = container_of(i2400m, struct i2400ms, i2400m); struct device *dev = &i2400ms->func->dev; + unsigned long flags; d_fnstart(3, dev, "(i2400m %p) = void\n", i2400m); /* schedule tx work, this is because tx may block, therefore * it has to run in a thread context. */ - queue_work(i2400ms->tx_workqueue, &i2400ms->tx_worker); + spin_lock_irqsave(&i2400m->tx_lock, flags); + if (i2400ms->tx_workqueue != NULL) + queue_work(i2400ms->tx_workqueue, &i2400ms->tx_worker); + spin_unlock_irqrestore(&i2400m->tx_lock, flags); d_fnend(3, dev, "(i2400m %p) = void\n", i2400m); } @@ -130,27 +134,40 @@ int i2400ms_tx_setup(struct i2400ms *i2400ms) int result; struct device *dev = &i2400ms->func->dev; struct i2400m *i2400m = &i2400ms->i2400m; + struct workqueue_struct *tx_workqueue; + unsigned long flags; d_fnstart(5, dev, "(i2400ms %p)\n", i2400ms); INIT_WORK(&i2400ms->tx_worker, i2400ms_tx_submit); snprintf(i2400ms->tx_wq_name, sizeof(i2400ms->tx_wq_name), "%s-tx", i2400m->wimax_dev.name); - i2400ms->tx_workqueue = + tx_workqueue = create_singlethread_workqueue(i2400ms->tx_wq_name); - if (NULL == i2400ms->tx_workqueue) { + if (tx_workqueue == NULL) { dev_err(dev, "TX: failed to create workqueue\n"); result = -ENOMEM; } else result = 0; + spin_lock_irqsave(&i2400m->tx_lock, flags); + i2400ms->tx_workqueue = tx_workqueue; + spin_unlock_irqrestore(&i2400m->tx_lock, flags); d_fnend(5, dev, "(i2400ms %p) = %d\n", i2400ms, result); return result; } void i2400ms_tx_release(struct i2400ms *i2400ms) { - if (i2400ms->tx_workqueue) { - destroy_workqueue(i2400ms->tx_workqueue); - i2400ms->tx_workqueue = NULL; - } + struct i2400m *i2400m = &i2400ms->i2400m; + struct workqueue_struct *tx_workqueue; + unsigned long flags; + + tx_workqueue = i2400ms->tx_workqueue; + + spin_lock_irqsave(&i2400m->tx_lock, flags); + i2400ms->tx_workqueue = NULL; + spin_unlock_irqrestore(&i2400m->tx_lock, flags); + + if (tx_workqueue) + destroy_workqueue(tx_workqueue); }