[PATCH 0/3] percpu-ida/iscsi-target: Address connection reset starved tag hang

[PATCH 0/3] percpu-ida/iscsi-target: Address connection reset starved tag hang

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi folks,

This short series addresses a bug in >= v3.12 iscsi-target code where
a connection reset occuring once percpu_ida_alloc() goes into starved
tag uninterruptible sleep mode causes an indefinate hang, due to the
SIGINT being ignored that normally forces the sleeping rx thread into
connection cleanup state.

This includes modifying existing percpu_ida_alloc() to allow an state
to be passed so that TASK_INTERRUPTIBLE may be optionally set to
address this particular case with iscsi-target, and also includes a
simple backwards compatibility wrapper for existing consumers.

It also bumps the number of pre-allocated tags by double the allowed
per session CmdSN queue_depth in order to avoid the percpu_ida_alloc()
straved tags slow path during normal ExpStatSN acknowledgement if at
all possible.

Thank you,

--nab

Kent Overstreet (1):
  percpu_ida: Add tag alloc interface for interruptible sleep

Nicholas Bellinger (2):
  iscsi-target: Fix connection reset hang with percpu_ida_alloc
  iscsi-target: Pre-allocate more tags to avoid ack starvation

 drivers/target/iscsi/iscsi_target_nego.c |    2 +-
 drivers/target/iscsi/iscsi_target_util.c |    8 ++++++--
 include/linux/percpu_ida.h               |   11 ++++++++++-
 lib/percpu_ida.c                         |   13 +++++++++----
 4 files changed, 26 insertions(+), 8 deletions(-)

-- 
1.7.10.4

[PATCH 1/3] percpu_ida: Add tag alloc interface for interruptible sleep

[Nicholas A. Bellinger]

From: Kent Overstreet <kmo@daterainc.com>

This patch adds an tag allocation interface that allows for
interruptible sleep with GFP_KERNEL, instead of always assuming
uninterruptible sleep preventing a signal from being delivered
to a scheduled process context in the tag stealing slow path.

This includes a wrapper for backwards compatiblity with
existing percpu_ida_alloc() consumers.

Signed-off-by: Kent Overstreet <kmo@daterainc.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 include/linux/percpu_ida.h |   11 ++++++++++-
 lib/percpu_ida.c           |   13 +++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/include/linux/percpu_ida.h b/include/linux/percpu_ida.h
index 1900bd0..bcf31a5 100644
--- a/include/linux/percpu_ida.h
+++ b/include/linux/percpu_ida.h
@@ -3,7 +3,9 @@
 
 #include <linux/types.h>
 #include <linux/bitops.h>
+#include <linux/gfp.h>
 #include <linux/init.h>
+#include <linux/sched.h>
 #include <linux/spinlock_types.h>
 #include <linux/wait.h>
 #include <linux/cpumask.h>
