All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Hutchings <ben@decadent.org.uk>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk,
	Roland Dreier <roland@purestorage.com>,
	Andy Grover <agrover@redhat.com>, Hannes Reinecke <hare@suse.de>,
	Christoph Hellwig <hch@lst.de>,
	Nicholas Bellinger <nab@linux-iscsi.org>
Subject: [ 51/82] iscsi-target: Fix missed wakeup race in TX thread
Date: Wed, 14 Nov 2012 05:40:24 +0000	[thread overview]
Message-ID: <20121114053940.926046549@decadent.org.uk> (raw)
In-Reply-To: <20121114053933.726869752@decadent.org.uk>

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

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

From: Roland Dreier <roland@purestorage.com>

commit d5627acba9ae584cf4928af19f7ddf5f6837de32 upstream.

The sleeping code in iscsi_target_tx_thread() is susceptible to the classic
missed wakeup race:

 - TX thread finishes handle_immediate_queue() and handle_response_queue(),
   thinks both queues are empty.
 - Another thread adds a queue entry and does wake_up_process(), which does
   nothing because the TX thread is still awake.
 - TX thread does schedule_timeout() and sleeps forever.

In practice this can kill an iSCSI connection if for example an initiator
does single-threaded writes and the target misses the wakeup window when
queueing an R2T; in this case the connection will be stuck until the
initiator loses patience and does some task management operation (or kills
the connection entirely).

Fix this by converting to wait_event_interruptible(), which does not
suffer from this sort of race.

Signed-off-by: Roland Dreier <roland@purestorage.com>
Cc: Andy Grover <agrover@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/target/iscsi/iscsi_target.c       |    4 +++-
 drivers/target/iscsi/iscsi_target_core.h  |    1 +
 drivers/target/iscsi/iscsi_target_login.c |    1 +
 drivers/target/iscsi/iscsi_target_util.c  |   22 ++++++++++++++++++++--
 drivers/target/iscsi/iscsi_target_util.h  |    1 +
 5 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index d6ce218..035c2c7 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -3719,7 +3719,9 @@ restart:
 		 */
 		iscsit_thread_check_cpumask(conn, current, 1);
 
-		schedule_timeout_interruptible(MAX_SCHEDULE_TIMEOUT);
+		wait_event_interruptible(conn->queues_wq,
+					 !iscsit_conn_all_queues_empty(conn) ||
+					 ts->status == ISCSI_THREAD_SET_RESET);
 
 		if ((ts->status == ISCSI_THREAD_SET_RESET) ||
 		     signal_pending(current))
