[SCSI] scsi_debug: fix logical block provisioning support
authorAkinobu Mita <akinobu.mita@gmail.com>
Tue, 16 Apr 2013 13:11:58 +0000 (22:11 +0900)
committerJames Bottomley <JBottomley@Parallels.com>
Thu, 2 May 2013 22:45:56 +0000 (15:45 -0700)
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 <akinobu.mita@gmail.com>
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
drivers/scsi/scsi_debug.c

index 4b5d388..154d987 100644 (file)
@@ -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)