On Wed, 2014-03-05 at 13:53 +0200, Sagi Grimberg wrote: > On 3/5/2014 4:52 AM, Nicholas A. Bellinger wrote: > > On Sun, 2014-03-02 at 19:51 +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 must not copy outside scatter range. Thus for each cmd prot > >> scatter we must take the min between cmd prot scatter, dev prot > >> scatter, and what's left (and loop in case we havn't copied enough > >> from/to 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). > >> > >> Debug example (single t_prot_sg of len 2048): > >> kernel: sbc_dif_copy_prot: se_cmd=ffff880380aaf970, left=2048, sg_off=3072, sg->length=4096, len=2048 > >> kernel: isert: se_cmd=ffff880380aaf970 PI error found type 0 at sector 0x2600 expected 0x0 vs actual 0x725f, lba=0x2580 > >> > >> Instead of copying 2048 bytes from offset 3072 (copying junk > >> outside sg limit 4096), we must copy 1024 bytes and continue to > >> next sg until we complete cmd prot scatter (1024 from next sg > >> in this case). > >> > >> Same example: > >> kernel: sbc_dif_copy_prot: se_cmd=ffff8803ea6aba28, left=2048, offset=3072, sg->length=4096, len=1024 > >> kernel: sg = sg_next(sg) > >> kernel: sbc_dif_copy_prot: se_cmd=ffff8803ea6aba28, left=1024, offset=0, sg->length=4096, len=1024 > >> > >> Signed-off-by: Sagi Grimberg <sagig@xxxxxxxxxxxx> > >> --- > > A few comments below.. > > <SNIP> > tested with several hours of fio "randrw". seems to work now. > Thanks Sagi! > More on this, > To tell you the truth I don't see a good reason to put this routine > here, It serves RD and fileIO DIF emulation, > but I think it is better to put this copy specific to rd backstore as > the protection information is stored as a SG-list. > For FileIO I don't see why we need a tmp SG-list to bounce between the > prot_file to cmd->t_prot_sg. > > Thoughts? The copy is required for tcm_loop + vhost-scsi who will already have protection SGLs coming from SCSI core, vs. iser-target that will be expecting target core to perform this allocation. For the iser-target type case, a direct map of protection scatterlists from FILEIO would be possible because they are being populated (per command) from a external protection file, however this would not work for RAMDISK given that the protection SGLs are coming from an internal per device list, that could possibility be overwritten in parallel from different outstanding commands. So I don't really see a point is breaking these up for the FILEIO + RAMDISK cases, given that the emulation is not intended for performance critical environments. For those environments, we'd expect folks to be using IBLOCK + bip passthrough to backends that support the VERIFY logic in underlying silicon. --nab -- 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
