stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk, Alan Stern <stern@rowland.harvard.edu>,
	Seth Forshee <seth.forshee@canonical.com>
Subject: [ 44/72] usb-storage: fix freezing of the scanning thread
Date: Mon, 27 Feb 2012 17:05:13 -0800	[thread overview]
Message-ID: <20120228010433.276663038@linuxfoundation.org> (raw)
In-Reply-To: <20120228010511.GA8453@kroah.com>

3.2-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Alan Stern <stern@rowland.harvard.edu>

commit bb94a406682770a35305daaa241ccdb7cab399de upstream.

This patch (as1521b) fixes the interaction between usb-storage's
scanning thread and the freezer.  The current implementation has a
race: If the device is unplugged shortly after being plugged in and
just as a system sleep begins, the scanning thread may get frozen
before the khubd task.  Khubd won't be able to freeze until the
disconnect processing is complete, and the disconnect processing can't
proceed until the scanning thread finishes, so the sleep transition
will fail.

The implementation in the 3.2 kernel suffers from an additional
problem.  There the scanning thread calls set_freezable_with_signal(),
and the signals sent by the freezer will mess up the thread's I/O
delays, which are all interruptible.

The solution to both problems is the same: Replace the kernel thread
used for scanning with a delayed-work routine on the system freezable
work queue.  Freezable work queues have the nice property that you can
cancel a work item even while the work queue is frozen, and no signals
are needed.

The 3.2 version of this patch solves the problem in Bugzilla #42730.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Acked-by: Seth Forshee <seth.forshee@canonical.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/usb/storage/usb.c |   91 ++++++++++++++++------------------------------
 drivers/usb/storage/usb.h |    7 ++-
 2 files changed, 36 insertions(+), 62 deletions(-)

