Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)

2017-04-14 Thread Logan Gunthorpe
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)

2017-04-14 Thread Julia Lawall
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

2017-04-13 Thread Logan Gunthorpe
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);
+