On Wed, 2014-03-05 at 14:05 +0200, Sagi Grimberg wrote:
> When copying between device and command protection scatters
> we must take into account that device scatters might be offset
> and we might copy outside scatter range. Thus for each cmd prot
> scatter we must take the min between cmd prot scatter, dev prot
> scatter, and whats left (and loop in case we havn't copied enough
> from/to cmd prot scatter).
>
> Example (single t_prot_sg of len 2048):
> kernel: sbc_dif_copy_prot: se_cmd=ffff880380aaf970, left=2048, len=2048, dev_prot_sg_offset=3072, dev_prot_sg_len=4096
> kernel: isert: se_cmd=ffff880380aaf970 PI error found type 0 at sector 0x2600 expected 0x0 vs actual 0x725f, lba=2580
>
> Instead of copying 2048 from offset 3072 (copying junk outside sg
> limit 4096), we must to copy 1024 and continue to next sg until
> we complete cmd prot scatter.
>
> This issue was found using iSER T10-PI offload over rd_mcp (wasn't
> discovered with fileio since file_dev prot sglists are never offset).
>
> Changes from v0:
> - Removed psg->offset consideration for psg_len computation
> - Removed sg->offset consideration for offset condition
> - Added copied consideraiton for len computation
> - Added copied offset to paddr when doing memcpy
>
> Signed-off-by: Sagi Grimberg <sagig@xxxxxxxxxxxx>
> ---
Applied to target-pending/master.
Thanks Sagi!
--nab
> drivers/target/target_core_sbc.c | 37 +++++++++++++++++++++----------------
> 1 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
> index 0d1476a..bd018be 100644
> --- a/drivers/target/target_core_sbc.c
> +++ b/drivers/target/target_core_sbc.c
> @@ -1137,25 +1137,30 @@ sbc_dif_copy_prot(struct se_cmd *cmd, unsigned int sectors, bool read,
> left = sectors * dev->prot_length;
>
> for_each_sg(cmd->t_prot_sg, psg, cmd->t_prot_nents, i) {
> -
> - len = min(psg->length, left);
> - if (offset >= sg->length) {
> - sg = sg_next(sg);
> - offset = 0;
> - }
> + unsigned int psg_len, copied = 0;
>
> paddr = kmap_atomic(sg_page(psg)) + psg->offset;
> - addr = kmap_atomic(sg_page(sg)) + sg->offset + offset;
> -
> - if (read)
> - memcpy(paddr, addr, len);
> - else
> - memcpy(addr, paddr, len);
> -
> - left -= len;
> - offset += len;
> + psg_len = min(left, psg->length);
> + while (copied < psg_len) {
> + len = min(psg_len, sg->length - offset) - copied;
> + addr = kmap_atomic(sg_page(sg)) + sg->offset + offset;
> +
> + if (read)
> + memcpy(paddr + copied, addr, len);
> + else
> + memcpy(addr, paddr + copied, len);
> +
> + left -= len;
> + offset += len;
> + copied += len;
> +
> + if (offset >= sg->length) {
> + sg = sg_next(sg);
> + offset = 0;
> + }
> + kunmap_atomic(addr);
> + }
> kunmap_atomic(paddr);
> - kunmap_atomic(addr);
> }
> }
>
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
