Re: [PATCH 4/6] scsi_debug: add translation functions between LBA and index of provisioning map

2013-05-02 Thread James Bottomley
On Tue, 2013-04-16 at 22:11 +0900, Akinobu Mita wrote:
> The translation from LBA to index of provisioning map (map_storep) is used
> in various places (map_state(), map_region(), and unmap_region()).  But it
> is not correctly calculated if scsi_debug_unmap_alignment is zero.
> 
> This introduces correct translation functions between LBA and index
> of  provisioning map:
> 
> static unsigned long lba_to_map_index(sector_t lba);
> static sector_t map_index_to_lba(unsigned long index);
> 
> Actual bug fixes with using these functions will be done by forthcoming
> patches.

That's not the correct way to split patches, because it leads to untidy
things like this:

drivers/scsi/scsi_debug.c:2000:22: warning: ‘lba_to_map_index’ defined
but not used [-Wunused-function]
drivers/scsi/scsi_debug.c:2011:17: warning: ‘map_index_to_lba’ defined
but not used [-Wunused-function]

I fixed this just by rolling the last three patches together, but in
future, just put the static function in with whatever is calling it.

This is also good commit history practise regardless of the static
warning because it adds the function and the callers of that function in
the same commit meaning people who look later don't have to rummage
around for both commits.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/6] scsi_debug: add translation functions between LBA and index of provisioning map

2013-04-24 Thread Martin K. Petersen
> "Akinobu" == Akinobu Mita  writes:

Akinobu> The translation from LBA to index of provisioning map
Akinobu> (map_storep) is used in various places (map_state(),
Akinobu> map_region(), and unmap_region()).  But it is not correctly
Akinobu> calculated if scsi_debug_unmap_alignment is zero.

Akinobu> This introduces correct translation functions between LBA and
Akinobu> index of provisioning map:

Akinobu> static unsigned long lba_to_map_index(sector_t lba);
Akinobu> static sector_t map_index_to_lba(unsigned long index);

Akinobu> Actual bug fixes with using these functions will be done by
Akinobu> forthcoming patches.

Acked-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/6] scsi_debug: add translation functions between LBA and index of provisioning map

2013-04-16 Thread Akinobu Mita
The translation from LBA to index of provisioning map (map_storep) is used
in various places (map_state(), map_region(), and unmap_region()).  But it
is not correctly calculated if scsi_debug_unmap_alignment is zero.

This introduces correct translation functions between LBA and index
of  provisioning map:

static unsigned long lba_to_map_index(sector_t lba);
static sector_t map_index_to_lba(unsigned long index);

Actual bug fixes with using these functions will be done by forthcoming
patches.

Signed-off-by: Akinobu Mita 
Cc: "James E.J. Bottomley" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 4b5d388..c6de36c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1997,6 +1997,23 @@ out:
return ret;
 }
 
+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)
+{
+   return index * scsi_debug_unmap_granularity -
+   scsi_debug_unmap_alignment;
+}
+
 static unsigned int map_state(sector_t lba, unsigned int *num)
 {
unsigned int granularity, alignment, mapped;
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html