--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -788,15 +788,19 @@ static void quiesce_and_remove_host(stru
 	struct Scsi_Host *host = us_to_host(us);
 
 	/* If the device is really gone, cut short reset delays */
-	if (us->pusb_dev->state == USB_STATE_NOTATTACHED)
+	if (us->pusb_dev->state == USB_STATE_NOTATTACHED) {
 		set_bit(US_FLIDX_DISCONNECTING, &us->dflags);
+		wake_up(&us->delay_wait);
+	}
 
-	/* Prevent SCSI-scanning (if it hasn't started yet)
-	 * and wait for the SCSI-scanning thread to stop.
+	/* Prevent SCSI scanning (if it hasn't started yet)
+	 * or wait for the SCSI-scanning routine to stop.
 	 */
-	set_bit(US_FLIDX_DONT_SCAN, &us->dflags);
-	wake_up(&us->delay_wait);
-	wait_for_completion(&us->scanning_done);
+	cancel_delayed_work_sync(&us->scan_dwork);
+
+	/* Balance autopm calls if scanning was cancelled */
+	if (test_bit(US_FLIDX_SCAN_PENDING, &us->dflags))
+		usb_autopm_put_interface_no_suspend(us->pusb_intf);
 
 	/* Removing the host will perform an orderly shutdown: caches
 	 * synchronized, disks spun down, etc.
@@ -823,52 +827,28 @@ static void release_everything(struct us
 	scsi_host_put(us_to_host(us));
 }
 
-/* Thread to carry out delayed SCSI-device scanning */
-static int usb_stor_scan_thread(void * __us)
+/* Delayed-work routine to carry out SCSI-device scanning */
+static void usb_stor_scan_dwork(struct work_struct *work)
 {
-	struct us_data *us = (struct us_data *)__us;
+	struct us_data *us = container_of(work, struct us_data,
+			scan_dwork.work);
 	struct device *dev = &us->pusb_intf->dev;
 
-	dev_dbg(dev, "device found\n");
-
-	set_freezable_with_signal();
-	/*
-	 * Wait for the timeout to expire or for a disconnect
-	 *
-	 * We can't freeze in this thread or we risk causing khubd to
-	 * fail to freeze, but we can't be non-freezable either. Nor can
-	 * khubd freeze while waiting for scanning to complete as it may
-	 * hold the device lock, causing a hang when suspending devices.
-	 * So we request a fake signal when freezing and use
-	 * interruptible sleep to kick us out of our wait early when
-	 * freezing happens.
-	 */
-	if (delay_use > 0) {
-		dev_dbg(dev, "waiting for device to settle "
-				"before scanning\n");
-		wait_event_interruptible_timeout(us->delay_wait,
-				test_bit(US_FLIDX_DONT_SCAN, &us->dflags),
-				delay_use * HZ);
-	}
-
-	/* If the device is still connected, perform the scanning */
-	if (!test_bit(US_FLIDX_DONT_SCAN, &us->dflags)) {
-
-		/* For bulk-only devices, determine the max LUN value */
-		if (us->protocol == USB_PR_BULK &&
-				!(us->fflags & US_FL_SINGLE_LUN)) {
-			mutex_lock(&us->dev_mutex);
-			us->max_lun = usb_stor_Bulk_max_lun(us);
-			mutex_unlock(&us->dev_mutex);
-		}
-		scsi_scan_host(us_to_host(us));
-		dev_dbg(dev, "scan complete\n");
+	dev_dbg(dev, "starting scan\n");
 
-		/* Should we unbind if no devices were detected? */
+	/* For bulk-only devices, determine the max LUN value */
+	if (us->protocol == USB_PR_BULK && !(us->fflags & US_FL_SINGLE_LUN)) {
+		mutex_lock(&us->dev_mutex);
+		us->max_lun = usb_stor_Bulk_max_lun(us);
+		mutex_unlock(&us->dev_mutex);
 	}
+	scsi_scan_host(us_to_host(us));
+	dev_dbg(dev, "scan complete\n");
+
+	/* Should we unbind if no devices were detected? */
 
 	usb_autopm_put_interface(us->pusb_intf);
-	complete_and_exit(&us->scanning_done, 0);
+	clear_bit(US_FLIDX_SCAN_PENDING, &us->dflags);
 }
 
 static unsigned int usb_stor_sg_tablesize(struct usb_interface *intf)
@@ -915,7 +895,7 @@ int usb_stor_probe1(struct us_data **pus
 	init_completion(&us->cmnd_ready);
 	init_completion(&(us->notify));
 	init_waitqueue_head(&us->delay_wait);
-	init_completion(&us->scanning_done);
+	INIT_DELAYED_WORK(&us->scan_dwork, usb_stor_scan_dwork);
 
 	/* Associate the us_data structure with the USB device */
 	result = associate_dev(us, intf);
@@ -946,7 +926,6 @@ EXPORT_SYMBOL_GPL(usb_stor_probe1);
 /* Second part of general USB mass-storage probing */
 int usb_stor_probe2(struct us_data *us)
 {
-	struct task_struct *th;
 	int result;
 	struct device *dev = &us->pusb_intf->dev;
 
@@ -987,20 +966,14 @@ int usb_stor_probe2(struct us_data *us)
 		goto BadDevice;
 	}
 
-	/* Start up the thread for delayed SCSI-device scanning */
-	th = kthread_create(usb_stor_scan_thread, us, "usb-stor-scan");
-	if (IS_ERR(th)) {
-		dev_warn(dev,
-				"Unable to start the device-scanning thread\n");
-		complete(&us->scanning_done);
-		quiesce_and_remove_host(us);
-		result = PTR_ERR(th);
-		goto BadDevice;
-	}
-
+	/* Submit the delayed_work for SCSI-device scanning */
 	usb_autopm_get_interface_no_resume(us->pusb_intf);
-	wake_up_process(th);
+	set_bit(US_FLIDX_SCAN_PENDING, &us->dflags);
 
+	if (delay_use > 0)
+		dev_dbg(dev, "waiting for device to settle before scanning\n");
+	queue_delayed_work(system_freezable_wq, &us->scan_dwork,
+			delay_use * HZ);
 	return 0;
 
 	/* We come here if there are any problems */
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -47,6 +47,7 @@
 #include <linux/blkdev.h>
 #include <linux/completion.h>
 #include <linux/mutex.h>
+#include <linux/workqueue.h>
 #include <scsi/scsi_host.h>
 
 struct us_data;
@@ -72,7 +73,7 @@ struct us_unusual_dev {
 #define US_FLIDX_DISCONNECTING	3	/* disconnect in progress   */
 #define US_FLIDX_RESETTING	4	/* device reset in progress */
 #define US_FLIDX_TIMED_OUT	5	/* SCSI midlayer timed out  */
-#define US_FLIDX_DONT_SCAN	6	/* don't scan (disconnect)  */
+#define US_FLIDX_SCAN_PENDING	6	/* scanning not yet done    */
 #define US_FLIDX_REDO_READ10	7	/* redo READ(10) command    */
 #define US_FLIDX_READ10_WORKED	8	/* previous READ(10) succeeded */
 
@@ -147,8 +148,8 @@ struct us_data {
 	/* mutual exclusion and synchronization structures */
 	struct completion	cmnd_ready;	 /* to sleep thread on	    */
 	struct completion	notify;		 /* thread begin/end	    */
-	wait_queue_head_t	delay_wait;	 /* wait during scan, reset */
-	struct completion	scanning_done;	 /* wait for scan thread    */
+	wait_queue_head_t	delay_wait;	 /* wait during reset	    */
+	struct delayed_work	scan_dwork;	 /* for async scanning      */
 
 	/* subdriver information */
 	void			*extra;		 /* Any extra data          */



  parent reply	other threads:[~2012-02-28  1:05 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-28  1:05 [ 00/72] 3.2.9-stable review Greg KH
2012-02-28  1:04 ` [ 01/72] Security: tomoyo: add .gitignore file Greg KH
2012-02-28  1:04 ` [ 02/72] powerpc/perf: power_pmu_start restores incorrect values, breaking frequency events Greg KH
2012-02-28  1:04 ` [ 03/72] ARM: at91: USB AT91 gadget registration for module Greg KH
2012-02-28  1:04 ` [ 04/72] drm/radeon/kms: fix MSI re-arm on rv370+ Greg KH
2012-02-28  1:04 ` [ 05/72] PCI: workaround hard-wired bus number V2 Greg KH
2012-02-28  1:04 ` [ 06/72] mac80211: Fix a rwlock bad magic bug Greg KH
2012-02-28  1:04 ` [ 07/72] ipheth: Add iPhone 4S Greg KH
2012-02-28  1:04 ` [ 08/72] regmap: Fix cache defaults initialization from raw cache defaults Greg KH
2012-02-28  1:04 ` [ 09/72] eCryptfs: Copy up lower inode attrs after setting lower xattr Greg KH
2012-02-28  1:04 ` [ 10/72] S390: correct ktime to tod clock comparator conversion Greg KH
2012-02-28  1:04 ` [ 11/72] vfs: fix d_inode_lookup() dentry ref leak Greg KH
2012-02-28  1:04 ` [ 12/72] ARM: 7326/2: PL330: fix null pointer dereference in pl330_chan_ctrl() Greg KH
2012-02-28  2:29   ` Mans Rullgard
2012-02-28  8:44     ` Russell King
2012-02-28  9:33       ` Javi Merino
2012-02-28 11:36         ` Mans Rullgard
2012-02-28  1:04 ` [ 13/72] ALSA: hda - Fix redundant jack creations for cx5051 Greg KH
2012-02-28  1:04 ` [ 14/72] mmc: core: check for zero length ioctl data Greg KH
2012-02-28  1:04 ` [ 15/72] NFSv4: Fix an Oops in the NFSv4 getacl code Greg KH
2012-02-28  1:04 ` [ 16/72] NFSv4: Ensure we throw out bad delegation stateids on NFS4ERR_BAD_STATEID Greg KH
2012-02-28  1:04 ` [ 17/72] NFSv4: fix server_scope memory leak Greg KH
2012-02-28  1:04 ` [ 18/72] ARM: 7321/1: cache-v7: Disable preemption when reading CCSIDR Greg KH
2012-02-28  1:04 ` [ 19/72] ARM: 7325/1: fix v7 boot with lockdep enabled Greg KH
2012-02-28  1:04 ` [ 20/72] 3c59x: shorten timer period for slave devices Greg KH
2012-02-28  1:04 ` [ 21/72] net: Dont proxy arp respond if iif == rt->dst.dev if private VLAN is disabled Greg KH
2012-02-28  1:04 ` [ 22/72] netpoll: netpoll_poll_dev() should access dev->flags Greg KH
2012-02-28  1:04 ` [ 23/72] net_sched: Bug in netem reordering Greg KH
2012-02-28  1:04 ` [ 24/72] veth: Enforce minimum size of VETH_INFO_PEER Greg KH
2012-02-28  1:04 ` [ 25/72] via-velocity: S3 resume fix Greg KH
2012-02-28  1:04 ` [ 26/72] ipv4: reset flowi parameters on route connect Greg KH
2012-02-28  1:04 ` [ 27/72] tcp_v4_send_reset: binding oif to iif in no sock case Greg KH
2012-02-28  1:04 ` [ 28/72] ipv4: Fix wrong order of ip_rt_get_source() and update iph->daddr Greg KH
2012-02-28  1:04 ` [ 29/72] net: Make qdisc_skb_cb upper size bound explicit Greg KH
2012-02-28  1:04 ` [ 30/72] IPoIB: Stop lying about hard_header_len and use skb->cb to stash LL addresses Greg KH
2012-02-28  1:05 ` [ 31/72] gro: more generic L2 header check Greg KH
2012-02-28  1:05 ` [ 32/72] tcp: allow tcp_sacktag_one() to tag ranges not aligned with skbs Greg KH
2012-02-28  1:05 ` [ 33/72] tcp: fix range tcp_shifted_skb() passes to tcp_sacktag_one() Greg KH
2012-02-28  1:05 ` [ 34/72] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK Greg KH
2012-02-28  1:05 ` [ 35/72] USB: Added Kamstrup VID/PIDs to cp210x serial driver Greg KH
2012-02-28  1:05 ` [ 36/72] USB: option: cleanup zte 3g-dongles pid in option.c Greg KH
2012-02-28  1:05 ` [ 37/72] USB: Serial: ti_usb_3410_5052: Add Abbot Diabetes Care cable id Greg KH
2012-02-28  1:05 ` [ 38/72] USB: Remove duplicate USB 3.0 hub feature #defines Greg KH
2012-02-28  1:05 ` [ 39/72] USB: Fix handoff when BIOS disables host PCI device Greg KH
2012-02-28  1:05 ` [ 40/72] xhci: Fix oops caused by more USB2 ports than USB3 ports Greg KH
2012-02-28  1:05 ` [ 41/72] xhci: Fix encoding for HS bulk/control NAK rate Greg KH
2012-02-28  1:05 ` [ 42/72] USB: Dont fail USB3 probe on missing legacy PCI IRQ Greg KH
2012-02-28  1:05 ` [ 43/72] USB: Set hub depth after USB3 hub reset Greg KH
2012-02-28  1:05 ` Greg KH [this message]
2012-02-28  1:05 ` [ 45/72] target: Allow control CDBs with data > 1 page Greg KH
2012-02-28  1:05 ` [ 46/72] ASoC: wm8962: Fix sidetone enumeration texts Greg KH
2012-02-28  1:05 ` [ 47/72] ALSA: hda/realtek - Fix overflow of vol/sw check bitmap Greg KH
2012-02-28  1:05 ` [ 48/72] ALSA: hda/realtek - Fix surround output regression on Acer Aspire 5935 Greg KH
2012-02-28  1:05 ` [ 49/72] NOMMU: Lock i_mmap_mutex for access to the VMA prio list Greg KH
2012-02-28  1:05 ` [ 50/72] hwmon: (max6639) Fix FAN_FROM_REG calculation Greg KH
2012-02-28  1:05 ` [ 51/72] hwmon: (max6639) Fix PPR register initialization to set both channels Greg KH
2012-02-28  1:05 ` [ 52/72] hwmon: (ads1015) Fix file leak in probe function Greg KH
2012-02-28  1:05 ` [ 53/72] ARM: omap: fix oops in drivers/video/omap2/dss/dpi.c Greg KH
2012-02-28  1:05 ` [ 54/72] ARM: omap: fix oops in arch/arm/mach-omap2/vp.c when pmic is not found Greg KH
2012-02-28  1:05 ` [ 55/72] x86/amd: Fix L1i and L2 cache sharing information for AMD family 15h processors Greg KH
2012-02-28  1:05 ` [ 56/72] ath9k: stop on rates with idx -1 in ath9k rate controls .tx_status Greg KH
2012-02-28  1:05 ` [ 57/72] genirq: Unmask oneshot irqs when thread was not woken Greg KH
2012-03-04 21:06   ` Sven Joachim
2012-03-04 21:53     ` Jonathan Nieder
2012-03-04 22:08       ` Sven Joachim
2012-03-05  0:43     ` Stefan Lippers-Hollmann
2012-03-06  0:34       ` Linus Torvalds
2012-03-06  8:28         ` Thomas Gleixner
2012-03-06  9:52           ` Thomas Gleixner
2012-03-06 19:31             ` Thomas Gleixner
2012-03-06 19:53               ` Sven Joachim
2012-03-06 20:26                 ` Thomas Gleixner
2012-03-06 20:54                   ` Thomas Gleixner
2012-03-06 21:07                   ` Sven Joachim
2012-03-06 21:11                     ` Thomas Gleixner
2012-03-06 21:40                       ` Linus Torvalds
2012-03-06 21:08                   ` Stefan Lippers-Hollmann
2012-03-06 21:40                   ` Linus Torvalds
2012-03-06 21:47                     ` Linus Torvalds
2012-03-06 22:18                     ` Thomas Gleixner
2012-03-06 22:33                       ` Linus Torvalds
2012-03-06 23:38                         ` Stefan Lippers-Hollmann
2012-03-07  5:36                         ` Sven Joachim
2012-03-06 20:25               ` Stefan Lippers-Hollmann
2012-03-06 19:45       ` Thomas Gleixner
2012-03-06 20:10         ` Sven Joachim
2012-02-28  1:05 ` [ 58/72] genirq: Handle pending irqs in irq_startup() Greg KH
2012-02-28  1:05 ` [ 59/72] [SCSI] scsi_scan: Fix Poison overwritten warning caused by using freed shost Greg KH
2012-02-28  1:05 ` [ 60/72] [SCSI] scsi_pm: Fix bug in the SCSI power management handler Greg KH
2012-02-28  1:05 ` [ 61/72] ipvs: fix matching of fwmark templates during scheduling Greg KH
2012-02-28  1:05 ` [ 62/72] jme: Fix FIFO flush issue Greg KH
2012-02-28  1:05 ` [ 63/72] davinci_emac: Do not free all rx dma descriptors during init Greg KH
2012-02-28  1:05 ` [ 64/72] builddeb: Dont create files in /tmp with predictable names Greg KH
2012-02-28  1:05 ` [ 65/72] can: sja1000: fix isr hang when hw is unplugged under load Greg KH
2012-02-28  1:05 ` [ 66/72] [media] hdpvr: fix race conditon during start of streaming Greg KH
2012-02-28  1:05 ` [ 67/72] [media] imon: dont wedge hardware after early callbacks Greg KH
2012-02-28  1:05 ` [ 68/72] hwmon: (f75375s) Fix register write order when setting fans to full speed Greg KH
2012-02-28  1:05 ` [ 69/72] epoll: introduce POLLFREE to flush ->signalfd_wqh before kfree() Greg KH
2012-02-28  1:05 ` [ 70/72] epoll: ep_unregister_pollwait() can use the freed pwq->whead Greg KH
2012-02-28  1:05 ` [ 71/72] epoll: limit paths Greg KH
2012-02-28  1:05 ` [ 72/72] cdrom: use copy_to_user() without the underscores Greg KH

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=20120228010433.276663038@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seth.forshee@canonical.com \
    --cc=stable@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=torvalds@linux-foundation.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).