linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andres Salomon <dilinger@queued.net>
To: dcbw@redhat.com
Cc: linville@tuxdriver.com, libertas-dev@lists.infradead.org,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, dsd@laptop.org
Subject: [PATCH] libertas: clean up scan thread handling
Date: Mon, 19 Dec 2011 12:22:58 -0800	[thread overview]
Message-ID: <20111219122258.26988450@queued.net> (raw)


The libertas scan thread expects priv->scan_req to be non-NULL.  In theory,
it should always be set.  In practice, we've seen the following oops:

[ 8363.067444] Unable to handle kernel NULL pointer dereference at virtual address 00000004
[ 8363.067490] pgd = c0004000
[ 8363.078393] [00000004] *pgd=00000000
[ 8363.086711] Internal error: Oops: 17 [#1] PREEMPT
[ 8363.091375] Modules linked in: fuse libertas_sdio libertas psmouse mousedev ov7670 mmp_camera joydev videobuf2_core videobuf2_dma_sg videobuf2_memops [last unloaded: scsi_wait_scan]
[ 8363.107490] CPU: 0    Not tainted  (3.0.0-gf7ccc69 #671)
[ 8363.112799] PC is at lbs_scan_worker+0x108/0x5a4 [libertas]
[ 8363.118326] LR is at 0x0
[ 8363.120836] pc : [<bf03a854>]    lr : [<00000000>]    psr: 60000113
[ 8363.120845] sp : ee66bf48  ip : 00000000  fp : 00000000
[ 8363.120845] r10: ee2c2088  r9 : c04e2efc  r8 : eef97005
[ 8363.132231] r7 : eee0716f  r6 : ee2c02c0  r5 : ee2c2088  r4 : eee07160
[ 8363.137419] r3 : 00000000  r2 : a0000113  r1 : 00000001  r0 : eee07160
[ 8363.143896] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[ 8363.157630] Control: 10c5387d  Table: 2e754019  DAC: 00000015
[ 8363.163334] Process kworker/u:1 (pid: 25, stack limit = 0xee66a2f8)

While I've not found a smoking gun, there are two places that raised red flags
for me.  The first is in _internal_start_scan, when we queue up a scan; we
first queue the worker, and then set priv->scan_req.  There's theoretically
a 50mS delay which should be plenty, but doing things that way just seems
racy (and not in the good way).

The second is in the scan worker thread itself.  Depending on the state of
priv->scan_channel, we cancel pending scan runs and then requeue a run in
300mS.  We then send the scan command down to the hardware, sleep, and if
we get scan results for all the desired channels, we set priv->scan_req to
NULL.  However, it that's happened in less than 300mS, what happens with
the pending scan run?

This patch addresses both of those concerns.  With the patch applied, we
have not seen the oops in the past two weeks.

Signed-off-by: Andres Salomon <dilinger@queued.net>
Cc: stable@kernel.org
---
 drivers/net/wireless/libertas/cfg.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
index a7f1ab2..db64ef1 100644
--- a/drivers/net/wireless/libertas/cfg.c
+++ b/drivers/net/wireless/libertas/cfg.c
@@ -728,9 +728,11 @@ static void lbs_scan_worker(struct work_struct *work)
 		le16_to_cpu(scan_cmd->hdr.size),
 		lbs_ret_scan, 0);
 
-	if (priv->scan_channel >= priv->scan_req->n_channels)
+	if (priv->scan_channel >= priv->scan_req->n_channels) {
 		/* Mark scan done */
+		cancel_delayed_work(&priv->scan_work);
 		lbs_scan_done(priv);
+	}
 
 	/* Restart network */
 	if (carrier)
@@ -759,12 +761,12 @@ static void _internal_start_scan(struct lbs_private *priv, bool internal,
 		request->n_ssids, request->n_channels, request->ie_len);
 
 	priv->scan_channel = 0;
-	queue_delayed_work(priv->work_thread, &priv->scan_work,
-		msecs_to_jiffies(50));
-
 	priv->scan_req = request;
 	priv->internal_scan = internal;
 
+	queue_delayed_work(priv->work_thread, &priv->scan_work,
+		msecs_to_jiffies(50));
+
 	lbs_deb_leave(LBS_DEB_CFG80211);
 }
 
-- 
1.7.2.5


                 reply	other threads:[~2011-12-19 20:32 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20111219122258.26988450@queued.net \
    --to=dilinger@queued.net \
    --cc=dcbw@redhat.com \
    --cc=dsd@laptop.org \
    --cc=libertas-dev@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.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).