@@ -61,9 +63,16 @@ struct percpu_ida {
 /* Max size of percpu freelist, */
 #define IDA_DEFAULT_PCPU_SIZE	((IDA_DEFAULT_PCPU_BATCH_MOVE * 3) / 2)
 
-int percpu_ida_alloc(struct percpu_ida *pool, gfp_t gfp);
+int percpu_ida_alloc_state(struct percpu_ida *pool, int state);
 void percpu_ida_free(struct percpu_ida *pool, unsigned tag);
 
+static inline int percpu_ida_alloc(struct percpu_ida *pool, gfp_t gfp)
+{
+	return percpu_ida_alloc_state(pool, (gfp & __GFP_WAIT)
+				      ? TASK_UNINTERRUPTIBLE
+				      : TASK_RUNNING);
+}
+
 void percpu_ida_destroy(struct percpu_ida *pool);
 int __percpu_ida_init(struct percpu_ida *pool, unsigned long nr_tags,
 	unsigned long max_size, unsigned long batch_size);
diff --git a/lib/percpu_ida.c b/lib/percpu_ida.c
index 9d054bf..58b2268 100644
--- a/lib/percpu_ida.c
+++ b/lib/percpu_ida.c
@@ -147,7 +147,7 @@ static inline unsigned alloc_local_tag(struct percpu_ida_cpu *tags)
  *
  * Will not fail if passed __GFP_WAIT.
  */
-int percpu_ida_alloc(struct percpu_ida *pool, gfp_t gfp)
+int percpu_ida_alloc_state(struct percpu_ida *pool, int state)
 {
 	DEFINE_WAIT(wait);
 	struct percpu_ida_cpu *tags;
@@ -174,7 +174,7 @@ int percpu_ida_alloc(struct percpu_ida *pool, gfp_t gfp)
 		 *
 		 * global lock held and irqs disabled, don't need percpu lock
 		 */
-		prepare_to_wait(&pool->wait, &wait, TASK_UNINTERRUPTIBLE);
+		prepare_to_wait(&pool->wait, &wait, state);
 
 		if (!tags->nr_free)
 			alloc_global_tags(pool, tags);
@@ -191,9 +191,14 @@ int percpu_ida_alloc(struct percpu_ida *pool, gfp_t gfp)
 		spin_unlock(&pool->lock);
 		local_irq_restore(flags);
 
-		if (tag >= 0 || !(gfp & __GFP_WAIT))
+		if (tag >= 0 || state == TASK_RUNNING)
 			break;
 
+		if (signal_pending_state(state, current)) {
+			tag = -ERESTARTSYS;
+			break;
+		}
+
 		schedule();
 
 		local_irq_save(flags);
@@ -203,7 +208,7 @@ int percpu_ida_alloc(struct percpu_ida *pool, gfp_t gfp)
 	finish_wait(&pool->wait, &wait);
 	return tag;
 }
-EXPORT_SYMBOL_GPL(percpu_ida_alloc);
+EXPORT_SYMBOL_GPL(percpu_ida_alloc_state);
 
 /**
  * percpu_ida_free - free a tag
-- 
1.7.10.4

[PATCH 2/3] iscsi-target: Fix connection reset hang with percpu_ida_alloc

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch addresses a bug where connection reset would hang
indefinately when percpu_ida_alloc() was starved for tags,
and always assumed uninterruptible sleep mode.

So now with percpu_ida_alloc_state() available to make
interruptible sleep optional, convert iscsit_allocate_cmd()
to set TASK_INTERRUPTIBLE when GFP_KERNEL is passed, or
set TASK_RUNNING when GFP_ATOMIC is passed.

Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/iscsi/iscsi_target_util.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index 0819e68..6cf1d95 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -156,9 +156,13 @@ struct iscsi_cmd *iscsit_allocate_cmd(struct iscsi_conn *conn, gfp_t gfp_mask)
 {
 	struct iscsi_cmd *cmd;
 	struct se_session *se_sess = conn->sess->se_sess;
-	int size, tag;
+	int size, tag, state = (gfp_mask & __GFP_WAIT) ? TASK_INTERRUPTIBLE :
+				TASK_RUNNING;
+
+	tag = percpu_ida_alloc_state(&se_sess->sess_tag_pool, state);
+	if (tag < 0)
+		return NULL;
 
-	tag = percpu_ida_alloc(&se_sess->sess_tag_pool, gfp_mask);
 	size = sizeof(struct iscsi_cmd) + conn->conn_transport->priv_size;
 	cmd = (struct iscsi_cmd *)(se_sess->sess_cmd_map + (tag * size));
 	memset(cmd, 0, size);
-- 
1.7.10.4

[PATCH 3/3] iscsi-target: Pre-allocate more tags to avoid ack starvation

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch addresses an traditional iscsi-target fabric ack starvation
issue where iscsit_allocate_cmd() -> percpu_ida_alloc_state() ends up
hitting slow path percpu-ida code, because iscsit_ack_from_expstatsn()
is expected to free ack'ed tags after tag allocation.

This is done to take into account the tags waiting to be acknowledged
and released in iscsit_ack_from_expstatsn(), but who's number are not
directly limited by the CmdSN Window queue_depth being enforced by
the target.

So that said, this patch bumps up the pre-allocated number of
per session tags to:

  min(queue_depth, ISCSIT_MIN_TAGS * 2) + ISCSIT_EXTRA_TAGS

for good measure to avoid the percpu_ida_alloc_state() slow path.

Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/iscsi/iscsi_target_nego.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
index 83c965c..582ba84 100644
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -1192,7 +1192,7 @@ get_target:
 	 */
 alloc_tags:
 	tag_num = max_t(u32, ISCSIT_MIN_TAGS, queue_depth);
-	tag_num += (tag_num / 2) + ISCSIT_EXTRA_TAGS;
+	tag_num = (tag_num * 2) + ISCSIT_EXTRA_TAGS;
 	tag_size = sizeof(struct iscsi_cmd) + conn->conn_transport->priv_size;
 
 	ret = transport_alloc_session_tags(sess->se_sess, tag_num, tag_size);
-- 
1.7.10.4