Re: [PATCH 1/6] scsi_debug: call map_region() and unmap_region() only when needed

[Martin K. Petersen]

> "Akinobu" == Akinobu Mita  writes:

Akinobu> If the logical block provisioning is not enabled, map_region()
Akinobu> and unmap_region() have no effect and they don't need to be
Akinobu> called.

Akinobu> So this makes map_region() and unmap_region() to be called only
Akinobu> when scsi_debug_lbp() returns true, i.e. logical block
Akinobu> provisioning is enabled.

Acked-by: Martin K. Petersen 


Akinobu> Because scsi_debug_unmap_granularity cannot be zero with usual
Akinobu> setting: scsi_debug_unmap_granularity is 1 by default, and it
Akinobu> can be changed to zero with explicit module parameter setting
Akinobu> only when the logical block provisioning is disabled.  But it
Akinobu> is only meaningful module parameter when the logical block
Akinobu> provisioning is enabled.

As you have found out, I generally allow module parameter values and
combinations thereof that do not make strict sense. That's because the
main reason for adding these parameters in the first place is to test
the block and SCSI layer code that queries them. scsi_debug is a test
vehicle and being able to simulate a device with broken reporting is
something I use a lot.

That said, I don't think we have any test scripts that actually depend
on this combination of unmap parameters so I'm ok with you cleaning them
up.

-- 
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 1/6] scsi_debug: call map_region() and unmap_region() only when needed

[Akinobu Mita]

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 
Cc: "James E.J. Bottomley" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 5cda11c..05abf4e 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);
-- 
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