diff --git a/drivers/target/iscsi/iscsi_target_core.h b/drivers/target/iscsi/iscsi_target_core.h
index 2ba9f9b..21048db 100644
--- a/drivers/target/iscsi/iscsi_target_core.h
+++ b/drivers/target/iscsi/iscsi_target_core.h
@@ -486,6 +486,7 @@ struct iscsi_tmr_req {
 };
 
 struct iscsi_conn {
+	wait_queue_head_t	queues_wq;
 	/* Authentication Successful for this connection */
 	u8			auth_complete;
 	/* State connection is currently in */
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index cdc8a10..f8dbec0 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -41,6 +41,7 @@
 
 static int iscsi_login_init_conn(struct iscsi_conn *conn)
 {
+	init_waitqueue_head(&conn->queues_wq);
 	INIT_LIST_HEAD(&conn->conn_list);
 	INIT_LIST_HEAD(&conn->conn_cmd_list);
 	INIT_LIST_HEAD(&conn->immed_queue_list);
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index afd98cc..1a91195 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -488,7 +488,7 @@ void iscsit_add_cmd_to_immediate_queue(
 	atomic_set(&conn->check_immediate_queue, 1);
 	spin_unlock_bh(&conn->immed_queue_lock);
 
-	wake_up_process(conn->thread_set->tx_thread);
+	wake_up(&conn->queues_wq);
 }
 
 struct iscsi_queue_req *iscsit_get_cmd_from_immediate_queue(struct iscsi_conn *conn)
@@ -562,7 +562,7 @@ void iscsit_add_cmd_to_response_queue(
 	atomic_inc(&cmd->response_queue_count);
 	spin_unlock_bh(&conn->response_queue_lock);
 
-	wake_up_process(conn->thread_set->tx_thread);
+	wake_up(&conn->queues_wq);
 }
 
 struct iscsi_queue_req *iscsit_get_cmd_from_response_queue(struct iscsi_conn *conn)
@@ -616,6 +616,24 @@ static void iscsit_remove_cmd_from_response_queue(
 	}
 }
 
+bool iscsit_conn_all_queues_empty(struct iscsi_conn *conn)
+{
+	bool empty;
+
+	spin_lock_bh(&conn->immed_queue_lock);
+	empty = list_empty(&conn->immed_queue_list);
+	spin_unlock_bh(&conn->immed_queue_lock);
+
+	if (!empty)
+		return empty;
+
+	spin_lock_bh(&conn->response_queue_lock);
+	empty = list_empty(&conn->response_queue_list);
+	spin_unlock_bh(&conn->response_queue_lock);
+
+	return empty;
+}
+
 void iscsit_free_queue_reqs_for_conn(struct iscsi_conn *conn)
 {
 	struct iscsi_queue_req *qr, *qr_tmp;
diff --git a/drivers/target/iscsi/iscsi_target_util.h b/drivers/target/iscsi/iscsi_target_util.h
index 44054bd..894d0f8 100644
--- a/drivers/target/iscsi/iscsi_target_util.h
+++ b/drivers/target/iscsi/iscsi_target_util.h
@@ -25,6 +25,7 @@ extern struct iscsi_queue_req *iscsit_get_cmd_from_immediate_queue(struct iscsi_
 extern void iscsit_add_cmd_to_response_queue(struct iscsi_cmd *, struct iscsi_conn *, u8);
 extern struct iscsi_queue_req *iscsit_get_cmd_from_response_queue(struct iscsi_conn *);
 extern void iscsit_remove_cmd_from_tx_queues(struct iscsi_cmd *, struct iscsi_conn *);
+extern bool iscsit_conn_all_queues_empty(struct iscsi_conn *);
 extern void iscsit_free_queue_reqs_for_conn(struct iscsi_conn *);
 extern void iscsit_release_cmd(struct iscsi_cmd *);
 extern void iscsit_free_cmd(struct iscsi_cmd *);



  parent reply	other threads:[~2012-11-14  5:50 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-14  5:39 [ 00/82] 3.2.34-stable review Ben Hutchings
2012-11-14  5:39 ` [ 01/82] Bluetooth: Always compile SCO and L2CAP in Bluetooth Core Ben Hutchings
2012-11-14 21:24   ` Gustavo Padovan
2012-11-14 21:30     ` David Miller
2012-11-14 21:38       ` Ben Hutchings
2012-11-15 20:04         ` Ben Hutchings
2012-11-15 20:13           ` David Miller
2012-11-15 21:04             ` Gustavo Padovan
2012-11-15 21:08               ` David Miller
2012-11-15 22:05               ` Joe Perches
2012-11-15 21:07           ` Gustavo Padovan
2012-11-16  2:08             ` Ben Hutchings
2012-11-14  5:39 ` [ 02/82] x86: Remove the ancient and deprecated disable_hlt() and enable_hlt() facility Ben Hutchings
2012-11-14  5:39 ` [ 03/82] drm/nouveau: silence modesetting spam on pre-gf8 chipsets Ben Hutchings
2012-11-14  5:39 ` [ 04/82] drm/nouveau: fix suspend/resume when in headless mode Ben Hutchings
2012-11-14  5:39 ` [ 05/82] drm/nouveau: headless mode by default if pci class != vga display Ben Hutchings
2012-11-14  5:39 ` [ 06/82] nfsd: add get_uint for u32s Ben Hutchings
2012-11-14  5:39 ` [ 07/82] ALSA: PCM: Fix some races at disconnection Ben Hutchings
2012-11-14  5:39 ` [ 08/82] ALSA: usb-audio: Fix " Ben Hutchings
2012-11-14  5:39 ` [ 09/82] ALSA: usb-audio: Use rwsem for disconnect protection Ben Hutchings
2012-11-14  5:39 ` [ 10/82] ALSA: usb-audio: Fix races at disconnection in mixer_quirks.c Ben Hutchings
2012-11-14  5:39 ` [ 11/82] ALSA: Add a reference counter to card instance Ben Hutchings
2012-11-14  5:39 ` [ 12/82] ALSA: Avoid endless sleep after disconnect Ben Hutchings
2012-11-14  5:39 ` [ 13/82] drm/radeon: fix typo in evergreen_mc_resume() Ben Hutchings
2012-11-14  5:39 ` [ 14/82] USB: mos7840: remove unused variable Ben Hutchings
2012-11-14  5:39 ` [ 15/82] rtnetlink: Fix problem with buffer allocation Ben Hutchings
2012-11-14  5:39 ` [ 16/82] rtnetlink: fix rtnl_calcit() and rtnl_dump_ifinfo() Ben Hutchings
2012-11-14  5:39 ` [ 17/82] gpio-timberdale: fix a potential wrapping issue Ben Hutchings
2012-11-14  5:39 ` [ 18/82] cfg80211: fix antenna gain handling Ben Hutchings
2012-11-14  5:39 ` [ 19/82] drm/i915: fix overlay on i830M Ben Hutchings
2012-11-14  5:39 ` [ 20/82] drm/i915: fixup infoframe support for sdvo Ben Hutchings
2012-11-14  5:39 ` [ 21/82] drm/i915: clear the entire sdvo infoframe buffer Ben Hutchings
2012-11-14  5:39 ` [ 22/82] crypto: cryptd - disable softirqs in cryptd_queue_worker to prevent data corruption Ben Hutchings
2012-11-14  5:39   ` Ben Hutchings
2012-11-14  5:39 ` [ 23/82] ARM: at91: at91sam9g10: fix SOC type detection Ben Hutchings
2012-11-14  5:39 ` [ 24/82] ARM: at91/i2c: change id to let i2c-gpio work Ben Hutchings
2012-11-14  5:39 ` [ 25/82] mac80211: Only process mesh config header on frames that RA_MATCH Ben Hutchings
2012-11-14  5:39 ` [ 26/82] mac80211: dont inspect Sequence Control field on control frames Ben Hutchings
2012-11-14  5:40 ` [ 27/82] gpiolib: Dont return -EPROBE_DEFER to sysfs, or for invalid gpios Ben Hutchings
2012-11-15 17:52   ` Herton Ronaldo Krzesinski
2012-11-15 20:06     ` Ben Hutchings
2012-11-14  5:40 ` [ 28/82] mac80211: fix SSID copy on IBSS JOIN Ben Hutchings
2012-11-14  5:40 ` [ 29/82] wireless: drop invalid mesh address extension frames Ben Hutchings
2012-11-14  5:40 ` [ 30/82] mac80211: check management frame header length Ben Hutchings
2012-11-14  5:40 ` [ 31/82] mac80211: verify that skb data is present Ben Hutchings
2012-11-14  5:40 ` [ 32/82] mac80211: make sure data is accessible in EAPOL check Ben Hutchings
2012-11-14  5:40 ` [ 33/82] ath9k: fix stale pointers potentially causing access to freed skbs Ben Hutchings
2012-11-14  5:40 ` [ 34/82] floppy: do put_disk on current dr if blk_init_queue fails Ben Hutchings
2012-11-14  5:40 ` [ 35/82] floppy: properly handle failure on add_disk loop Ben Hutchings
2012-11-14  5:40 ` [ 36/82] xen/gntdev: dont leak memory from IOCTL_GNTDEV_MAP_GRANT_REF Ben Hutchings
2012-11-14  5:40 ` [ 37/82] rt2800: validate step value for temperature compensation Ben Hutchings
2012-11-14  5:40 ` [ 38/82] ath9k: Test for TID only in BlockAcks while checking tx status Ben Hutchings
2012-11-14  5:40 ` [ 39/82] module: fix out-by-one error in kallsyms Ben Hutchings
2012-11-14  5:40 ` [ 40/82] Input: tsc40 - remove wrong announcement of pressure support Ben Hutchings
2012-11-14  5:40 ` [ 41/82] HID: microsoft: fix invalid rdesc for 3k kbd Ben Hutchings
2012-11-14  5:40 ` [ 42/82] xen/mmu: Use Xen specific TLB flush instead of the generic one Ben Hutchings
2012-11-14  5:40 ` [ 43/82] NFS: Wait for session recovery to finish before returning Ben Hutchings
2012-11-14  5:40 ` [ 44/82] NFSv4.1: We must release the sequence id when we fail to get a session slot Ben Hutchings
2012-11-14  5:40 ` [ 45/82] NFSv4: nfs4_locku_done must release the sequence id Ben Hutchings
2012-11-14  5:40 ` [ 46/82] NFS: fix bug in legacy DNS resolver Ben Hutchings
2012-11-14  5:40 ` [ 47/82] nfsv3: Make v3 mounts fail with ETIMEDOUTs instead EIO on mountd timeouts Ben Hutchings
2012-11-14  5:40 ` [ 48/82] nfs: Show original device name verbatim in /proc/*/mount{s,info} Ben Hutchings
2012-11-14  5:40 ` [ 49/82] target: Dont return success from module_init() if setup fails Ben Hutchings
2012-11-14  5:40 ` [ 50/82] target: Avoid integer overflow in se_dev_align_max_sectors() Ben Hutchings
2012-11-14  5:40 ` Ben Hutchings [this message]
2012-11-14  5:40 ` [ 52/82] DRM/Radeon: Fix Load Detection on legacy primary DAC Ben Hutchings
2012-11-14  5:40 ` [ 53/82] cifs: fix potential buffer overrun in cifs.idmap handling code Ben Hutchings
2012-11-14  5:40 ` [ 54/82] ALSA: hda: Cirrus: Fix coefficient index for beep configuration Ben Hutchings
2012-11-14  5:40 ` [ 55/82] ALSA: HDA: Fix digital microphone on CS420x Ben Hutchings
2012-11-14  5:40 ` [ 56/82] ALSA: hda - Force to reset IEC958 status bits for AD codecs Ben Hutchings
2012-11-14  5:40 ` [ 57/82] hwmon: (w83627ehf) Force initial bank selection Ben Hutchings
2012-11-14  5:40 ` [ 58/82] drm: restore open_count if drm_setup fails Ben Hutchings
2012-11-14  5:40 ` [ 59/82] ALSA: hda - Fix empty DAC filling in patch_via.c Ben Hutchings
2012-11-14  5:40 ` [ 60/82] ALSA: hda - Fix invalid connections in VT1802 codec Ben Hutchings
2012-11-14  5:40 ` [ 61/82] ALSA: hda - Add new codec ALC668 and ALC900 (default name ALC1150) Ben Hutchings
2012-11-14  5:40 ` [ 62/82] ALSA: Fix card refcount unbalance Ben Hutchings
2012-11-14  5:40 ` [ 63/82] xfs: fix reading of wrapped log data Ben Hutchings
2012-11-14  5:40 ` [ 64/82] fanotify: fix missing break Ben Hutchings
2012-11-14  5:40 ` [ 65/82] mm: bugfix: set current->reclaim_state to NULL while returning from kswapd() Ben Hutchings
2012-11-14  5:40 ` [ 66/82] drm/vmwgfx: Fix hibernation device reset Ben Hutchings
2012-11-14  5:40 ` [ 67/82] drm/vmwgfx: Fix a case where the code would BUG when trying to pin GMR memory Ben Hutchings
2012-11-14  5:40 ` [ 68/82] sctp: fix call to SCTP_CMD_PROCESS_SACK in sctp_cmd_interpreter() Ben Hutchings
2012-11-14  5:40 ` [ 69/82] netlink: use kfree_rcu() in netlink_release() Ben Hutchings
2012-11-14  5:40 ` [ 70/82] tcp: fix FIONREAD/SIOCINQ Ben Hutchings
2012-11-14  5:40 ` [ 71/82] ipv6: Set default hoplimit as zero Ben Hutchings
2012-11-14  5:40 ` [ 72/82] net: usb: Fix memory leak on Tx data path Ben Hutchings
2012-11-14  5:40 ` [ 73/82] net: fix divide by zero in tcp algorithm illinois Ben Hutchings
2012-11-14  5:40 ` [ 74/82] l2tp: fix oops in l2tp_eth_create() error path Ben Hutchings
2012-11-14  5:40 ` [ 75/82] af-packet: fix oops when socket is not present Ben Hutchings
2012-11-14  5:40 ` [ 76/82] ipv6: send unsolicited neighbour advertisements to all-nodes Ben Hutchings
2012-11-14  5:40 ` [ 77/82] eCryptfs: Copy up POSIX ACL and read-only flags from lower mount Ben Hutchings
2012-11-14  5:40 ` [ 78/82] eCryptfs: check for eCryptfs cipher support at mount Ben Hutchings
2012-11-14  5:40 ` [ 79/82] r8169: allow multicast packets on sub-8168f chipset Ben Hutchings
2012-11-14  5:40 ` [ 80/82] r8169: Fix WoL on RTL8168d/8111d Ben Hutchings
2012-11-14  5:40 ` [ 81/82] r8169: use unlimited DMA burst for TX Ben Hutchings
2012-11-14  5:40 ` [ 82/82] sky2: Fix for interrupt handler Ben Hutchings
2012-11-14  6:13 ` [ 00/82] 3.2.34-stable review Ben Hutchings

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=20121114053940.926046549@decadent.org.uk \
    --to=ben@decadent.org.uk \
    --cc=agrover@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nab@linux-iscsi.org \
    --cc=roland@purestorage.com \
    --cc=stable@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 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.