All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Fenkart <afenkart@gmail.com>
To: linux-wireless@vger.kernel.org
Cc: Amitkumar Karwar <akarwar@marvell.com>,
	Avinash Patil <patila@marvell.com>,
	Andreas Fenkart <afenkart@gmail.com>
Subject: [PATCH 2/2] mwifiex: sdio: bug: dead-lock in card reset
Date: Mon, 13 Apr 2015 11:18:24 +0200	[thread overview]
Message-ID: <1428916704-9635-2-git-send-email-afenkart@gmail.com> (raw)
In-Reply-To: <1428916704-9635-1-git-send-email-afenkart@gmail.com>

Card reset is implemented by removing/re-adding the adapter instance.
This is implemented by removing the mmc host, which will then trigger
a cascade of removal callbacks including mwifiex_sdio_remove.
The dead-lock results in the latter function, trying to cancel the work
item executing the mmc host removal. This patch adds a driver level,
not adapter level, work struct to break the dead-lock

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/net/wireless/mwifiex/main.h |  1 -
 drivers/net/wireless/mwifiex/sdio.c | 63 +++++++++++++++++++++++++++++--------
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index f0a6af1..702f348 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -437,7 +437,6 @@ enum rdwr_status {
 
 enum mwifiex_iface_work_flags {
 	MWIFIEX_IFACE_WORK_FW_DUMP,
-	MWIFIEX_IFACE_WORK_CARD_RESET,
 };
 
 struct mwifiex_adapter;
diff --git a/drivers/net/wireless/mwifiex/sdio.c b/drivers/net/wireless/mwifiex/sdio.c
index a70e114..0996b0e 100644
--- a/drivers/net/wireless/mwifiex/sdio.c
+++ b/drivers/net/wireless/mwifiex/sdio.c
@@ -75,6 +75,19 @@ static LIST_HEAD(cards);
 static DEFINE_MUTEX(cards_mutex);
 
 /*
+ * Card removal will remove the interface then re-add it
+ * Hence the work_struct can't be part of the adapter
+ * otherwise there will be deadlock when we remove the
+ * adapter, and cancel the work struct executing the reset
+ */
+static struct
+{
+	struct work_struct work;
+	struct sdio_mmc_card *card;
+} s_card_reset;
+static DEFINE_SEMAPHORE(s_card_reset_sem);
+
+/*
  * SDIO probe.
  *
  * This function probes an mwifiex device and registers it. It allocates
@@ -1964,10 +1977,36 @@ mwifiex_update_mp_end_port(struct mwifiex_adapter *adapter, u16 port)
 		port, card->mp_data_port_mask);
 }
 
-static void mwifiex_sdio_card_reset_work(struct mwifiex_adapter *adapter)
+static void mwifiex_sdio_card_reset_work(struct work_struct *work)
 {
-	struct sdio_mmc_card *card = adapter->card;
-	struct mmc_host *target = card->func->card->host;
+	struct sdio_mmc_card *card = NULL;
+	struct mmc_host *target;
+	struct list_head *cur;
+
+	WARN_ON(work != &s_card_reset.work);
+
+	/* adapter pointer might have been invalidated
+	 * e.g. card removed from slot
+	 */
+	mutex_lock(&cards_mutex);
+	list_for_each_prev(cur, &cards) {
+		card = list_entry(cur, struct sdio_mmc_card, next);
+		if (card == s_card_reset.card)
+			break;
+	}
+	if (!card || card != s_card_reset.card) {
+		pr_err("card was removed, cancel card reset\n");
+		mutex_unlock(&cards_mutex);
+		return;
+	}
+
+	/* card pointer still valid inc ref in host, it's all we need */
+	target = card->func->card->host;
+	mmc_claim_host(target);
+	mutex_unlock(&cards_mutex);
+
+	/* release the work struct, we are done with it */
+	up(&s_card_reset_sem);
 
 	/* The actual reset operation must be run outside of driver thread.
 	 * This is because mmc_remove_host() will cause the device to be
@@ -1976,13 +2015,14 @@ static void mwifiex_sdio_card_reset_work(struct mwifiex_adapter *adapter)
 	 *
 	 * We run it in a totally independent workqueue.
 	 */
-
 	pr_err("Resetting card...\n");
 	mmc_remove_host(target);
 	/* 200ms delay is based on experiment with sdhci controller */
 	mdelay(200);
 	target->rescan_entered = 0; /* rescan non-removable cards */
 	mmc_add_host(target);
+
+	mmc_release_host(target);
 }
 
 /* This function read/write firmware */
@@ -2160,9 +2200,6 @@ static void mwifiex_sdio_work(struct work_struct *work)
 	struct mwifiex_adapter *adapter =
 			container_of(work, struct mwifiex_adapter, iface_work);
 
-	if (test_and_clear_bit(MWIFIEX_IFACE_WORK_CARD_RESET,
-			       &adapter->iface_work_flags))
-		mwifiex_sdio_card_reset_work(adapter);
 	if (test_and_clear_bit(MWIFIEX_IFACE_WORK_FW_DUMP,
 			       &adapter->iface_work_flags))
 		mwifiex_sdio_fw_dump_work(work);
@@ -2171,12 +2208,9 @@ static void mwifiex_sdio_work(struct work_struct *work)
 /* This function resets the card */
 static void mwifiex_sdio_card_reset(struct mwifiex_adapter *adapter)
 {
-	if (test_bit(MWIFIEX_IFACE_WORK_CARD_RESET, &adapter->iface_work_flags))
-		return;
-
-	set_bit(MWIFIEX_IFACE_WORK_CARD_RESET, &adapter->iface_work_flags);
-
-	schedule_work(&adapter->iface_work);
+	down(&s_card_reset_sem);
+	s_card_reset.card = adapter->card;
+	schedule_work(&s_card_reset.work);
 }
 
 /* This function dumps FW information */
@@ -2317,6 +2351,7 @@ static int
 mwifiex_sdio_init_module(void)
 {
 	sema_init(&add_remove_card_sem, 1);
+	INIT_WORK(&s_card_reset.work, mwifiex_sdio_card_reset_work);
 
 	/* Clear the flag in case user removes the card. */
 	user_rmmod = 0;
@@ -2339,6 +2374,8 @@ mwifiex_sdio_cleanup_module(void)
 	if (!down_interruptible(&add_remove_card_sem))
 		up(&add_remove_card_sem);
 
+	cancel_work_sync(&s_card_reset.work);
+
 	/* Set the flag as user is removing this module. */
 	user_rmmod = 1;
 
-- 
2.1.4


  reply	other threads:[~2015-04-13  9:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-13  9:18 [PATCH 1/2] mwifiex: sdio: create global list of adapters Andreas Fenkart
2015-04-13  9:18 ` Andreas Fenkart [this message]
2015-04-13 10:27   ` [PATCH 2/2] mwifiex: sdio: bug: dead-lock in card reset Amitkumar Karwar
2015-04-16  9:02     ` Andreas Fenkart
2015-04-16  9:59       ` Amitkumar Karwar
2015-04-28 16:04     ` Kalle Valo
2015-04-28 21:15       ` Andreas Fenkart
2015-04-28 15:58 ` [PATCH 1/2] mwifiex: sdio: create global list of adapters Kalle Valo

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=1428916704-9635-2-git-send-email-afenkart@gmail.com \
    --to=afenkart@gmail.com \
    --cc=akarwar@marvell.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=patila@marvell.com \
    /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.