Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1608830
| From | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [PATCH 4.9 17/24] scsi: libiscsi: add lock around task lists to fix list corruption regression |
| Date | 2017-03-24 19:30 +0100 |
| Message-ID | <toC8i-5sk-29@gated-at.bofh.it> (permalink) |
| References | <toBOW-5jj-15@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
4.9-stable review patch. If anyone has any objections, please let me know.
------------------
From: Chris Leech <cleech@redhat.com>
commit 6f8830f5bbab16e54f261de187f3df4644a5b977 upstream.
There's a rather long standing regression from the commit "libiscsi:
Reduce locking contention in fast path"
Depending on iSCSI target behavior, it's possible to hit the case in
iscsi_complete_task where the task is still on a pending list
(!list_empty(&task->running)). When that happens the task is removed
from the list while holding the session back_lock, but other task list
modification occur under the frwd_lock. That leads to linked list
corruption and eventually a panicked system.
Rather than back out the session lock split entirely, in order to try
and keep some of the performance gains this patch adds another lock to
maintain the task lists integrity.
Major enterprise supported kernels have been backing out the lock split
for while now, thanks to the efforts at IBM where a lab setup has the
most reliable reproducer I've seen on this issue. This patch has been
tested there successfully.
Signed-off-by: Chris Leech <cleech@redhat.com>
Fixes: 659743b02c41 ("[SCSI] libiscsi: Reduce locking contention in fast path")
Reported-by: Prashantha Subbarao <psubbara@us.ibm.com>
Reviewed-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/scsi/libiscsi.c | 26 +++++++++++++++++++++++++-
include/scsi/libiscsi.h | 1 +
2 files changed, 26 insertions(+), 1 deletion(-)
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -560,8 +560,12 @@ static void iscsi_complete_task(struct i
WARN_ON_ONCE(task->state == ISCSI_TASK_FREE);
task->state = state;
- if (!list_empty(&task->running))
+ spin_lock_bh(&conn->taskqueuelock);
+ if (!list_empty(&task->running)) {
+ pr_debug_once("%s while task on list", __func__);
list_del_init(&task->running);
+ }
+ spin_unlock_bh(&conn->taskqueuelock);
if (conn->task == task)
conn->task = NULL;
@@ -783,7 +787,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn
if (session->tt->xmit_task(task))
goto free_task;
} else {
+ spin_lock_bh(&conn->taskqueuelock);
list_add_tail(&task->running, &conn->mgmtqueue);
+ spin_unlock_bh(&conn->taskqueuelock);
iscsi_conn_queue_work(conn);
}
@@ -1474,8 +1480,10 @@ void iscsi_requeue_task(struct iscsi_tas
* this may be on the requeue list already if the xmit_task callout
* is handling the r2ts while we are adding new ones
*/
+ spin_lock_bh(&conn->taskqueuelock);
if (list_empty(&task->running))
list_add_tail(&task->running, &conn->requeue);
+ spin_unlock_bh(&conn->taskqueuelock);
iscsi_conn_queue_work(conn);
}
EXPORT_SYMBOL_GPL(iscsi_requeue_task);
@@ -1512,22 +1520,26 @@ static int iscsi_data_xmit(struct iscsi_
* only have one nop-out as a ping from us and targets should not
* overflow us with nop-ins
*/
+ spin_lock_bh(&conn->taskqueuelock);
check_mgmt:
while (!list_empty(&conn->mgmtqueue)) {
conn->task = list_entry(conn->mgmtqueue.next,
struct iscsi_task, running);
list_del_init(&conn->task->running);
+ spin_unlock_bh(&conn->taskqueuelock);
if (iscsi_prep_mgmt_task(conn, conn->task)) {
/* regular RX path uses back_lock */
spin_lock_bh(&conn->session->back_lock);
__iscsi_put_task(conn->task);
spin_unlock_bh(&conn->session->back_lock);
conn->task = NULL;
+ spin_lock_bh(&conn->taskqueuelock);
continue;
}
rc = iscsi_xmit_task(conn);
if (rc)
goto done;
+ spin_lock_bh(&conn->taskqueuelock);
}
/* process pending command queue */
@@ -1535,19 +1547,24 @@ check_mgmt:
conn->task = list_entry(conn->cmdqueue.next, struct iscsi_task,
running);
list_del_init(&conn->task->running);
+ spin_unlock_bh(&conn->taskqueuelock);
if (conn->session->state == ISCSI_STATE_LOGGING_OUT) {
fail_scsi_task(conn->task, DID_IMM_RETRY);
+ spin_lock_bh(&conn->taskqueuelock);
continue;
}
rc = iscsi_prep_scsi_cmd_pdu(conn->task);
if (rc) {
if (rc == -ENOMEM || rc == -EACCES) {
+ spin_lock_bh(&conn->taskqueuelock);
list_add_tail(&conn->task->running,
&conn->cmdqueue);
conn->task = NULL;
+ spin_unlock_bh(&conn->taskqueuelock);
goto done;
} else
fail_scsi_task(conn->task, DID_ABORT);
+ spin_lock_bh(&conn->taskqueuelock);
continue;
}
rc = iscsi_xmit_task(conn);
@@ -1558,6 +1575,7 @@ check_mgmt:
* we need to check the mgmt queue for nops that need to
* be sent to aviod starvation
*/
+ spin_lock_bh(&conn->taskqueuelock);
if (!list_empty(&conn->mgmtqueue))
goto check_mgmt;
}
@@ -1577,12 +1595,15 @@ check_mgmt:
conn->task = task;
list_del_init(&conn->task->running);
conn->task->state = ISCSI_TASK_RUNNING;
+ spin_unlock_bh(&conn->taskqueuelock);
rc = iscsi_xmit_task(conn);
if (rc)
goto done;
+ spin_lock_bh(&conn->taskqueuelock);
if (!list_empty(&conn->mgmtqueue))
goto check_mgmt;
}
+ spin_unlock_bh(&conn->taskqueuelock);
spin_unlock_bh(&conn->session->frwd_lock);
return -ENODATA;
@@ -1738,7 +1759,9 @@ int iscsi_queuecommand(struct Scsi_Host
goto prepd_reject;
}
} else {
+ spin_lock_bh(&conn->taskqueuelock);
list_add_tail(&task->running, &conn->cmdqueue);
+ spin_unlock_bh(&conn->taskqueuelock);
iscsi_conn_queue_work(conn);
}
@@ -2897,6 +2920,7 @@ iscsi_conn_setup(struct iscsi_cls_sessio
INIT_LIST_HEAD(&conn->mgmtqueue);
INIT_LIST_HEAD(&conn->cmdqueue);
INIT_LIST_HEAD(&conn->requeue);
+ spin_lock_init(&conn->taskqueuelock);
INIT_WORK(&conn->xmitwork, iscsi_xmitworker);
/* allocate login_task used for the login/text sequences */
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -196,6 +196,7 @@ struct iscsi_conn {
struct iscsi_task *task; /* xmit task in progress */
/* xmit */
+ spinlock_t taskqueuelock; /* protects the next three lists */
struct list_head mgmtqueue; /* mgmt (control) xmit queue */
struct list_head cmdqueue; /* data-path cmd queue */
struct list_head requeue; /* tasks needing another run */
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 4.9 00/24] 4.9.18-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:20 +0100
[PATCH 4.9 14/24] md/raid1/10: fix potential deadlock Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 15/24] target/pscsi: Fix TYPE_TAPE + TYPE_MEDIMUM_CHANGER export Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 06/24] parisc: Optimize flush_kernel_vmap_range and invalidate_kernel_vmap_range Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 20/24] gfs2: Avoid alignment hole in struct lm_lockname Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 17/24] scsi: libiscsi: add lock around task lists to fix list corruption regression Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 01/24] drm/vc4: Fix termination of the initial scan for branch targets. Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 16/24] scsi: lpfc: Add shutdown method for kexec Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 13/24] powerpc/boot: Fix zImage TOC alignment Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 22/24] cgroup/pids: remove spurious suspicious RCU usage warning Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 18/24] target: Fix VERIFY_16 handling in sbc_parse_cdb Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 03/24] give up on gcc ilog2() constant optimizations Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 05/24] qla2xxx: Fix request queue corruption. Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 12/24] cpufreq: Fix and clean up show_cpuinfo_cur_freq() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 10/24] xprtrdma: Squelch kbuild sparse complaint Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
[PATCH 4.9 11/24] NFS prevent double free in async nfs4_exchange_id Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-24 19:30 +0100
Re: [PATCH 4.9 00/24] 4.9.18-stable review Shuah Khan <shuahkh@osg.samsung.com> - 2017-03-25 01:10 +0100
Re: [PATCH 4.9 00/24] 4.9.18-stable review Javier Martinez Canillas <javier@osg.samsung.com> - 2017-03-25 02:50 +0100
Re: [PATCH 4.9 00/24] 4.9.18-stable review Kevin Hilman <khilman@baylibre.com> - 2017-03-27 19:00 +0200
Re: [PATCH 4.9 00/24] 4.9.18-stable review Guenter Roeck <linux@roeck-us.net> - 2017-03-25 05:20 +0100
csiph-web