mirror of https://lore.kernel.org/lkml/
 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: 96+ 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 ` [ 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome