Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)
Thanks Julia. I missed that and I'll fix it in my series. Logan On 14/04/17 09:19 AM, Julia Lawall wrote: > It looks like >cmdr_lock should be released at line 512 if it has > not been released otherwise. The lock was taken at line 438. > > julia > > -- Forwarded message -- > Date: Fri, 14 Apr 2017 22:21:44 +0800 > From: kbuild test robot <fengguang...@intel.com> > To: kbu...@01.org > Cc: Julia Lawall <julia.law...@lip6.fr> > Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 > call sites > > Hi Logan, > > [auto build test WARNING on scsi/for-next] > [also build test WARNING on v4.11-rc6] > [cannot apply to next-20170413] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518 > base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next > :: branch date: 8 hours ago > :: commit date: 8 hours ago > >>> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438 >drivers/target/target_core_user.c:512:2-8: preceding lock on line 471 > > git remote add linux-review https://github.com/0day-ci/linux > git remote update linux-review > git checkout 78082134e7afdc59d744eb8d2def5c588e89c378 > vim +512 drivers/target/target_core_user.c > > 7c9e7a6f Andy Grover 2014-10-01 432 > sizeof(struct tcmu_cmd_entry)); > 7c9e7a6f Andy Grover 2014-10-01 433 command_size = base_command_size > 7c9e7a6f Andy Grover 2014-10-01 434 + > round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE); > 7c9e7a6f Andy Grover 2014-10-01 435 > 7c9e7a6f Andy Grover 2014-10-01 436 WARN_ON(command_size & > (TCMU_OP_ALIGN_SIZE-1)); > 7c9e7a6f Andy Grover 2014-10-01 437 > 7c9e7a6f Andy Grover 2014-10-01 @438 spin_lock_irq(>cmdr_lock); > 7c9e7a6f Andy Grover 2014-10-01 439 > 7c9e7a6f Andy Grover 2014-10-01 440 mb = udev->mb_addr; > 7c9e7a6f Andy Grover 2014-10-01 441 cmd_head = mb->cmd_head % > udev->cmdr_size; /* UAM */ > 26418649 Sheng Yang 2016-02-26 442 data_length = > se_cmd->data_length; > 26418649 Sheng Yang 2016-02-26 443 if (se_cmd->se_cmd_flags & > SCF_BIDI) { > 26418649 Sheng Yang 2016-02-26 444 > BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents)); > 26418649 Sheng Yang 2016-02-26 445 data_length += > se_cmd->t_bidi_data_sg->length; > 26418649 Sheng Yang 2016-02-26 446 } > 554617b2 Andy Grover 2016-08-25 447 if ((command_size > > (udev->cmdr_size / 2)) || > 554617b2 Andy Grover 2016-08-25 448 data_length > > udev->data_size) { > 554617b2 Andy Grover 2016-08-25 449 pr_warn("TCMU: Request > of size %zu/%zu is too big for %u/%zu " > 3d9b9555 Andy Grover 2016-08-25 450 "cmd ring/data > area\n", command_size, data_length, > 7c9e7a6f Andy Grover 2014-10-01 451 > udev->cmdr_size, udev->data_size); > 554617b2 Andy Grover 2016-08-25 452 > spin_unlock_irq(>cmdr_lock); > 554617b2 Andy Grover 2016-08-25 453 return > TCM_INVALID_CDB_FIELD; > 554617b2 Andy Grover 2016-08-25 454 } > 7c9e7a6f Andy Grover 2014-10-01 455 > 26418649 Sheng Yang 2016-02-26 456 while > (!is_ring_space_avail(udev, command_size, data_length)) { > 7c9e7a6f Andy Grover 2014-10-01 457 int ret; > 7c9e7a6f Andy Grover 2014-10-01 458 DEFINE_WAIT(__wait); > 7c9e7a6f Andy Grover 2014-10-01 459 > 7c9e7a6f Andy Grover 2014-10-01 460 > prepare_to_wait(>wait_cmdr, &__wait, TASK_INTERRUPTIBLE); > 7c9e7a6f Andy Grover 2014-10-01 461 > 7c9e7a6f Andy Grover 2014-10-01 462 pr_debug("sleeping for > ring space\n"); > 7c9e7a6f Andy Grover 2014-10-01 463 > spin_unlock_irq(>cmdr_lock); > 7c9e7a6f Andy Grover 2014-10-01 464 ret = > schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT)); > 7c9e7a6f Andy Grover 2014-10-01 465 > finish_wait(>wait_cmdr, &__wait); > 7c9e7a6f Andy Grover 2014-10-01 466 if (!ret) { > 7c9e7a6f Andy Grover 2014-10-01 467 pr_warn("tcmu: > command timed out\n"); > 02eb924f Andy Grover 2016-10-06 468
Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)
It looks like >cmdr_lock should be released at line 512 if it has not been released otherwise. The lock was taken at line 438. julia -- Forwarded message -- Date: Fri, 14 Apr 2017 22:21:44 +0800 From: kbuild test robot <fengguang...@intel.com> To: kbu...@01.org Cc: Julia Lawall <julia.law...@lip6.fr> Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites Hi Logan, [auto build test WARNING on scsi/for-next] [also build test WARNING on v4.11-rc6] [cannot apply to next-20170413] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next :: branch date: 8 hours ago :: commit date: 8 hours ago >> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438 drivers/target/target_core_user.c:512:2-8: preceding lock on line 471 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 78082134e7afdc59d744eb8d2def5c588e89c378 vim +512 drivers/target/target_core_user.c 7c9e7a6f Andy Grover 2014-10-01 432 sizeof(struct tcmu_cmd_entry)); 7c9e7a6f Andy Grover 2014-10-01 433 command_size = base_command_size 7c9e7a6f Andy Grover 2014-10-01 434 + round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE); 7c9e7a6f Andy Grover 2014-10-01 435 7c9e7a6f Andy Grover 2014-10-01 436 WARN_ON(command_size & (TCMU_OP_ALIGN_SIZE-1)); 7c9e7a6f Andy Grover 2014-10-01 437 7c9e7a6f Andy Grover 2014-10-01 @438 spin_lock_irq(>cmdr_lock); 7c9e7a6f Andy Grover 2014-10-01 439 7c9e7a6f Andy Grover 2014-10-01 440 mb = udev->mb_addr; 7c9e7a6f Andy Grover 2014-10-01 441 cmd_head = mb->cmd_head % udev->cmdr_size; /* UAM */ 26418649 Sheng Yang 2016-02-26 442 data_length = se_cmd->data_length; 26418649 Sheng Yang 2016-02-26 443 if (se_cmd->se_cmd_flags & SCF_BIDI) { 26418649 Sheng Yang 2016-02-26 444 BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents)); 26418649 Sheng Yang 2016-02-26 445 data_length += se_cmd->t_bidi_data_sg->length; 26418649 Sheng Yang 2016-02-26 446 } 554617b2 Andy Grover 2016-08-25 447 if ((command_size > (udev->cmdr_size / 2)) || 554617b2 Andy Grover 2016-08-25 448 data_length > udev->data_size) { 554617b2 Andy Grover 2016-08-25 449 pr_warn("TCMU: Request of size %zu/%zu is too big for %u/%zu " 3d9b9555 Andy Grover 2016-08-25 450 "cmd ring/data area\n", command_size, data_length, 7c9e7a6f Andy Grover 2014-10-01 451 udev->cmdr_size, udev->data_size); 554617b2 Andy Grover 2016-08-25 452 spin_unlock_irq(>cmdr_lock); 554617b2 Andy Grover 2016-08-25 453 return TCM_INVALID_CDB_FIELD; 554617b2 Andy Grover 2016-08-25 454 } 7c9e7a6f Andy Grover 2014-10-01 455 26418649 Sheng Yang 2016-02-26 456 while (!is_ring_space_avail(udev, command_size, data_length)) { 7c9e7a6f Andy Grover 2014-10-01 457 int ret; 7c9e7a6f Andy Grover 2014-10-01 458 DEFINE_WAIT(__wait); 7c9e7a6f Andy Grover 2014-10-01 459 7c9e7a6f Andy Grover 2014-10-01 460 prepare_to_wait(>wait_cmdr, &__wait, TASK_INTERRUPTIBLE); 7c9e7a6f Andy Grover 2014-10-01 461 7c9e7a6f Andy Grover 2014-10-01 462 pr_debug("sleeping for ring space\n"); 7c9e7a6f Andy Grover 2014-10-01 463 spin_unlock_irq(>cmdr_lock); 7c9e7a6f Andy Grover 2014-10-01 464 ret = schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT)); 7c9e7a6f Andy Grover 2014-10-01 465 finish_wait(>wait_cmdr, &__wait); 7c9e7a6f Andy Grover 2014-10-01 466 if (!ret) { 7c9e7a6f Andy Grover 2014-10-01 467 pr_warn("tcmu: command timed out\n"); 02eb924f Andy Grover 2016-10-06 468 return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; 7c9e7a6f Andy Grover 2014-10-01 469 } 7c9e7a6f Andy Grover 2014-10-01 470 7c9e7a6f Andy Grover 2014-10-01 471 spin_lock_irq(>cmdr_lock); 7c9e7a6f Andy Grover 2014-10-01 472 7c9e7a6f Andy Grover 2014-10-01 473 /* We dropped cmdr_lock, cmd_head is stale */ 7c9e7a6f Andy Grover 2014-10-01 474 cmd_head = mb->cmd_head % udev->cmdr_size; /* UAM */ 7c9e7a6f Andy Grover
[PATCH 04/22] target: Make use of the new sg_map function at 16 call sites
Fairly straightforward conversions in all spots. In a couple of cases any error gets propogated up should sg_map fail. In other cases a warning is issued if the kmap fails seeing there's no clear error path. This should not be an issue until someone tries to use unmappable memory in the sgl with this driver. Signed-off-by: Logan Gunthorpe--- drivers/target/iscsi/iscsi_target.c| 27 +--- drivers/target/target_core_rd.c| 3 +- drivers/target/target_core_sbc.c | 122 +++-- drivers/target/target_core_transport.c | 18 +++-- drivers/target/target_core_user.c | 43 include/target/target_core_backend.h | 4 +- 6 files changed, 149 insertions(+), 68 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index a918024..e3e0d8f 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -579,7 +579,7 @@ iscsit_xmit_nondatain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd, } static int iscsit_map_iovec(struct iscsi_cmd *, struct kvec *, u32, u32); -static void iscsit_unmap_iovec(struct iscsi_cmd *); +static void iscsit_unmap_iovec(struct iscsi_cmd *, struct kvec *); static u32 iscsit_do_crypto_hash_sg(struct ahash_request *, struct iscsi_cmd *, u32, u32, u32, u8 *); static int @@ -646,7 +646,7 @@ iscsit_xmit_datain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd, ret = iscsit_fe_sendpage_sg(cmd, conn); - iscsit_unmap_iovec(cmd); + iscsit_unmap_iovec(cmd, >iov_data[1]); if (ret < 0) { iscsit_tx_thread_wait_for_tcp(conn); @@ -925,7 +925,10 @@ static int iscsit_map_iovec( while (data_length) { u32 cur_len = min_t(u32, data_length, sg->length - page_off); - iov[i].iov_base = kmap(sg_page(sg)) + sg->offset + page_off; + iov[i].iov_base = sg_map_offset(sg, page_off, SG_KMAP); + if (IS_ERR(iov[i].iov_base)) + goto map_err; + iov[i].iov_len = cur_len; data_length -= cur_len; @@ -937,17 +940,25 @@ static int iscsit_map_iovec( cmd->kmapped_nents = i; return i; + +map_err: + cmd->kmapped_nents = i - 1; + iscsit_unmap_iovec(cmd, iov); + return -1; } -static void iscsit_unmap_iovec(struct iscsi_cmd *cmd) +static void iscsit_unmap_iovec(struct iscsi_cmd *cmd, struct kvec *iov) { u32 i; struct scatterlist *sg; + unsigned int page_off = cmd->first_data_sg_off; sg = cmd->first_data_sg; - for (i = 0; i < cmd->kmapped_nents; i++) - kunmap(sg_page([i])); + for (i = 0; i < cmd->kmapped_nents; i++) { + sg_unmap_offset([i], iov[i].iov_base, page_off, SG_KMAP); + page_off = 0; + } } static void iscsit_ack_from_expstatsn(struct iscsi_conn *conn, u32 exp_statsn) @@ -1610,7 +1621,7 @@ iscsit_get_dataout(struct iscsi_conn *conn, struct iscsi_cmd *cmd, rx_got = rx_data(conn, >iov_data[0], iov_count, rx_size); - iscsit_unmap_iovec(cmd); + iscsit_unmap_iovec(cmd, iov); if (rx_got != rx_size) return -1; @@ -2626,7 +2637,7 @@ static int iscsit_handle_immediate_data( rx_got = rx_data(conn, >iov_data[0], iov_count, rx_size); - iscsit_unmap_iovec(cmd); + iscsit_unmap_iovec(cmd, cmd->iov_data); if (rx_got != rx_size) { iscsit_rx_thread_wait_for_tcp(conn); diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c index ddc216c..22c5ad5 100644 --- a/drivers/target/target_core_rd.c +++ b/drivers/target/target_core_rd.c @@ -431,7 +431,8 @@ static sense_reason_t rd_do_prot_rw(struct se_cmd *cmd, bool is_read) cmd->t_prot_sg, 0); if (!rc) - sbc_dif_copy_prot(cmd, sectors, is_read, prot_sg, prot_offset); + rc = sbc_dif_copy_prot(cmd, sectors, is_read, prot_sg, + prot_offset); return rc; } diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index c194063..67cb420 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -420,17 +420,17 @@ static sense_reason_t xdreadwrite_callback(struct se_cmd *cmd, bool success, offset = 0; for_each_sg(cmd->t_bidi_data_sg, sg, cmd->t_bidi_data_nents, count) { - addr = kmap_atomic(sg_page(sg)); - if (!addr) { + addr = sg_map(sg, SG_KMAP_ATOMIC); + if (IS_ERR(addr)) { ret = TCM_OUT_OF_RESOURCES; goto out; } for (i = 0; i < sg->length; i++) - *(addr + sg->offset + i) ^= *(buf + offset + i); +