From 9ed8d3dc5b46f86ab9117937bd24427ac10e8de5 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Tue, 16 Apr 2013 22:11:55 +0900 Subject: [SCSI] scsi_debug: call map_region() and unmap_region() only when needed If the logical block provisioning is not enabled, map_region() and unmap_region() have no effect and they don't need to be called. So this makes map_region() and unmap_region() to be called only when scsi_debug_lbp() returns true, i.e. logical block provisioning is enabled. While I'm at it, this also removes meaningless non-zero check for scsi_debug_unmap_granularity. Because scsi_debug_unmap_granularity cannot be zero with usual setting: scsi_debug_unmap_granularity is 1 by default, and it can be changed to zero with explicit module parameter setting only when the logical block provisioning is disabled. But it is only meaningful module parameter when the logical block provisioning is enabled. Signed-off-by: Akinobu Mita Acked-by: Martin K. Petersen Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 5cda11c07c68..05abf4e153a3 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -2089,7 +2089,7 @@ static int resp_write(struct scsi_cmnd *SCpnt, unsigned long long lba, write_lock_irqsave(&atomic_rw, iflags); ret = do_device_access(SCpnt, devip, lba, num, 1); - if (scsi_debug_unmap_granularity) + if (scsi_debug_lbp()) map_region(lba, num); write_unlock_irqrestore(&atomic_rw, iflags); if (-1 == ret) @@ -2122,7 +2122,7 @@ static int resp_write_same(struct scsi_cmnd *scmd, unsigned long long lba, write_lock_irqsave(&atomic_rw, iflags); - if (unmap && scsi_debug_unmap_granularity) { + if (unmap && scsi_debug_lbp()) { unmap_region(lba, num); goto out; } @@ -2146,7 +2146,7 @@ static int resp_write_same(struct scsi_cmnd *scmd, unsigned long long lba, fake_storep + (lba * scsi_debug_sector_size), scsi_debug_sector_size); - if (scsi_debug_unmap_granularity) + if (scsi_debug_lbp()) map_region(lba, num); out: write_unlock_irqrestore(&atomic_rw, iflags); -- cgit v1.2.3 From ac17078ae6947254331f56ce4f1db9ea221d43d6 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Tue, 16 Apr 2013 22:11:56 +0900 Subject: [SCSI] scsi_debug: prohibit scsi_debug_unmap_granularity == scsi_debug_unmap_alignment scsi_debug prohibits setting scsi_debug_unmap_alignment to be greater than scsi_debug_unmap_granularity. But setting them to be the same value is not prohibited. In this case, the only difference with scsi_debug_unmap_alignment == 0 is the logical blocks from 0 to scsi_debug_unmap_alignment - 1 cannot be unmapped. But the difference is not properly handled in the current code. So this prohibits such unusual setting. Signed-off-by: Akinobu Mita Acked-by: Martin K. Petersen Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 05abf4e153a3..5c321409fff3 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -3413,9 +3413,10 @@ static int __init scsi_debug_init(void) clamp(scsi_debug_unmap_granularity, 1U, 0xffffffffU); if (scsi_debug_unmap_alignment && - scsi_debug_unmap_granularity < scsi_debug_unmap_alignment) { + scsi_debug_unmap_granularity <= + scsi_debug_unmap_alignment) { printk(KERN_ERR - "%s: ERR: unmap_granularity < unmap_alignment\n", + "%s: ERR: unmap_granularity <= unmap_alignment\n", __func__); return -EINVAL; } -- cgit v1.2.3 From cc34a8e663b2908b9ab487dab8456d117a1e0b93 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Tue, 16 Apr 2013 22:11:57 +0900 Subject: [SCSI] scsi_debug: clear correct memory region when LBPRZ is enabled The function unmap_region() clears memory region specified as the logical block address and the number of logical blocks in ramdisk storage (fake_storep) if lbpu and lbprz module parameters are enabled. In the while loop of unmap_region(), it advances optimal unmap granularity in logical blocks. But it only clears one logical block at LBA 'block' per loop iteration. And furthermore, the 'block' is not pointing to a logical block address which should be cleared, it is a index of probisioning map (map_storep). Signed-off-by: Akinobu Mita Acked-by: Martin K. Petersen Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 5c321409fff3..4b5d3887ff47 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -2059,8 +2059,9 @@ static void unmap_region(sector_t lba, unsigned int len) clear_bit(block, map_storep); if (scsi_debug_lbprz) memset(fake_storep + - block * scsi_debug_sector_size, 0, - scsi_debug_sector_size); + lba * scsi_debug_sector_size, 0, + scsi_debug_sector_size * + scsi_debug_unmap_granularity); } lba += granularity - rem; } -- cgit v1.2.3 From b90ebc3d5c41c9164ae04efd2e4f8204c2a186f1 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Tue, 16 Apr 2013 22:11:58 +0900 Subject: [SCSI] scsi_debug: fix logical block provisioning support provisioning map (map_storep) is a bitmap accessed by bitops. So the allocation size should be a multiple of sizeof(unsigned long) and also the bitmap should be cleared by using bitmap_clear() instead of memset(). Otherwise it will cause problem on big-endian architecture if the number of bits is not a multiple of BITS_PER_LONG. I tried testing the logical block provisioning support in scsi_debug, but it didn't work as I expected. For example, load scsi_debug module with UNMAP command supported and fill the storage with random data. # modprobe scsi_debug lbpu=1 # dd if=/dev/urandom of=/dev/sdb Then, try to unmap LBA 0, but Get LBA status reports: # sg_unmap --lba=0 --num=1 /dev/sdb # sg_get_lba_status --lba=0 /dev/sdb descriptor LBA: 0x0000000000000000 blocks: 16384 mapped This is unexpected result. Because UNMAP command to LBA 0 finished without any errors, but Get LBA status shows that LBA 0 is still mapped. This problem is due to the wrong translation between LBA and index of provisioning map. Fix it by using correct translation functions. Signed-off-by: Akinobu Mita Acked-by: Martin K. Petersen Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 81 ++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 40 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 4b5d3887ff47..154d9870dc5a 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -1997,24 +1997,39 @@ out: return ret; } -static unsigned int map_state(sector_t lba, unsigned int *num) +static unsigned long lba_to_map_index(sector_t lba) +{ + if (scsi_debug_unmap_alignment) { + lba += scsi_debug_unmap_granularity - + scsi_debug_unmap_alignment; + } + do_div(lba, scsi_debug_unmap_granularity); + + return lba; +} + +static sector_t map_index_to_lba(unsigned long index) { - unsigned int granularity, alignment, mapped; - sector_t block, next, end; + return index * scsi_debug_unmap_granularity - + scsi_debug_unmap_alignment; +} - granularity = scsi_debug_unmap_granularity; - alignment = granularity - scsi_debug_unmap_alignment; - block = lba + alignment; - do_div(block, granularity); +static unsigned int map_state(sector_t lba, unsigned int *num) +{ + sector_t end; + unsigned int mapped; + unsigned long index; + unsigned long next; - mapped = test_bit(block, map_storep); + index = lba_to_map_index(lba); + mapped = test_bit(index, map_storep); if (mapped) - next = find_next_zero_bit(map_storep, map_size, block); + next = find_next_zero_bit(map_storep, map_size, index); else - next = find_next_bit(map_storep, map_size, block); + next = find_next_bit(map_storep, map_size, index); - end = next * granularity - scsi_debug_unmap_alignment; + end = min_t(sector_t, sdebug_store_sectors, map_index_to_lba(next)); *num = end - lba; return mapped; @@ -2022,48 +2037,37 @@ static unsigned int map_state(sector_t lba, unsigned int *num) static void map_region(sector_t lba, unsigned int len) { - unsigned int granularity, alignment; sector_t end = lba + len; - granularity = scsi_debug_unmap_granularity; - alignment = granularity - scsi_debug_unmap_alignment; - while (lba < end) { - sector_t block, rem; - - block = lba + alignment; - rem = do_div(block, granularity); + unsigned long index = lba_to_map_index(lba); - if (block < map_size) - set_bit(block, map_storep); + if (index < map_size) + set_bit(index, map_storep); - lba += granularity - rem; + lba = map_index_to_lba(index + 1); } } static void unmap_region(sector_t lba, unsigned int len) { - unsigned int granularity, alignment; sector_t end = lba + len; - granularity = scsi_debug_unmap_granularity; - alignment = granularity - scsi_debug_unmap_alignment; - while (lba < end) { - sector_t block, rem; - - block = lba + alignment; - rem = do_div(block, granularity); + unsigned long index = lba_to_map_index(lba); - if (rem == 0 && lba + granularity < end && block < map_size) { - clear_bit(block, map_storep); - if (scsi_debug_lbprz) + if (lba == map_index_to_lba(index) && + lba + scsi_debug_unmap_granularity <= end && + index < map_size) { + clear_bit(index, map_storep); + if (scsi_debug_lbprz) { memset(fake_storep + lba * scsi_debug_sector_size, 0, scsi_debug_sector_size * scsi_debug_unmap_granularity); + } } - lba += granularity - rem; + lba = map_index_to_lba(index + 1); } } @@ -3402,8 +3406,6 @@ static int __init scsi_debug_init(void) /* Logical Block Provisioning */ if (scsi_debug_lbp()) { - unsigned int map_bytes; - scsi_debug_unmap_max_blocks = clamp(scsi_debug_unmap_max_blocks, 0U, 0xffffffffU); @@ -3422,9 +3424,8 @@ static int __init scsi_debug_init(void) return -EINVAL; } - map_size = (sdebug_store_sectors / scsi_debug_unmap_granularity); - map_bytes = map_size >> 3; - map_storep = vmalloc(map_bytes); + map_size = lba_to_map_index(sdebug_store_sectors - 1) + 1; + map_storep = vmalloc(BITS_TO_LONGS(map_size) * sizeof(long)); printk(KERN_INFO "scsi_debug_init: %lu provisioning blocks\n", map_size); @@ -3435,7 +3436,7 @@ static int __init scsi_debug_init(void) goto free_vm; } - memset(map_storep, 0x0, map_bytes); + bitmap_zero(map_storep, map_size); /* Map first 1KB for partition table */ if (scsi_debug_num_parts) -- cgit v1.2.3