| Message ID | 20190125174609.1855-1-dgilbert@interlog.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 40d07b523cf434f252b134c86b1f8f2d907ffb0b |
| Headers | show |
| Series | scsi_debug: fix write_same with virtual_gb problem | expand |
Doug, > The WRITE SAME(10) and (16) implementations didn't take account of the > buffer wrap required when the virtual_gb parameter is greater than 0. > > Fix that and rename the fake_store() function to lba2fake_store() to > lessen confusion with the global fake_storep pointer. Bump version > date. Applied to 5.0/scsi-fixes, thank you!
On Tue, 2019-01-29 at 01:26 -0500, Martin K. Petersen wrote: > Doug, > > > The WRITE SAME(10) and (16) implementations didn't take account of the > > buffer wrap required when the virtual_gb parameter is greater than 0. > > > > Fix that and rename the fake_store() function to lba2fake_store() to > > lessen confusion with the global fake_storep pointer. Bump version > > date. > > Applied to 5.0/scsi-fixes, thank you! Hi Martin, Has this patch already been sent to Linus? I see it in your 5.0/scsi-fixes branch but I can't find it in Linus' current master branch. Thanks, Bart.
On Tue, 2019-03-19 at 12:24 -0700, Bart Van Assche wrote: > On Tue, 2019-01-29 at 01:26 -0500, Martin K. Petersen wrote: > > Doug, > > > > > The WRITE SAME(10) and (16) implementations didn't take account of the > > > buffer wrap required when the virtual_gb parameter is greater than 0. > > > > > > Fix that and rename the fake_store() function to lba2fake_store() to > > > lessen confusion with the global fake_storep pointer. Bump version > > > date. > > > > Applied to 5.0/scsi-fixes, thank you! > > Hi Martin, > > Has this patch already been sent to Linus? I see it in your 5.0/scsi-fixes > branch but I can't find it in Linus' current master branch. Sorry, I was looking at the wrong branch. The patch is in Linus' master but it seems like that patch is not yet in your 5.1/scsi-queue branch? Thanks, Bart.
Hi Bart, > Has this patch already been sent to Linus? I see it in your > 5.0/scsi-fixes branch but I can't find it in Linus' current master > branch. commit 40d07b523cf434f252b134c86b1f8f2d907ffb0b Author: Douglas Gilbert <dgilbert@interlog.com> Date: Fri Jan 25 12:46:09 2019 -0500 scsi: scsi_debug: fix write_same with virtual_gb problem The WRITE SAME(10) and (16) implementations didn't take account of the buffer wrap required when the virtual_gb parameter is greater than 0. Fix that and rename the fake_store() function to lba2fake_store() to lessen confusion with the global fake_storep pointer. Bump version date. Signed-off-by: Douglas Gilbert <dgilbert@interlog.com> Reported-by: Bart Van Assche <bvanassche@acm.org> Tested by: Bart Van Assche <bvanassche@acm.org> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> $ git tag --contains 40d07b523cf434f252b134c86b1f8f2d907ffb0b v5.0 v5.0-rc5 v5.0-rc6 v5.0-rc7 v5.0-rc8 v5.0.1 v5.0.2 v5.0.3 v5.1-rc1
Bart, > Sorry, I was looking at the wrong branch. The patch is in Linus' > master but it seems like that patch is not yet in your 5.1/scsi-queue > branch? It wouldn't be as 5.1/scsi-queue is based on 5.0-rc1 and the fix went into 5.0-rc5.
On Tue, 2019-03-19 at 15:37 -0400, Martin K. Petersen wrote: > Bart, > > > Sorry, I was looking at the wrong branch. The patch is in Linus' > > master but it seems like that patch is not yet in your 5.1/scsi- > > queue branch? > > It wouldn't be as 5.1/scsi-queue is based on 5.0-rc1 and the fix went > into 5.0-rc5. I tend to manage the merges and conflicts, so if you want everything for both scsi-fixes and scsi-queue, you need to look at my for-next branch. James
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 661512bec3ac..e27f4df24021 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -62,7 +62,7 @@ /* make sure inq_product_rev string corresponds to this version */ #define SDEBUG_VERSION "0188" /* format to fit INQUIRY revision field */ -static const char *sdebug_version_date = "20180128"; +static const char *sdebug_version_date = "20190125"; #define MY_NAME "scsi_debug" @@ -735,7 +735,7 @@ static inline bool scsi_debug_lbp(void) (sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10); } -static void *fake_store(unsigned long long lba) +static void *lba2fake_store(unsigned long long lba) { lba = do_div(lba, sdebug_store_sectors); @@ -2514,8 +2514,8 @@ static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba, return ret; } -/* If fake_store(lba,num) compares equal to arr(num), then copy top half of - * arr into fake_store(lba,num) and return true. If comparison fails then +/* If lba2fake_store(lba,num) compares equal to arr(num), then copy top half of + * arr into lba2fake_store(lba,num) and return true. If comparison fails then * return false. */ static bool comp_write_worker(u64 lba, u32 num, const u8 *arr) { @@ -2643,7 +2643,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec, if (sdt->app_tag == cpu_to_be16(0xffff)) continue; - ret = dif_verify(sdt, fake_store(sector), sector, ei_lba); + ret = dif_verify(sdt, lba2fake_store(sector), sector, ei_lba); if (ret) { dif_errors++; return ret; @@ -3261,10 +3261,12 @@ static int resp_write_scat(struct scsi_cmnd *scp, static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, u32 ei_lba, bool unmap, bool ndob) { + int ret; unsigned long iflags; unsigned long long i; - int ret; - u64 lba_off; + u32 lb_size = sdebug_sector_size; + u64 block, lbaa; + u8 *fs1p; ret = check_device_access_params(scp, lba, num); if (ret) @@ -3276,31 +3278,30 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, unmap_region(lba, num); goto out; } - - lba_off = lba * sdebug_sector_size; + lbaa = lba; + block = do_div(lbaa, sdebug_store_sectors); /* if ndob then zero 1 logical block, else fetch 1 logical block */ + fs1p = fake_storep + (block * lb_size); if (ndob) { - memset(fake_storep + lba_off, 0, sdebug_sector_size); + memset(fs1p, 0, lb_size); ret = 0; } else - ret = fetch_to_dev_buffer(scp, fake_storep + lba_off, - sdebug_sector_size); + ret = fetch_to_dev_buffer(scp, fs1p, lb_size); if (-1 == ret) { write_unlock_irqrestore(&atomic_rw, iflags); return DID_ERROR << 16; - } else if (sdebug_verbose && !ndob && (ret < sdebug_sector_size)) + } else if (sdebug_verbose && !ndob && (ret < lb_size)) sdev_printk(KERN_INFO, scp->device, "%s: %s: lb size=%u, IO sent=%d bytes\n", - my_name, "write same", - sdebug_sector_size, ret); + my_name, "write same", lb_size, ret); /* Copy first sector to remaining blocks */ - for (i = 1 ; i < num ; i++) - memcpy(fake_storep + ((lba + i) * sdebug_sector_size), - fake_storep + lba_off, - sdebug_sector_size); - + for (i = 1 ; i < num ; i++) { + lbaa = lba + i; + block = do_div(lbaa, sdebug_store_sectors); + memmove(fake_storep + (block * lb_size), fs1p, lb_size); + } if (scsi_debug_lbp()) map_region(lba, num); out:
The WRITE SAME(10) and (16) implementations didn't take account of the buffer wrap required when the virtual_gb parameter is greater than 0. Fix that and rename the fake_store() function to lba2fake_store() to lessen confusion with the global fake_storep pointer. Bump version date. Signed-off-by: Douglas Gilbert <dgilbert@interlog.com> Reported-by: Bart Van Assche <bvanassche@acm.org> Tested by: Bart Van Assche <bvanassche@acm.org> --- A change since the test by Bart is memcpy() to memmove() since part of the "fake_store" buffer is being copied to itself which with some LBAs will be overlapping. drivers/scsi/scsi_debug.c | 41 ++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 20 deletions(-)