iscsi bugfixes for 2.6.30
Subsystems (4)
These patches fix performance problems hit with cxgb3i including a NULL pointer hit while running heavier workloads.
It also fixes a locking bug in the libiscsi_tcp code that can be hit with bad targets.
PATCH 1/5 iscsi_tcp: don't fire conn error if pdu init fails
From: Mike Christie <[email protected]>
If a command's scsi cmd pdu setup fails then we can just fail the IO to the scsi layer. If a DATA_OUT for a R2T fails then we will want to drop the session, because it means we got a bad request from the target (iscsi protocol error).
This patch has us propogate the error upwards so libiscsi_tcp or libiscsi can decide what the best action is to take. It also fixes a bug where we could try to grab the session lock while holding it, because if iscsi_tcp drops the session in the pdu setup callout the session lock is held when setting up the scsi cmd pdu.
--- drivers/scsi/iscsi_tcp.c | 2 +- drivers/scsi/libiscsi_tcp.c | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-)
drivers/scsi/iscsi_tcp.c +1 -1
@@ -463,7 +463,7 @@ static int iscsi_sw_tcp_pdu_init(struct iscsi_task *task, }if (err) { -iscsi_conn_failure (conn, err); +/* got invalid offset/len */ return -EIO ; }return 0 ;
drivers/scsi/libiscsi_tcp.c +4 -1
@@ -1036,8 +1036,11 @@ flush: rc = conn->session->tt->init_pdu (task, r2t->data_offset + r2t->sent, r2t->data_count); -if (rc) +if (rc) { +iscsi_conn_failure (conn,ISCSI_ERR_XMIT_FAILED );return rc; + } + r2t->sent += r2t->data_count; goto flush; }
Signature · 1 line
1.6.0.6
PATCH 2/5 cxgb3i, iser, iscsi_tcp: set target can queue
From: Mike Christie <[email protected]>
Set target can queue limit to the number of preallocated session tasks we have.
This along with the cxgb3i can_queue patch will fix a throughput problem where it could only queue one LU worth of data at a time.
--- drivers/infiniband/ulp/iser/iscsi_iser.c | 1 + drivers/scsi/cxgb3i/cxgb3i_iscsi.c | 1 + drivers/scsi/iscsi_tcp.c | 1 + drivers/scsi/libiscsi.c | 10 ++++++++++ include/scsi/libiscsi.h | 2 ++ 5 files changed, 15 insertions(+), 0 deletions(-)
drivers/infiniband/ulp/iser/iscsi_iser.c +1 -0
@@ -599,6 +599,7 @@ static struct scsi_host_template iscsi_iser_sht = { .eh_abort_handler = iscsi_eh_abort, .eh_device_reset_handler= iscsi_eh_device_reset, .eh_target_reset_handler= iscsi_eh_target_reset, + .target_alloc = iscsi_target_alloc, .use_clustering = DISABLE_CLUSTERING, .proc_name = "iscsi_iser", .this_id = -1,
drivers/scsi/cxgb3i/cxgb3i_iscsi.c +1 -0
@@ -883,6 +883,7 @@ static struct scsi_host_template cxgb3i_host_template = { .eh_abort_handler = iscsi_eh_abort, .eh_device_reset_handler = iscsi_eh_device_reset, .eh_target_reset_handler = iscsi_eh_target_reset, + .target_alloc = iscsi_target_alloc, .use_clustering = DISABLE_CLUSTERING, .this_id = -1, };
drivers/scsi/iscsi_tcp.c +1 -0
@@ -851,6 +851,7 @@ static struct scsi_host_template iscsi_sw_tcp_sht = { .use_clustering = DISABLE_CLUSTERING, .slave_alloc = iscsi_sw_tcp_slave_alloc, .slave_configure = iscsi_sw_tcp_slave_configure, + .target_alloc = iscsi_target_alloc, .proc_name = "iscsi_tcp", .this_id = -1, };
drivers/scsi/libiscsi.c +10 -0
@@ -1463,6 +1463,16 @@ int iscsi_change_queue_depth(struct scsi_device *sdev, int depth) }EXPORT_SYMBOL_GPL (iscsi_change_queue_depth); +int iscsi_target_alloc (struct scsi_target *starget) +{ +struct iscsi_cls_session *cls_session =starget_to_session (starget); +struct iscsi_session *session = cls_session->dd_data; + + starget->can_queue = session->scsi_cmds_max; +return 0 ; +} +EXPORT_SYMBOL_GPL (iscsi_target_alloc); +void iscsi_session_recovery_timedout (struct iscsi_cls_session *cls_session) {struct iscsi_session *session = cls_session->dd_data;
include/scsi/libiscsi.h +2 -0
@@ -36,6 +36,7 @@ struct scsi_transport_template;struct scsi_host_template ;struct scsi_device ;struct Scsi_Host ; +struct scsi_target ;struct scsi_cmnd ;struct socket ;struct iscsi_transport ;
@@ -350,6 +351,7 @@ extern struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht, bool xmit_can_sleep);extern void iscsi_host_remove (struct Scsi_Host *shost);extern void iscsi_host_free (struct Scsi_Host *shost); +extern int iscsi_target_alloc (struct scsi_target *starget);/* * session management
Signature · 1 line
1.6.0.6
PATCH 3/5 cxgb3i: fix can_queue and cmd_per_lun initialization
From: Mike Christie <[email protected]>
cxgb3i was setting can_queue to only 128 commands, and was setting the can_queue and cmd_per_lun to the same value.
This sets the can_queue to 1024 commands, and sets the cmd_per_lun to a safer default of 32.
--- drivers/scsi/cxgb3i/cxgb3i.h | 2 +- drivers/scsi/cxgb3i/cxgb3i_iscsi.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/scsi/cxgb3i/cxgb3i.h +1 -1
@@ -34,7 +34,7 @@#include "cxgb3i_offload.h" #include "cxgb3i_ddp.h" -#define CXGB3I_SCSI_QDEPTH_DFLT 128 +#define CXGB3I_SCSI_HOST_QDEPTH 1024#define CXGB3I_MAX_TARGET CXGB3I_MAX_CONN#define CXGB3I_MAX_LUN 512#define ISCSI_PDU_NONPAYLOAD_MAX \
drivers/scsi/cxgb3i/cxgb3i_iscsi.c +2 -2
@@ -876,10 +876,10 @@ static struct scsi_host_template cxgb3i_host_template = { .proc_name = "cxgb3i", .queuecommand = iscsi_queuecommand, .change_queue_depth = iscsi_change_queue_depth, - .can_queue = CXGB3I_SCSI_QDEPTH_DFLT - 1, + .can_queue = CXGB3I_SCSI_HOST_QDEPTH, .sg_tablesize = SG_ALL, .max_sectors = 0xFFFF, - .cmd_per_lun = CXGB3I_SCSI_QDEPTH_DFLT, + .cmd_per_lun = ISCSI_DEF_CMD_PER_LUN, .eh_abort_handler = iscsi_eh_abort, .eh_device_reset_handler = iscsi_eh_device_reset, .eh_target_reset_handler = iscsi_eh_target_reset,
Signature · 1 line
1.6.0.6
PATCH 4/5 cxgb3i: fix cpu use abuse during writes
From: Mike Christie <[email protected]>
When doing a lot (128) of large writes (256K) we can hit the cxgb3_snd_win check pretty easily. The driver's xmit thread then takes 100% of the cpu.
The driver should not be returning -EAGAIN for this problem. It should be returing -ENOBUFS, then when the window is opened again it should queue the xmit thread (it already wakes the xmit thread).
--- drivers/scsi/cxgb3i/cxgb3i_offload.c | 4 +++- drivers/scsi/cxgb3i/cxgb3i_pdu.c | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-)
drivers/scsi/cxgb3i/cxgb3i_offload.c +3 -1
@@ -1737,7 +1737,7 @@ int cxgb3i_c3cn_send_pdus(struct s3_conn *c3cn, struct sk_buff *skb)c3cn_tx_debug ("c3cn 0x%p, snd %u - %u > %u.\n" , c3cn, c3cn->write_seq, c3cn->snd_una, cxgb3_snd_win); - err = -EAGAIN ; + err = -ENOBUFS ; goto out_err; }
@@ -1775,6 +1775,8 @@ done: out_err:if (copied ==0 && err == -EPIPE ) copied = c3cn->err ? c3cn->err : -EPIPE ; +else + copied = err; goto done; }
drivers/scsi/cxgb3i/cxgb3i_pdu.c +10 -9
@@ -400,17 +400,18 @@ int cxgb3i_conn_xmit_pdu(struct iscsi_task *task)return 0 ; } -if (err <0 && err != -EAGAIN ) { -kfree_skb (skb); -cxgb3i_tx_debug ("itt 0x%x, skb 0x%p, len %u/%u, xmit err %d.\n" , - task->itt, skb, skb->len, skb->data_len, err); -iscsi_conn_printk (KERN_ERR , task->conn,"xmit err %d.\n" , err); -iscsi_conn_failure (task->conn,ISCSI_ERR_XMIT_FAILED ); +if (err == -EAGAIN || err == -ENOBUFS ) { +/* reset skb to send when we are called again */ + tdata->skb = skb;return err; } -/* reset skb to send when we are called again */ - tdata->skb = skb; -return -EAGAIN ; + +kfree_skb (skb); +cxgb3i_tx_debug ("itt 0x%x, skb 0x%p, len %u/%u, xmit err %d.\n" , + task->itt, skb, skb->len, skb->data_len, err); +iscsi_conn_printk (KERN_ERR , task->conn,"xmit err %d.\n" , err); +iscsi_conn_failure (task->conn,ISCSI_ERR_XMIT_FAILED ); +return err; }int cxgb3i_pdu_init (void )
Signature · 1 line
1.6.0.6
PATCH 5/5 cxgb3i: fix ddp map overrun (v2)
From: [email protected] <[email protected]>
[PATCH v2] cxgb3i: fix ddp map overrun (version 2)
From: Karen Xie <[email protected]>
Fixed a bug in calculating ddp map range when search for free entries:
it was going beyond the end by one, thus corrupting gl_skb[0].
--- drivers/scsi/cxgb3i/cxgb3i_ddp.c | 32 +++++++++++++++++++------------- 1 files changed, 19 insertions(+), 13 deletions(-)
drivers/scsi/cxgb3i/cxgb3i_ddp.c +19 -13
@@ -120,20 +120,26 @@ static void clear_ddp_map(struct cxgb3i_ddp_info *ddp, unsigned int tag, }static inline int ddp_find_unused_entries (struct cxgb3i_ddp_info *ddp, -int start,int max,int count, +unsigned int start,unsigned int max, +unsigned int count,struct cxgb3i_gather_list *gl) { -unsigned int i, j; +unsigned int i, j, k; +/* not enough entries */ +if ((max - start) < count) +return -EBUSY ; + + max -= count;spin_lock (&ddp->map_lock); -for (i = start; i <= max;) { -for (j =0 ; j < count; j++) { -if (ddp->gl_map[i + j]) +for (i = start; i < max;) { +for (j =0 , k = i; j < count; j++, k++) { +if (ddp->gl_map[k])break ; }if (j == count) { -for (j =0 ; j < count; j++) - ddp->gl_map[i + j] = gl; +for (j =0 , k = i; j < count; j++, k++) + ddp->gl_map[k] = gl;spin_unlock (&ddp->map_lock);return i; }
@@ -354,7 +360,7 @@ int cxgb3i_ddp_tag_reserve(struct t3cdev *tdev, unsigned int tid,struct cxgb3i_ddp_info *ddp = tdev->ulp_iscsi;struct pagepod_hdr hdr;unsigned int npods; -int idx =-1 , idx_max; +int idx =-1 ;int err = -ENOMEM ;u32 sw_tag = *tagp;u32 tag;
@@ -367,17 +373,17 @@ int cxgb3i_ddp_tag_reserve(struct t3cdev *tdev, unsigned int tid, } npods = (gl->nelem +PPOD_PAGES_MAX -1 ) >>PPOD_PAGES_SHIFT ; - idx_max = ddp->nppods - npods +1 ;if (ddp->idx_last == ddp->nppods) - idx =ddp_find_unused_entries (ddp,0 , idx_max, npods, gl); + idx =ddp_find_unused_entries (ddp,0 , ddp->nppods, npods, gl);else { idx =ddp_find_unused_entries (ddp, ddp->idx_last +1 , - idx_max, npods, gl); -if (idx <0 && ddp->idx_last >= npods) + ddp->nppods, npods, gl); +if (idx <0 && ddp->idx_last >= npods) { idx =ddp_find_unused_entries (ddp,0 , - ddp->idx_last - npods +1 , +min (ddp->idx_last + npods, ddp->nppods), npods, gl); + } }if (idx <0 ) { ddp_log_debug("xferlen %u, gl %u, npods %u NO DDP.\n",
Signature · 1 line
1.6.0.6