[PATCH 3/5] scsi_debug: avoid partial copying PI from prot_sglist to dif_storep

[Akinobu Mita]

If data integrity support is enabled, prot_verify_write() is called in
response to WRITE commands and it verifies protection info from
prot_sglist by comparing against data sglist, and copies protection info
to dif_storep.

When multiple blocks are transfered by a WRITE command, it verifies and
copies these blocks one by one.  So if it fails to verify protection
info in the middle of blocks, the actual data transfer to fake_storep
isn't proceeded at all although protection info for some blocks are
already copied to dif_storep.  Therefore, it breaks the data integrity
between fake_storep and dif_storep.

This fixes it by ensuring that copying protection info to dif_storep is
done after all blocks are successfully verified.  Reusing dif_copy_prot()
with supporting the opposite direction simplifies this fix.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 40 +---
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 99e74d7..43369e9 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1790,7 +1790,7 @@ static int dif_verify(struct sd_dif_tuple *sdt, const 
void *data,
 }
 
 static void dif_copy_prot(struct scsi_cmnd *SCpnt, sector_t sector,
- unsigned int sectors)
+ unsigned int sectors, bool read)
 {
unsigned int i, resid;
struct scatterlist *psgl;
@@ -1809,10 +1809,18 @@ static void dif_copy_prot(struct scsi_cmnd *SCpnt, 
sector_t sector,
rest = start + len - dif_store_end;
 
paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
-   memcpy(paddr, start, len - rest);
 
-   if (rest)
-   memcpy(paddr + len - rest, dif_storep, rest);
+   if (read)
+   memcpy(paddr, start, len - rest);
+   else
+   memcpy(start, paddr, len - rest);
+
+   if (rest) {
+   if (read)
+   memcpy(paddr + len - rest, dif_storep, rest);
+   else
+   memcpy(dif_storep, paddr + len - rest, rest);
+   }
 
sector += len / sizeof(*dif_storep);
resid -= len;
@@ -1845,7 +1853,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ei_lba++;
}
 
-   dif_copy_prot(SCpnt, start_sec, sectors);
+   dif_copy_prot(SCpnt, start_sec, sectors, true);
dix_reads++;
 
return 0;
@@ -1928,15 +1936,12 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 {
int i, j, ret;
struct sd_dif_tuple *sdt;
-   struct scatterlist *dsgl = scsi_sglist(SCpnt);
+   struct scatterlist *dsgl;
struct scatterlist *psgl = scsi_prot_sglist(SCpnt);
void *daddr, *paddr;
-   sector_t tmp_sec = start_sec;
-   sector_t sector;
+   sector_t sector = start_sec;
int ppage_offset;
 
-   sector = do_div(tmp_sec, sdebug_store_sectors);
-
BUG_ON(scsi_sg_count(SCpnt) == 0);
BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
 
@@ -1964,25 +1969,13 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
sdt = paddr + ppage_offset;
 
-   ret = dif_verify(sdt, daddr + j, start_sec, ei_lba);
+   ret = dif_verify(sdt, daddr + j, sector, ei_lba);
if (ret) {
dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
-   /* Would be great to copy this in bigger
-* chunks.  However, for the sake of
-* correctness we need to verify each sector
-* before writing it to stable storage
-*/
-   memcpy(dif_storep + sector, sdt, sizeof(*sdt));
-
sector++;
-
-   if (sector == sdebug_store_sectors)
-   sector = 0; /* Force wrap */
-
-   start_sec++;
ei_lba++;
ppage_offset += sizeof(struct sd_dif_tuple);
}
@@ -1991,6 +1984,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
kunmap_atomic(daddr);
}
 
+   dif_copy_prot(SCpnt, start_sec, sectors, false);
dix_writes++;
 
return 0;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord

[PATCH 4/5] scsi_debug: fix invalid value check for guard module parameter

[Akinobu Mita]

In the module initialization, invalid value for guard module parameter
is detected by the following check:

if (scsi_debug_guard  1) {
printk(KERN_ERR scsi_debug_init: guard must be 0 or 1\n);
return -EINVAL;
}

But this check isn't enough, because the type of scsi_debug_guard is
'int' and scsi_debug_guard could be a negative value.

This fixes it by changing the type of scsi_debug_guard to 'unsigned int'
instead of adding extra check for a negative value.

Reported-by: Joe Perches j...@perches.com
Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Joe Perches j...@perches.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
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 43369e9..a21322d 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -169,7 +169,7 @@ static int scsi_debug_dix = DEF_DIX;
 static int scsi_debug_dsense = DEF_D_SENSE;
 static int scsi_debug_every_nth = DEF_EVERY_NTH;
 static int scsi_debug_fake_rw = DEF_FAKE_RW;
-static int scsi_debug_guard = DEF_GUARD;
+static unsigned int scsi_debug_guard = DEF_GUARD;
 static int scsi_debug_lowest_aligned = DEF_LOWEST_ALIGNED;
 static int scsi_debug_max_luns = DEF_MAX_LUNS;
 static int scsi_debug_max_queue = SCSI_DEBUG_CANQUEUE;
@@ -2754,7 +2754,7 @@ module_param_named(dix, scsi_debug_dix, int, S_IRUGO);
 module_param_named(dsense, scsi_debug_dsense, int, S_IRUGO | S_IWUSR);
 module_param_named(every_nth, scsi_debug_every_nth, int, S_IRUGO | S_IWUSR);
 module_param_named(fake_rw, scsi_debug_fake_rw, int, S_IRUGO | S_IWUSR);
-module_param_named(guard, scsi_debug_guard, int, S_IRUGO);
+module_param_named(guard, scsi_debug_guard, uint, S_IRUGO);
 module_param_named(lbpu, scsi_debug_lbpu, int, S_IRUGO);
 module_param_named(lbpws, scsi_debug_lbpws, int, S_IRUGO);
 module_param_named(lbpws10, scsi_debug_lbpws10, int, S_IRUGO);
@@ -3184,7 +3184,7 @@ DRIVER_ATTR(dif, S_IRUGO, sdebug_dif_show, NULL);
 
 static ssize_t sdebug_guard_show(struct device_driver *ddp, char *buf)
 {
-   return scnprintf(buf, PAGE_SIZE, %d\n, scsi_debug_guard);
+   return scnprintf(buf, PAGE_SIZE, %u\n, scsi_debug_guard);
 }
 DRIVER_ATTR(guard, S_IRUGO, sdebug_guard_show, NULL);
 
-- 
1.8.3.1

--
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 2/5] scsi_debug: factor out copying PI from dif_storep to prot_sglist

[Akinobu Mita]

If data integrity support is enabled, prot_verify_read() is called in
response to READ commands and it verifies protection info from dif_storep
by comparing against fake_storep, and copies protection info to
prot_sglist.

This factors out the portion of copying protection info into a separate
function.  It will also be reused in the next change after supporting
the opposite direction (copying prot_sglist to dif_storep).

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 52 ++-
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index f640b6b..99e74d7 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1789,37 +1789,16 @@ static int dif_verify(struct sd_dif_tuple *sdt, const 
void *data,
return 0;
 }
 
-static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
-   unsigned int sectors, u32 ei_lba)
+static void dif_copy_prot(struct scsi_cmnd *SCpnt, sector_t sector,
+ unsigned int sectors)
 {
unsigned int i, resid;
struct scatterlist *psgl;
-   struct sd_dif_tuple *sdt;
-   sector_t sector;
void *paddr;
const void *dif_store_end = dif_storep + sdebug_store_sectors;
 
-   for (i = 0; i  sectors; i++) {
-   int ret;
-
-   sector = start_sec + i;
-   sdt = dif_store(sector);
-
-   if (sdt-app_tag == 0x)
-   continue;
-
-   ret = dif_verify(sdt, fake_store(sector), sector, ei_lba);
-   if (ret) {
-   dif_errors++;
-   return ret;
-   }
-
-   ei_lba++;
-   }
-
/* Bytes of protection data to copy into sgl */
resid = sectors * sizeof(*dif_storep);
-   sector = start_sec;
 
scsi_for_each_prot_sg(SCpnt, psgl, scsi_prot_sg_count(SCpnt), i) {
int len = min(psgl-length, resid);
@@ -1839,7 +1818,34 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
resid -= len;
kunmap_atomic(paddr);
}
+}
+
+static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
+   unsigned int sectors, u32 ei_lba)
+{
+   unsigned int i;
+   struct sd_dif_tuple *sdt;
+   sector_t sector;
+
+   for (i = 0; i  sectors; i++) {
+   int ret;
+
+   sector = start_sec + i;
+   sdt = dif_store(sector);
+
+   if (sdt-app_tag == 0x)
+   continue;
+
+   ret = dif_verify(sdt, fake_store(sector), sector, ei_lba);
+   if (ret) {
+   dif_errors++;
+   return ret;
+   }
+
+   ei_lba++;
+   }
 
+   dif_copy_prot(SCpnt, start_sec, sectors);
dix_reads++;
 
return 0;
-- 
1.8.3.1

--
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 -next] ufs: fix source address of the read descriptor

[Akinobu Mita]

When the query request with read descriptor opcode is completed, the
descriptor is copied from response UPIU to the buffer that the caller
has specified.  Unfortunately the source address of the descriptor is
broken due to the unnecessary address-of operator.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Dolev Raviv dra...@codeaurora.org
Cc: Sujit Reddy Thumma sthu...@codeaurora.org
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a0f5ac2..7a319c6 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -454,8 +454,7 @@ void ufshcd_copy_query_response(struct ufs_hba *hba, struct 
ufshcd_lrb *lrbp)
 
/* Get the descriptor */
if (lrbp-ucd_rsp_ptr-qr.opcode == UPIU_QUERY_OPCODE_READ_DESC) {
-   u8 *descp = (u8 *)lrbp-ucd_rsp_ptr +
-   GENERAL_UPIU_REQUEST_SIZE;
+   u8 *descp = (u8 *)lrbp-ucd_rsp_ptr + GENERAL_UPIU_REQUEST_SIZE;
u16 len;
 
/* data segment length */
-- 
1.8.3.1

--
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 2/2] scsi_debug: fix logical block provisioning support when unmap_alignment != 0

[Akinobu Mita]

Commit b90ebc3d5c41c9164ae04efd2e4f8204c2a186f1 ([SCSI] scsi_debug:
fix logical block provisioning support) fixed several issues with
logical block provisioning support, but it still doesn't properly fix
the cases when unmap_alignment  0.

For example, load scsi_debug module with the following module parameters
and make all blocks mapped by filling the storage with zero.

# modprobe scsi_debug lbpu=1 unmap_alignment=1 unmap_granularity=4
# dd if=/dev/zero of=$DEV

Then, try to unmap the first unmappable blocks at lba=1, but GET LBA STATUS
unexpectedly reports that the last UNMAP has done nothing.

# sg_unmap --lba=1 --num=4 $DEV
# sg_get_lba_status --lba=1 $DEV
descriptor LBA: 0x0001  blocks: 16383  mapped

The problem is in map_index_to_lba(), which should return the first
LBA which is corresponding to a given index of provisioning map
(map_storep).

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Douglas Gilbert dgilb...@interlog.com
Acked-by: Martin K. Petersen martin.peter...@oracle.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 2f39b13..01c0ffa 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1997,8 +1997,14 @@ static unsigned long lba_to_map_index(sector_t lba)
 
 static sector_t map_index_to_lba(unsigned long index)
 {
-   return index * scsi_debug_unmap_granularity -
-   scsi_debug_unmap_alignment;
+   sector_t lba = index * scsi_debug_unmap_granularity;
+
+   if (scsi_debug_unmap_alignment) {
+   lba -= scsi_debug_unmap_granularity -
+   scsi_debug_unmap_alignment;
+   }
+
+   return lba;
 }
 
 static unsigned int map_state(sector_t lba, unsigned int *num)
-- 
1.8.3.1

--
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 0/2] scsi_debug: bug fixes for certain module parameters

[Akinobu Mita]

Hi James,

Please consider to apply these patches to your tree.  These patches are
subset of the patch set I sent before:
http://marc.info/?l=linux-scsim=137388918402750w=2
I'm sending the patches which have been acked by the appropriate reviewers.
The rest of the patches need some rework, and I'll try to submit next time.

Akinobu Mita (2):
  scsi_debug: fix endianness bug in sdebug_build_parts()
  scsi_debug: fix logical block provisioning support when
unmap_alignment != 0

 drivers/scsi/scsi_debug.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Martin Peschke mpesc...@linux.vnet.ibm.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
-- 
1.8.3.1

--
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/2] scsi_debug: fix endianness bug in sdebug_build_parts()

[Akinobu Mita]

With module parameter num_parts  0, partition table is built on the
ramdisk storage when loading the driver.  Unfortunately, there is an
endianness bug in sdebug_build_parts().  So the partition table is not
correctly initialized on big-endian systems.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Martin Peschke mpesc...@linux.vnet.ibm.com
Acked-by: Douglas Gilbert dgilb...@interlog.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..2f39b13 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2659,8 +2659,8 @@ static void __init sdebug_build_parts(unsigned char *ramp,
   / sdebug_sectors_per;
pp-end_sector = (end_sec % sdebug_sectors_per) + 1;
 
-   pp-start_sect = start_sec;
-   pp-nr_sects = end_sec - start_sec + 1;
+   pp-start_sect = cpu_to_le32(start_sec);
+   pp-nr_sects = cpu_to_le32(end_sec - start_sec + 1);
pp-sys_ind = 0x83; /* plain Linux partition */
}
 }
-- 
1.8.3.1

--
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] scsi: fix the build warning

[Akinobu Mita]

2013/8/22 Martin K. Petersen martin.peter...@oracle.com:
 Joe == Joe Perches j...@perches.com writes:

 Joe I don't get this build warning in the first place and I think the
 Joe scsi_debug file is quite old and probably doesn't need to be
 Joe changed at all.

 guard isn't a boolean, it selects the checksum algorithm used.

 Also, I believe Akinobu's recent reorganization of this code in question
 fixed the warning.

Unfortunately, this warning isn't fixed in linux-next, either.
Paul Bolle also sent a patch that fixes the same warning in a little
bit different way.

http://marc.info/?l=linux-scsim=137404660520109w=2
--
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] scsi: fix the build warning

[Akinobu Mita]

2013/8/22 James Bottomley jbottom...@parallels.com:
 On Thu, 2013-08-22 at 21:42 +0900, Akinobu Mita wrote:
 2013/8/22 Martin K. Petersen martin.peter...@oracle.com:
  Joe == Joe Perches j...@perches.com writes:
 
  Joe I don't get this build warning in the first place and I think the
  Joe scsi_debug file is quite old and probably doesn't need to be
  Joe changed at all.
 
  guard isn't a boolean, it selects the checksum algorithm used.
 
  Also, I believe Akinobu's recent reorganization of this code in question
  fixed the warning.

 Unfortunately, this warning isn't fixed in linux-next, either.
 Paul Bolle also sent a patch that fixes the same warning in a little
 bit different way.

 Well, it is and it isn't.  Whether you see the warning seems to depend
 on how gcc was built.  My take is that an impossible default case just
 to keep some versions of gcc quiet is a bit pointless.

As Joe said in the other reply, scsi_debug_guard could be a negative
value (scsi_debug_guard  1 is only prohibited).  So this warning
does not seem a false positive.
--
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 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0

[Akinobu Mita]

2013/8/21 Douglas Gilbert dgilb...@interlog.com:
 On 13-08-19 10:16 AM, Akinobu Mita wrote:

 Hi Douglas, Martin,

 Could you review this patch when you have a time?  I would like to
 submit at least this patch 2/4, and 1/4 which has already been acked
 by Douglas for the next merge window.

 Although the patches 2/4 ~ 4/4 are all related to the logical block
 provisioning support, the problems that fixed by 3/4 and 4/4 only
 happen with virtual_gb option enabled, too.  On the other hand, the
 problem that fixed by 2/4 is easily reproduced by, for example,
 'modprobe scsi_debug lbpu=1 unmap_alignment=1 unmap_granularity=4'.
 So the patch 2/4 has rather higher severity than others.


 This is Martin's area of expertise so I hope he also
 acks it.

 Acked-by: Douglas Gilbert dgilb...@interlog.com

Thanks.

BTW, I realized that the commit log for this change didn't fully
describe the problem.  So I'll update it so that it includes a
concrete example like below.

How to reproduce this problem:
# modprobe scsi_debug lbpu=1 unmap_alignment=1 unmap_granularity=4
# dd if=/dev/zero of=/dev/sdb
# sg_unmap --lba=1 --num=4 /dev/sdb

GET LBA STATUS command for lba=1 shows that the last UNMAP command didn't work:
# sg_get_lba_status --lba=1 /dev/sdb
descriptor LBA: 0x0001  blocks: 16383  mapped
--
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 1/2] ufs-pltfrm: initialize DMA mask for device-tree probed device

[Akinobu Mita]

 On 8/19/2013 8:02 PM, James Bottomley wrote:
 On Mon, 2013-08-19 at 22:56 +0900, Akinobu Mita wrote:
 The device-tree probed device for ARM doesn't have dev-dma_mask.
 So dma_set_mask() for the device doesn't succeed.  The popular trick
 for this is - dev-dma_mask = dev-coherent_dma_mask;

 Currently there is no dma_set_mask() call in ufs-pltfrm, but the
 forthcoming fix needs proper DMA mask setting in ufs core driver.  So
 initializing dev-dma_mask as described above is required.

 Signed-off-by: Akinobu Mita m...@fixstars.com
 Cc: Sujit Reddy Thumma sthu...@codeaurora.org
 Cc: Vinayak Holikatti vinholika...@gmail.com
 Cc: Santosh Y santos...@gmail.com
 Cc: James E.J. Bottomley jbottom...@parallels.com
 Cc: linux-scsi@vger.kernel.org
 ---
   drivers/scsi/ufs/ufshcd-pltfrm.c | 3 +++
   1 file changed, 3 insertions(+)

 diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c
 b/drivers/scsi/ufs/ufshcd-pltfrm.c
 index 94ba40c..c780840 100644
 --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
 +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
 @@ -122,6 +122,9 @@ static int ufshcd_pltfrm_probe(struct
 platform_device *pdev)
   goto out;
   }

 +if (!dev-dma_mask)
 +dev-dma_mask = dev-coherent_dma_mask;
 +

 If the DMA mask is NULL, it means there's buggy platform code somewhere;
 I'm not sure we should be hacking a fix in a SCSI driver.

 Yes, ideally DT core should do this, there are patches lying around but
 are not converged. Adding devicet...@vger.kernel.org to see if someone
 has better suggestions.

 Recent additions to kernel with similar hacks -
 https://patchwork.kernel.org/patch/2537021/

The discussion in that thread is useful.  Also, I found that Russell King
proposed replacing the boilerplate by using dma_coerce_mask_and_coherent()
in his patch set Preview of DMA mask changes.
https://patchwork.kernel.org/patch/2837359/

--
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/2] ufs-pltfrm: initialize DMA mask for device-tree probed device

[Akinobu Mita]

The device-tree probed device for ARM doesn't have dev-dma_mask.
So dma_set_mask() for the device doesn't succeed.  The popular trick
for this is - dev-dma_mask = dev-coherent_dma_mask;

Currently there is no dma_set_mask() call in ufs-pltfrm, but the
forthcoming fix needs proper DMA mask setting in ufs core driver.  So
initializing dev-dma_mask as described above is required.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Sujit Reddy Thumma sthu...@codeaurora.org
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 94ba40c..c780840 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -122,6 +122,9 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
goto out;
}
 
+   if (!dev-dma_mask)
+   dev-dma_mask = dev-coherent_dma_mask;
+
err = ufshcd_init(dev, hba, mmio_base, irq);
if (err) {
dev_err(dev, Intialization failed\n);
-- 
1.8.3.1

--
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 2/2] ufs: fix DMA mask setting

[Akinobu Mita]

If the controller doesn't support 64-bit addressing mode, it must not
set the DMA mask to 64-bit.  But it's unconditionally trying to set to
64-bit without checking 64-bit addressing support in the controller
capabilities.

It was correctly checked before commit 3b1d05807a9a68c6d0580e9248247a774a4d3be6
([SCSI] ufs: Segregate PCI Specific Code), this aims to restores
the correct behaviour.

To achieve this in a generic way, firstly we should push down the DMA
mask setting routine ufshcd_set_dma_mask() from PCI glue driver to core
driver in order to do it for both PCI glue driver and Platform glue
driver.  Secondly, we should change pci_ DMA mapping API to dma_ DMA
mapping API because core driver is independent of glue drivers.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Sujit Reddy Thumma sthu...@codeaurora.org
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
This patch was rejected because of the incorrect usage of dma_set_mask().
Now it is fixed, but it also requires ufshcd-pltfrm change in the separate
patch 1/2.

In order to apply this patch cleanly, this patch depends on 
'ufshcd-pci: release ioremapped region during removing driver' which
has already acked by Santosh.

 drivers/scsi/ufs/ufshcd-pci.c | 26 --
 drivers/scsi/ufs/ufshcd.c | 30 ++
 2 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 2349c0e..91a2e79 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -96,26 +96,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
 }
 
 /**
- * ufshcd_set_dma_mask - Set dma mask based on the controller
- *  addressing capability
- * @pdev: PCI device structure
- *
- * Returns 0 for success, non-zero for failure
- */
-static int ufshcd_set_dma_mask(struct pci_dev *pdev)
-{
-   int err;
-
-   if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(64))
-!pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)))
-   return 0;
-   err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
-   if (!err)
-   err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
-   return err;
-}
-
-/**
  * ufshcd_pci_probe - probe routine of the driver
  * @pdev: pointer to PCI device handle
  * @id: PCI device id
@@ -145,12 +125,6 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
 
mmio_base = pcim_iomap_table(pdev)[0];
 
-   err = ufshcd_set_dma_mask(pdev);
-   if (err) {
-   dev_err(pdev-dev, set dma mask failed\n);
-   return err;
-   }
-
err = ufshcd_init(pdev-dev, hba, mmio_base, pdev-irq);
if (err) {
dev_err(pdev-dev, Initialization failed\n);
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bd4d418..1f37c36 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1667,6 +1667,30 @@ void ufshcd_remove(struct ufs_hba *hba)
 EXPORT_SYMBOL_GPL(ufshcd_remove);
 
 /**
+ * ufshcd_set_dma_mask - Set dma mask based on the controller
+ *  addressing capability
+ * @hba: per adapter instance
+ *
+ * Returns 0 for success, non-zero for failure
+ */
+static int ufshcd_set_dma_mask(struct ufs_hba *hba)
+{
+   int err;
+
+   if (hba-capabilities  MASK_64_ADDRESSING_SUPPORT) {
+   if (!dma_set_mask(hba-dev, DMA_BIT_MASK(64))) {
+   dma_set_coherent_mask(hba-dev, DMA_BIT_MASK(64));
+   return 0;
+   }
+   }
+   err = dma_set_mask(hba-dev, DMA_BIT_MASK(32));
+   if (!err)
+   dma_set_coherent_mask(hba-dev, DMA_BIT_MASK(32));
+
+   return err;
+}
+
+/**
  * ufshcd_init - Driver initialization routine
  * @dev: pointer to device handle
  * @hba_handle: driver private handle
@@ -1717,6 +1741,12 @@ int ufshcd_init(struct device *dev, struct ufs_hba 
**hba_handle,
/* Get Interrupt bit mask per version */
hba-intr_mask = ufshcd_get_intr_mask(hba);
 
+   err = ufshcd_set_dma_mask(hba);
+   if (err) {
+   dev_err(hba-dev, set dma mask failed\n);
+   goto out_disable;
+   }
+
/* Allocate memory for host memory space */
err = ufshcd_memory_alloc(hba);
if (err) {
-- 
1.8.3.1

--
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 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0

[Akinobu Mita]

Hi Douglas, Martin,

Could you review this patch when you have a time?  I would like to
submit at least this patch 2/4, and 1/4 which has already been acked
by Douglas for the next merge window.

Although the patches 2/4 ~ 4/4 are all related to the logical block
provisioning support, the problems that fixed by 3/4 and 4/4 only
happen with virtual_gb option enabled, too.  On the other hand, the
problem that fixed by 2/4 is easily reproduced by, for example,
'modprobe scsi_debug lbpu=1 unmap_alignment=1 unmap_granularity=4'.
So the patch 2/4 has rather higher severity than others.

2013/7/15 Akinobu Mita akinobu.m...@gmail.com:
 Commit b90ebc3d5c41c9164ae04efd2e4f8204c2a186f1 ([SCSI] scsi_debug:
 fix logical block provisioning support) fixed several issues with
 logical block provisioning support, but it still doesn't properly fix
 the cases when unmap_alignment  0.  (for example, unmap_alignment=1
 and unmap_granularity=3)

 The problem is in map_index_to_lba(), which should return the first
 LBA which is corresponding to a given index of provisioning map
 (map_storep).

 Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
 Cc: James E.J. Bottomley jbottom...@parallels.com
 Cc: Douglas Gilbert dgilb...@interlog.com
 Cc: Martin K. Petersen martin.peter...@oracle.com
 Cc: linux-scsi@vger.kernel.org
 ---
  drivers/scsi/scsi_debug.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

 diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
 index 2f39b13..01c0ffa 100644
 --- a/drivers/scsi/scsi_debug.c
 +++ b/drivers/scsi/scsi_debug.c
 @@ -1997,8 +1997,14 @@ static unsigned long lba_to_map_index(sector_t lba)

  static sector_t map_index_to_lba(unsigned long index)
  {
 -   return index * scsi_debug_unmap_granularity -
 -   scsi_debug_unmap_alignment;
 +   sector_t lba = index * scsi_debug_unmap_granularity;
 +
 +   if (scsi_debug_unmap_alignment) {
 +   lba -= scsi_debug_unmap_granularity -
 +   scsi_debug_unmap_alignment;
 +   }
 +
 +   return lba;
  }

  static unsigned int map_state(sector_t lba, unsigned int *num)
 --
 1.8.3.1

--
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 10/10] ufs: fix DMA mask setting

[Akinobu Mita]

 On 6/29/2013 11:10 AM, Akinobu Mita wrote:
 2013/6/29 James Bottomley james.bottom...@hansenpartnership.com:
 On Wed, 2013-06-26 at 22:39 +0530, Santosh Y wrote:
 index 19618c6..431ddb2 100644
 --- a/drivers/scsi/ufs/ufshcd.c
 +++ b/drivers/scsi/ufs/ufshcd.c
 @@ -1711,6 +1711,25 @@ void ufshcd_remove(struct ufs_hba *hba)
   EXPORT_SYMBOL_GPL(ufshcd_remove);

   /**
 + * ufshcd_set_dma_mask - Set dma mask based on the controller
 + *addressing capability
 + * @hba: per adapter instance
 + *
 + * Returns 0 for success, non-zero for failure
 + */
 +static int ufshcd_set_dma_mask(struct ufs_hba *hba)
 +{
 + if (hba-capabilities  MASK_64_ADDRESSING_SUPPORT) {
 + if (!dma_set_mask(hba-dev, DMA_BIT_MASK(64)) 
 + !dma_set_coherent_mask(hba-dev, DMA_BIT_MASK(64)))
 + return 0;
 + }
 + dma_set_mask(hba-dev, DMA_BIT_MASK(32));
 +
 + return dma_set_coherent_mask(hba-dev, DMA_BIT_MASK(32));
 +}

 This isn't right per the API spec.  The guarantee is that if
 dma_set_mask() succeeds then dma_set_coherent_mask of the same mask
 will
 succeed,  so this should read

  int err;

  if (hba-capabilities  MASK_64_ADDRESSING_SUPPORT) {
  if (!dma_set_mask(hba-dev, DMA_BIT_MASK(64))) {
  dma_set_coherent_mask(hba-dev,
 DMA_BIT_MASK(64)))
  return 0;
  }
  }
  err = dma_set_mask(hba-dev, DMA_BIT_MASK(32));
  if (!err)
  dma_set_coherent_mask(hba-dev, DMA_BIT_MASK(32));
  return err;

 Thanks for the explanation.  I agree that this is the correct definision
 of ufshcd_set_dma_mask().

 The reason that I omitted the error check on
 dma_set_mask(DMA_BIT_MASK(32))
 in the patch was that I was seeing that error due to the luck of
 valid dev-dma_mask pointer on OF platform devices although
 dma_supported(DMA_BIT_MASK(32)) returns true.

 The popular trick implemented for device-tree probed devices is -
 dev-dma_mask = dev-coherent_dma_mask;

This looks the right solution for now.  I'll send revised patches that
include this trick in ufshcd-pltfrm and fixed ufshcd_set_dma_mask() as
James suggested above.

--
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 1/4] scsi_debug: fix endianness bug in sdebug_build_parts()

[Akinobu Mita]

2013/7/26 Martin Peschke mpesc...@linux.vnet.ibm.com:
 On Mon, 2013-07-15 at 20:52 +0900, Akinobu Mita wrote:
 With module parameter num_parts  0, partition table is built on the
 ramdisk storage when loading the driver.  Unfortunately, there is an
 endianness bug in sdebug_build_parts().  So the partition table is not
 correctly initialized on big-endian systems.

 Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
 Cc: James E.J. Bottomley jbottom...@parallels.com
 Cc: Douglas Gilbert dgilb...@interlog.com
 Cc: linux-scsi@vger.kernel.org
 ---
  drivers/scsi/scsi_debug.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
 index cb4fefa..2f39b13 100644
 --- a/drivers/scsi/scsi_debug.c
 +++ b/drivers/scsi/scsi_debug.c
 @@ -2659,8 +2659,8 @@ static void __init sdebug_build_parts(unsigned char 
 *ramp,
  / sdebug_sectors_per;
   pp-end_sector = (end_sec % sdebug_sectors_per) + 1;

 - pp-start_sect = start_sec;
 - pp-nr_sects = end_sec - start_sec + 1;
 + pp-start_sect = cpu_to_le32(start_sec);
 + pp-nr_sects = cpu_to_le32(end_sec - start_sec + 1);
   pp-sys_ind = 0x83; /* plain Linux partition */
   }
  }


 I have posted the same fix several times, e.g.
 http://marc.info/?l=linux-scsim=137051617907423w=2
 Good luck!

 Acked-by: Martin Peschke mpesc...@linux.vnet.ibm.com

Thanks.  I found this problem from sparse warning noticed by 0-DAY kernel
build testing.
--
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 v2 2/3] ufs: don't disable_irq() if the IRQ can be shared among devices

[Akinobu Mita]

When removing the UFS driver, disable_irq() is called and the IRQ is
not enabled again.  Unfortunately, the IRQ is requested with IRQF_SHARED
and it can be shared among several devices.  So disabling the IRQ in
this way is just breaking other devices which are sharing the IRQ.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd-pci.c| 1 -
 drivers/scsi/ufs/ufshcd-pltfrm.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index de4c52b..2349c0e 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -91,7 +91,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
 {
struct ufs_hba *hba = pci_get_drvdata(pdev);
 
-   disable_irq(pdev-irq);
ufshcd_remove(hba);
pci_set_drvdata(pdev, NULL);
 }
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index c42db40..94ba40c 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -144,7 +144,6 @@ static int ufshcd_pltfrm_remove(struct platform_device 
*pdev)
 {
struct ufs_hba *hba =  platform_get_drvdata(pdev);
 
-   disable_irq(hba-irq);
ufshcd_remove(hba);
return 0;
 }
-- 
1.8.3.1

--
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 v2 3/3] ufs: don't stop controller before scsi_remove_host()

[Akinobu Mita]

scsi_remove_host() sends SYNCHRONIZE CACHE commands for write cache
enabled scsi disk devices.  So stopping controller working shouldn't
be done before scsi_remove_host().

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..bd4d418 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1657,11 +1657,11 @@ EXPORT_SYMBOL_GPL(ufshcd_resume);
  */
 void ufshcd_remove(struct ufs_hba *hba)
 {
+   scsi_remove_host(hba-host);
/* disable interrupts */
ufshcd_disable_intr(hba, hba-intr_mask);
ufshcd_hba_stop(hba);
 
-   scsi_remove_host(hba-host);
scsi_host_put(hba-host);
 }
 EXPORT_SYMBOL_GPL(ufshcd_remove);
-- 
1.8.3.1

--
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 v2 1/3] ufshcd-pci: release ioremapped region during removing driver

[Akinobu Mita]

Before commit 2953f850c3b80bdca004967c83733365d8aa0aa2 ([SCSI] ufs:
use devres functions for ufshcd), UFSHCI register was ioremapped by
each glue-driver (ufshcd-pltfrm and ufshcd-pci) during probing and it
was iounmapped by core-driver during removing driver.  The commit
converted ufshcd-pltfrm to use devres functions, but it didn't convert
ufshcd-pci.

Therefore, the change causes ufshcd-pci driver not to iounmap UFSHCI
register region during removing driver.  This fixes it by converting
ufshcd-pci to use devres functions.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Seungwon Jeon tgih@samsung.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd-pci.c | 37 +
 1 file changed, 9 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 48be39a..de4c52b 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -93,10 +93,7 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
 
disable_irq(pdev-irq);
ufshcd_remove(hba);
-   pci_release_regions(pdev);
pci_set_drvdata(pdev, NULL);
-   pci_clear_master(pdev);
-   pci_disable_device(pdev);
 }
 
 /**
@@ -133,53 +130,37 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
void __iomem *mmio_base;
int err;
 
-   err = pci_enable_device(pdev);
+   err = pcim_enable_device(pdev);
if (err) {
-   dev_err(pdev-dev, pci_enable_device failed\n);
-   goto out_error;
+   dev_err(pdev-dev, pcim_enable_device failed\n);
+   return err;
}
 
pci_set_master(pdev);
 
-
-   err = pci_request_regions(pdev, UFSHCD);
+   err = pcim_iomap_regions(pdev, 1  0, UFSHCD);
if (err  0) {
-   dev_err(pdev-dev, request regions failed\n);
-   goto out_disable;
+   dev_err(pdev-dev, request and iomap failed\n);
+   return err;
}
 
-   mmio_base = pci_ioremap_bar(pdev, 0);
-   if (!mmio_base) {
-   dev_err(pdev-dev, memory map failed\n);
-   err = -ENOMEM;
-   goto out_release_regions;
-   }
+   mmio_base = pcim_iomap_table(pdev)[0];
 
err = ufshcd_set_dma_mask(pdev);
if (err) {
dev_err(pdev-dev, set dma mask failed\n);
-   goto out_iounmap;
+   return err;
}
 
err = ufshcd_init(pdev-dev, hba, mmio_base, pdev-irq);
if (err) {
dev_err(pdev-dev, Initialization failed\n);
-   goto out_iounmap;
+   return err;
}
 
pci_set_drvdata(pdev, hba);
 
return 0;
-
-out_iounmap:
-   iounmap(mmio_base);
-out_release_regions:
-   pci_release_regions(pdev);
-out_disable:
-   pci_clear_master(pdev);
-   pci_disable_device(pdev);
-out_error:
-   return err;
 }
 
 static DEFINE_PCI_DEVICE_TABLE(ufshcd_pci_tbl) = {
-- 
1.8.3.1

--
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 v2 0/3] ufs: fix bugs in probing and removing driver paths

[Akinobu Mita]

The changes in this patch set are almost same as the previous one
that I send on Jul 8.  But the previous version wasn't cleanly applied
to the upstream kernel because it depend on my local work in progress
patches.  So this version fixes it and it also includes another bug fix
in the same area.

Akinobu Mita (3):
  ufshcd-pci: release ioremapped region during removing driver
  ufs: don't disable_irq() if the IRQ can be shared among devices
  ufs: don't stop controller before scsi_remove_host()

 drivers/scsi/ufs/ufshcd-pci.c| 38 +-
 drivers/scsi/ufs/ufshcd-pltfrm.c |  1 -
 drivers/scsi/ufs/ufshcd.c|  2 +-
 3 files changed, 10 insertions(+), 31 deletions(-)

Cc: Seungwon Jeon tgih@samsung.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org

-- 
1.8.3.1

--
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 v2] [SCSI] scsi_debug: silence GCC warning

[Akinobu Mita]

2013/7/16 Paul Bolle pebo...@tiscali.nl:
 Building scsi_debug.o triggers a GCC warning:
 drivers/scsi/scsi_debug.c: In function ‘dif_verify’:
 drivers/scsi/scsi_debug.c:1755:3: warning: ‘csum’ may be used 
 uninitialized in this function [-Wmaybe-uninitialized]

 This is a false positive. But if we transform the switch statement in
 dif_compute_csum() to a straightforward if/else statement we supply GCC
 with enough information to determine that csum will not be used
 uninitialized.

 Signed-off-by: Paul Bolle pebo...@tiscali.nl
 ---
 0) v2 because I started to worry whether v1 would change an interface
 (ie, the way the guard module parameter behaves). This patch is much
 simpler, and doesn't change any behavior.

 1) Still only compile tested.

This one looks good to me.  It would be much better if this commit
log had a reference to the commit that introduced this warning as
you described after '---' in v1 patch.

  drivers/scsi/scsi_debug.c | 9 +++--
  1 file changed, 3 insertions(+), 6 deletions(-)

 diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
 index cb4fefa..7565ec5 100644
 --- a/drivers/scsi/scsi_debug.c
 +++ b/drivers/scsi/scsi_debug.c
 @@ -1735,14 +1735,11 @@ static u16 dif_compute_csum(const void *buf, int len)
  {
 u16 csum;

 -   switch (scsi_debug_guard) {
 -   case 1:
 +   if (scsi_debug_guard == 1)
 csum = ip_compute_csum(buf, len);
 -   break;
 -   case 0:
 +   else
 csum = cpu_to_be16(crc_t10dif(buf, len));
 -   break;
 -   }
 +
 return csum;
  }

 --
 1.8.1.4

 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
--
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 v2] [SCSI] scsi_debug: silence GCC warning

[Akinobu Mita]

2013/7/17 Paul Bolle pebo...@tiscali.nl:
 On Wed, 2013-07-17 at 00:21 +0900, Akinobu Mita wrote:
 This one looks good to me.

 Thanks.

 It would be much better if this commit
 log had a reference to the commit that introduced this warning as
 you described after '---' in v1 patch.

 Do you mean that I should submit a v3, with a commit explanation that
 also has one or two lines that mention commit beb40ea42b ([SCSI]
 scsi_debug: reduce duplication between prot_verify_read and
 prot_verify_write)?

Yes.  I think it will help someone to understand the background of this patch.
--
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 3/4] scsi_debug: fix WRITE_SAME with virtual_gb 0

[Akinobu Mita]

With module parameter virtual_gb  0, the device accesses may go beyond
the actual ramdisk storage (fake_storep).  Such requests should be
treated as fake_storep is repeatedly mirrored up to virtual size
(virtual_gb * 1GB).

Unfortunately, WRITE_SAME commands with such requests access out of
fake_storep region.  For writing to the first LBA, this fixes it by
switching to use existing do_device_access() which does the correct
conversion of LBA.  For spreading the first LBA over the remaining
blocks, this fixes it by using newly introduced fake_store() for getting
valid address which is corresponding to a given LBA in fake_storep.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 01c0ffa..1e25c1e 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -257,7 +257,7 @@ struct sdebug_queued_cmd {
 };
 static struct sdebug_queued_cmd queued_arr[SCSI_DEBUG_CANQUEUE];
 
-static unsigned char * fake_storep;/* ramdisk storage */
+static void *fake_storep;  /* ramdisk storage */
 static struct sd_dif_tuple *dif_storep;/* protection info */
 static void *map_storep;   /* provisioning map */
 
@@ -293,6 +293,13 @@ static unsigned char ctrl_m_pg[] = {0xa, 10, 2, 0, 0, 0, 
0, 0,
 static unsigned char iec_m_pg[] = {0x1c, 0xa, 0x08, 0, 0, 0, 0, 0,
   0, 0, 0x0, 0x0};
 
+static void *fake_store(unsigned long long lba)
+{
+   lba = do_div(lba, sdebug_store_sectors);
+
+   return fake_storep + lba * scsi_debug_sector_size;
+}
+
 static int sdebug_add_adapter(void);
 static void sdebug_remove_adapter(void);
 
@@ -2131,9 +2138,7 @@ static int resp_write_same(struct scsi_cmnd *scmd, 
unsigned long long lba,
}
 
/* Else fetch one logical block */
-   ret = fetch_to_dev_buffer(scmd,
- fake_storep + (lba * scsi_debug_sector_size),
- scsi_debug_sector_size);
+   ret = do_device_access(scmd, devip, lba, 1, 1);
 
if (-1 == ret) {
write_unlock_irqrestore(atomic_rw, iflags);
@@ -2145,8 +2150,7 @@ static int resp_write_same(struct scsi_cmnd *scmd, 
unsigned long long lba,
 
/* Copy first sector to remaining blocks */
for (i = 1 ; i  num ; i++)
-   memcpy(fake_storep + ((lba + i) * scsi_debug_sector_size),
-  fake_storep + (lba * scsi_debug_sector_size),
+   memcpy(fake_store(lba + i), fake_store(lba),
   scsi_debug_sector_size);
 
if (scsi_debug_lbp())
-- 
1.8.3.1

--
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 0/4] scsi_debug: fix bugs with certain module parameters

[Akinobu Mita]

This patch set includes bug fixes with certain module parameters of
scsi_debug.  First one fixes bug with num_parts  0.  Others fix logical
block provisioning support with unmap_alignment != 0 and with virtual_gb  0.

Akinobu Mita (4):
  scsi_debug: fix endianness bug in sdebug_build_parts()
  scsi_debug: fix logical block provisioning support when
unmap_alignment != 0
  scsi_debug: fix WRITE_SAME with virtual_gb  0
  scsi_debug: fix out of range access by Get_LBA_status with virtual_gb
 0

 drivers/scsi/scsi_debug.c | 41 ++---
 1 file changed, 30 insertions(+), 11 deletions(-)

Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org

-- 
1.8.3.1

--
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/4] scsi_debug: fix endianness bug in sdebug_build_parts()

[Akinobu Mita]

With module parameter num_parts  0, partition table is built on the
ramdisk storage when loading the driver.  Unfortunately, there is an
endianness bug in sdebug_build_parts().  So the partition table is not
correctly initialized on big-endian systems.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..2f39b13 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2659,8 +2659,8 @@ static void __init sdebug_build_parts(unsigned char *ramp,
   / sdebug_sectors_per;
pp-end_sector = (end_sec % sdebug_sectors_per) + 1;
 
-   pp-start_sect = start_sec;
-   pp-nr_sects = end_sec - start_sec + 1;
+   pp-start_sect = cpu_to_le32(start_sec);
+   pp-nr_sects = cpu_to_le32(end_sec - start_sec + 1);
pp-sys_ind = 0x83; /* plain Linux partition */
}
 }
-- 
1.8.3.1

--
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/4] scsi_debug: fix out of range access by Get_LBA_status with virtual_gb 0

[Akinobu Mita]

With logical block provisioning support enabled, the provisioning map
(map_storep) keeps track of the provisioning status (mapped or unmapped)
for actual ramdisk storage range (fake_storep).  The provisioning status
for out of fake_storep range with module parameter virtual_gb  0 is
not tracked, and it should be assumed always mapped.  It is reasonable,
because Unmap commands for such virtual range are always ignored.

Unfortunately, Get_LBA_status command for virtual range accesses out of
map_storep range.  This fixes invalid access and makes it return correct
provisioning status.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 1e25c1e..c519c9f 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2014,6 +2014,7 @@ static sector_t map_index_to_lba(unsigned long index)
return lba;
 }
 
+/* LBA from sdebug_store_sectors to sdebug_capacity is assumed mapped */
 static unsigned int map_state(sector_t lba, unsigned int *num)
 {
sector_t end;
@@ -2022,6 +2023,10 @@ static unsigned int map_state(sector_t lba, unsigned int 
*num)
unsigned long next;
 
index = lba_to_map_index(lba);
+   if (index = map_size) {
+   *num = sdebug_capacity - lba;
+   return 1;
+   }
mapped = test_bit(index, map_storep);
 
if (mapped)
@@ -2029,7 +2034,11 @@ static unsigned int map_state(sector_t lba, unsigned int 
*num)
else
next = find_next_bit(map_storep, map_size, index);
 
-   end = min_t(sector_t, sdebug_store_sectors,  map_index_to_lba(next));
+   if (next = map_size)
+   end = mapped ? sdebug_capacity : sdebug_store_sectors;
+   else
+   end = map_index_to_lba(next);
+
*num = end - lba;
 
return mapped;
-- 
1.8.3.1

--
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 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0

[Akinobu Mita]

Commit b90ebc3d5c41c9164ae04efd2e4f8204c2a186f1 ([SCSI] scsi_debug:
fix logical block provisioning support) fixed several issues with
logical block provisioning support, but it still doesn't properly fix
the cases when unmap_alignment  0.  (for example, unmap_alignment=1
and unmap_granularity=3)

The problem is in map_index_to_lba(), which should return the first
LBA which is corresponding to a given index of provisioning map
(map_storep).

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 2f39b13..01c0ffa 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1997,8 +1997,14 @@ static unsigned long lba_to_map_index(sector_t lba)
 
 static sector_t map_index_to_lba(unsigned long index)
 {
-   return index * scsi_debug_unmap_granularity -
-   scsi_debug_unmap_alignment;
+   sector_t lba = index * scsi_debug_unmap_granularity;
+
+   if (scsi_debug_unmap_alignment) {
+   lba -= scsi_debug_unmap_granularity -
+   scsi_debug_unmap_alignment;
+   }
+
+   return lba;
 }
 
 static unsigned int map_state(sector_t lba, unsigned int *num)
-- 
1.8.3.1

--
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 2/2] ufs: don't stop controller before scsi_remove_host()

[Akinobu Mita]

scsi_remove_host() sends SYNCHRONIZE CACHE commands for write cache
enabled scsi disk devices.  So stopping controller working shouldn't
be done before scsi_remove_host().

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd-pci.c| 1 -
 drivers/scsi/ufs/ufshcd-pltfrm.c | 1 -
 drivers/scsi/ufs/ufshcd.c| 3 ++-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index c34efb9..91a2e79 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -91,7 +91,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
 {
struct ufs_hba *hba = pci_get_drvdata(pdev);
 
-   disable_irq(pdev-irq);
ufshcd_remove(hba);
pci_set_drvdata(pdev, NULL);
 }
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index c42db40..94ba40c 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -144,7 +144,6 @@ static int ufshcd_pltfrm_remove(struct platform_device 
*pdev)
 {
struct ufs_hba *hba =  platform_get_drvdata(pdev);
 
-   disable_irq(hba-irq);
ufshcd_remove(hba);
return 0;
 }
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index cfa9633..f2832b8 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1657,11 +1657,12 @@ EXPORT_SYMBOL_GPL(ufshcd_resume);
  */
 void ufshcd_remove(struct ufs_hba *hba)
 {
+   scsi_remove_host(hba-host);
/* disable interrupts */
+   disable_irq(hba-irq);
ufshcd_disable_intr(hba, hba-intr_mask);
ufshcd_hba_stop(hba);
 
-   scsi_remove_host(hba-host);
scsi_host_put(hba-host);
 }
 EXPORT_SYMBOL_GPL(ufshcd_remove);
-- 
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

[PATCH 1/2] ufshcd-pci: release ioremapped region during removing driver

[Akinobu Mita]

Before commit 2953f850c3b80bdca004967c83733365d8aa0aa2 ([SCSI] ufs:
use devres functions for ufshcd), UFSHCI register was ioremapped by
each glue-driver (ufshcd-pltfrm and ufshcd-pci) during probing and it
was iounmapped by core-driver during removing driver.  The commit
converted ufshcd-pltfrm to use devres functions, but it didn't convert
ufshcd-pci.

Therefore, the change causes ufshcd-pci driver not to iounmap UFSHCI
register region during removing driver.  This fixes it by converting
ufshcd-pci to use devres functions.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Seungwon Jeon tgih@samsung.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd-pci.c | 35 ---
 1 file changed, 8 insertions(+), 27 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 64d36eb..c34efb9 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -93,10 +93,7 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
 
disable_irq(pdev-irq);
ufshcd_remove(hba);
-   pci_release_regions(pdev);
pci_set_drvdata(pdev, NULL);
-   pci_clear_master(pdev);
-   pci_disable_device(pdev);
 }
 
 /**
@@ -113,47 +110,31 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
void __iomem *mmio_base;
int err;
 
-   err = pci_enable_device(pdev);
+   err = pcim_enable_device(pdev);
if (err) {
-   dev_err(pdev-dev, pci_enable_device failed\n);
-   goto out_error;
+   dev_err(pdev-dev, pcim_enable_device failed\n);
+   return err;
}
 
pci_set_master(pdev);
 
-
-   err = pci_request_regions(pdev, UFSHCD);
+   err = pcim_iomap_regions(pdev, 1  0, UFSHCD);
if (err  0) {
-   dev_err(pdev-dev, request regions failed\n);
-   goto out_disable;
+   dev_err(pdev-dev, request and iomap failed\n);
+   return err;
}
 
-   mmio_base = pci_ioremap_bar(pdev, 0);
-   if (!mmio_base) {
-   dev_err(pdev-dev, memory map failed\n);
-   err = -ENOMEM;
-   goto out_release_regions;
-   }
+   mmio_base = pcim_iomap_table(pdev)[0];
 
err = ufshcd_init(pdev-dev, hba, mmio_base, pdev-irq);
if (err) {
dev_err(pdev-dev, Initialization failed\n);
-   goto out_iounmap;
+   return err;
}
 
pci_set_drvdata(pdev, hba);
 
return 0;
-
-out_iounmap:
-   iounmap(mmio_base);
-out_release_regions:
-   pci_release_regions(pdev);
-out_disable:
-   pci_clear_master(pdev);
-   pci_disable_device(pdev);
-out_error:
-   return err;
 }
 
 static DEFINE_PCI_DEVICE_TABLE(ufshcd_pci_tbl) = {
-- 
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

[PATCH v4 6/6] scsi_debug: reduce duplication between prot_verify_read and prot_verify_write

[Akinobu Mita]

In order to reduce code duplication between prot_verify_read() and
prot_verify_write(), this moves common code into the new functions.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Douglas Gilbert dgilb...@interlog.com
Acked-by: Martin K. Petersen martin.peter...@oracle.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 139 ++
 1 file changed, 54 insertions(+), 85 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index fc8b3aa..9790d56 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1710,6 +1710,52 @@ static int do_device_access(struct scsi_cmnd *scmd,
return ret;
 }
 
+static u16 dif_compute_csum(const void *buf, int len)
+{
+   u16 csum;
+
+   switch (scsi_debug_guard) {
+   case 1:
+   csum = ip_compute_csum(buf, len);
+   break;
+   case 0:
+   csum = cpu_to_be16(crc_t10dif(buf, len));
+   break;
+   default:
+   BUG();
+   }
+   return csum;
+}
+
+static int dif_verify(struct sd_dif_tuple *sdt, const void *data,
+ sector_t sector, u32 ei_lba)
+{
+   u16 csum = dif_compute_csum(data, scsi_debug_sector_size);
+
+   if (sdt-guard_tag != csum) {
+   pr_err(%s: GUARD check failed on sector %lu rcvd 0x%04x, data 
0x%04x\n,
+   __func__,
+   (unsigned long)sector,
+   be16_to_cpu(sdt-guard_tag),
+   be16_to_cpu(csum));
+   return 0x01;
+   }
+   if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION 
+   be32_to_cpu(sdt-ref_tag) != (sector  0x)) {
+   pr_err(%s: REF check failed on sector %lu\n,
+   __func__, (unsigned long)sector);
+   return 0x03;
+   }
+   if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION 
+   be32_to_cpu(sdt-ref_tag) != ei_lba) {
+   pr_err(%s: REF check failed on sector %lu\n,
+   __func__, (unsigned long)sector);
+   dif_errors++;
+   return 0x03;
+   }
+   return 0;
+}
+
 static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
unsigned int sectors, u32 ei_lba)
 {
@@ -1725,53 +1771,19 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
sdt = dif_storep + start_sec;
 
for (i = 0 ; i  sectors ; i++) {
-   u16 csum;
+   int ret;
 
if (sdt[i].app_tag == 0x)
continue;
 
sector = start_sec + i;
 
-   switch (scsi_debug_guard) {
-   case 1:
-   csum = ip_compute_csum(fake_storep +
-  sector * scsi_debug_sector_size,
-  scsi_debug_sector_size);
-   break;
-   case 0:
-   csum = crc_t10dif(fake_storep +
- sector * scsi_debug_sector_size,
- scsi_debug_sector_size);
-   csum = cpu_to_be16(csum);
-   break;
-   default:
-   BUG();
-   }
-
-   if (sdt[i].guard_tag != csum) {
-   printk(KERN_ERR %s: GUARD check failed on sector %lu \
-   rcvd 0x%04x, data 0x%04x\n, __func__,
-  (unsigned long)sector,
-  be16_to_cpu(sdt[i].guard_tag),
-  be16_to_cpu(csum));
+   ret = dif_verify(sdt[i],
+fake_storep + sector * scsi_debug_sector_size,
+sector, ei_lba);
+   if (ret) {
dif_errors++;
-   return 0x01;
-   }
-
-   if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION 
-   be32_to_cpu(sdt[i].ref_tag) != (sector  0x)) {
-   printk(KERN_ERR %s: REF check failed on sector %lu\n,
-  __func__, (unsigned long)sector);
-   dif_errors++;
-   return 0x03;
-   }
-
-   if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION 
-   be32_to_cpu(sdt[i].ref_tag) != ei_lba) {
-   printk(KERN_ERR %s: REF check failed on sector %lu\n,
-  __func__, (unsigned long)sector);
-   dif_errors++;
-   return 0x03;
+   return ret

[PATCH v4 1/6] scsi_debug: fix invalid address passed to kunmap_atomic()

[Akinobu Mita]

In the function prot_verify_write(), the kmap address 'daddr' is
incremented in the loop for each data page.  Finally 'daddr' reaches
the next page boundary in the end of the loop, and the invalid address
is passed to kunmap_atomic().

Fix the issue by not incrementing 'daddr' in the loop and offsetting it
by the loop counter on demand.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Douglas Gilbert dgilb...@interlog.com
Acked-by: Martin K. Petersen martin.peter...@oracle.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 0a537a0..d51bddd 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1899,7 +1899,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
daddr = kmap_atomic(sg_page(dsgl)) + dsgl-offset;
 
/* For each sector-sized chunk in data page */
-   for (j = 0 ; j  dsgl-length ; j += scsi_debug_sector_size) {
+   for (j = 0; j  dsgl-length; j += scsi_debug_sector_size) {
 
/* If we're at the end of the current
 * protection page advance to the next one
@@ -1917,11 +1917,11 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
switch (scsi_debug_guard) {
case 1:
-   csum = ip_compute_csum(daddr,
+   csum = ip_compute_csum(daddr + j,
   scsi_debug_sector_size);
break;
case 0:
-   csum = cpu_to_be16(crc_t10dif(daddr,
+   csum = cpu_to_be16(crc_t10dif(daddr + j,
  scsi_debug_sector_size));
break;
default:
@@ -1938,7 +1938,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
   be16_to_cpu(sdt-guard_tag),
   be16_to_cpu(csum));
ret = 0x01;
-   dump_sector(daddr, scsi_debug_sector_size);
+   dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
@@ -1949,7 +1949,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
   %s: REF check failed on sector %lu\n,
   __func__, (unsigned long)sector);
ret = 0x03;
-   dump_sector(daddr, scsi_debug_sector_size);
+   dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
@@ -1959,7 +1959,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
   %s: REF check failed on sector %lu\n,
   __func__, (unsigned long)sector);
ret = 0x03;
-   dump_sector(daddr, scsi_debug_sector_size);
+   dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
@@ -1977,7 +1977,6 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
start_sec++;
ei_lba++;
-   daddr += scsi_debug_sector_size;
ppage_offset += sizeof(struct sd_dif_tuple);
}
 
-- 
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

[PATCH v4 3/6] scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1

[Akinobu Mita]

The protection info dif_storep is allocated only when parameter dif is
not zero.  But it will be accessed when reading or writing to the storage
installed with parameter dix is not zero.

So kernel crashes if scsi_debug module is loaded with parameters dix=1 and
dif=0.

This fixes it by making dif_storep available if parameter dix is not zero
instead of checking if parameter dif is not zero.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Douglas Gilbert dgilb...@interlog.com
Acked-by: Martin K. Petersen martin.peter...@oracle.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index bcf73e4..e83e661 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3372,7 +3372,7 @@ static int __init scsi_debug_init(void)
if (scsi_debug_num_parts  0)
sdebug_build_parts(fake_storep, sz);
 
-   if (scsi_debug_dif) {
+   if (scsi_debug_dix) {
int dif_size;
 
dif_size = sdebug_store_sectors * sizeof(struct sd_dif_tuple);
-- 
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

[PATCH v4 4/6] scsi_debug: invalidate protection info for unmapped region

[Akinobu Mita]

When UNMAP command is issued with the data integrity support enabled,
the protection info for the unmapped region is remain unchanged.
So READ command for the region later on causes data integrity failure.

This fixes it by invalidating protection info for the unmapped region
by filling with 0xff pattern.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Douglas Gilbert dgilb...@interlog.com
Acked-by: Martin K. Petersen martin.peter...@oracle.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index e83e661..83efec2 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2064,6 +2064,11 @@ static void unmap_region(sector_t lba, unsigned int len)
   scsi_debug_sector_size *
   scsi_debug_unmap_granularity);
}
+   if (dif_storep) {
+   memset(dif_storep + lba, 0xff,
+  sizeof(*dif_storep) *
+  scsi_debug_unmap_granularity);
+   }
}
lba = map_index_to_lba(index + 1);
}
-- 
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

[PATCH v4 2/6] scsi_debug: fix incorrectly nested kmap_atomic()

[Akinobu Mita]

In the function prot_verify_write(), kmap_atomic()/kunmap_atomic() for
data page and kmap_atomic()/kunmap_atomic() for protection information
page are not nested each other.

It worked perfectly before commit 3e4d3af501cccdc8a8cca41bdbe57d54ad7e7e73
(mm: stack based kmap_atomic()).  Because the kmap_atomic slot KM_IRQ0
was used for data page and the slot KM_IRQ1 was used for protection page.

But KM_types are gone and kmap_atomic() is using stack based implementation.
So two different kmap_atomic() usages must be strictly nested now.

This change ensures kmap_atomic() usage is strictly nested.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Douglas Gilbert dgilb...@interlog.com
Acked-by: Martin K. Petersen martin.peter...@oracle.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d51bddd..bcf73e4 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1891,12 +1891,12 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
BUG_ON(scsi_sg_count(SCpnt) == 0);
BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
 
-   paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
ppage_offset = 0;
 
/* For each data page */
scsi_for_each_sg(SCpnt, dsgl, scsi_sg_count(SCpnt), i) {
daddr = kmap_atomic(sg_page(dsgl)) + dsgl-offset;
+   paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
 
/* For each sector-sized chunk in data page */
for (j = 0; j  dsgl-length; j += scsi_debug_sector_size) {
@@ -1980,19 +1980,18 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ppage_offset += sizeof(struct sd_dif_tuple);
}
 
+   kunmap_atomic(paddr);
kunmap_atomic(daddr);
}
 
-   kunmap_atomic(paddr);
-
dix_writes++;
 
return 0;
 
 out:
dif_errors++;
-   kunmap_atomic(daddr);
kunmap_atomic(paddr);
+   kunmap_atomic(daddr);
return ret;
 }
 
-- 
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

[PATCH v3 0/4] introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()

[Akinobu Mita]

This patch set introduces sg_pcopy_from_buffer() and sg_pcopy_to_buffer(),
which copy data between a linear buffer and an SG list.

The only difference between sg_pcopy_{from,to}_buffer() and
sg_copy_{from,to}_buffer() is an additional argument that specifies
the number of bytes to skip the SG list before copying.

The main reason for introducing these functions is to fix a problem
in scsi_debug module.  And there is a local function in crypto/talitos
module, which can be replaced by sg_pcopy_to_buffer().

* Changes from v2
- Add Acked-by line
- Rename sg_miter_seek() to sg_miter_skip()
- Remove the restriction on when sg_miter_skip() can be called

* Changes from v1
- Separate the change that factors out sg_miter_get_next_page() from
  the patch introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()
- Add function comment for internal function sg_miter_seek()
- Simplify the assignment of sdb-resid in fill_from_dev_buffer() in
  scsi_debug

Akinobu Mita (4):
  lib/scatterlist: factor out sg_miter_get_next_page() from
sg_miter_next()
  lib/scatterlist: introduce sg_pcopy_from_buffer() and
sg_pcopy_to_buffer()
  crypto: talitos: use sg_pcopy_to_buffer()
  scsi_debug: fix do_device_access() with wrap around range

 drivers/crypto/talitos.c|  60 +
 drivers/scsi/scsi_debug.c   |  48 +
 include/linux/scatterlist.h |   5 ++
 lib/scatterlist.c   | 127 +---
 4 files changed, 150 insertions(+), 90 deletions(-)

Cc: Tejun Heo t...@kernel.org
Cc: Imre Deak imre.d...@intel.com
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Cc: linux-cry...@vger.kernel.org
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org

-- 
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

[PATCH v3 2/4] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()

[Akinobu Mita]

The only difference between sg_pcopy_{from,to}_buffer() and
sg_copy_{from,to}_buffer() is an additional argument that specifies
the number of bytes to skip the SG list before copying.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Tejun Heo t...@kernel.org
Cc: Imre Deak imre.d...@intel.com
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Cc: linux-cry...@vger.kernel.org
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---

* Changes from v2
- Rename sg_miter_seek() to sg_miter_skip()
- Remove the restriction on when sg_miter_skip() can be called

 include/linux/scatterlist.h |  5 +++
 lib/scatterlist.c   | 88 ++---
 2 files changed, 88 insertions(+), 5 deletions(-)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 2680677..adae88f 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -244,6 +244,11 @@ size_t sg_copy_from_buffer(struct scatterlist *sgl, 
unsigned int nents,
 size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents,
 void *buf, size_t buflen);
 
+size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents,
+   void *buf, size_t buflen, off_t skip);
+size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
+ void *buf, size_t buflen, off_t skip);
+
 /*
  * Maximum number of entries that will be allocated in one piece, if
  * a list larger than this is required then chaining will be utilized.
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index c35b929..129a82f 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -476,6 +476,43 @@ static bool sg_miter_get_next_page(struct sg_mapping_iter 
*miter)
 }
 
 /**
+ * sg_miter_skip - reposition mapping iterator
+ * @miter: sg mapping iter to be skipped
+ * @offset: number of bytes to plus the current location
+ *
+ * Description:
+ *   Sets the offset of @miter to its current location plus @offset bytes.
+ *   If mapping iterator @miter has been proceeded by sg_miter_next(), this
+ *   stops @miter.
+ *
+ * Context:
+ *   Don't care if @miter is stopped, or not proceeded yet.
+ *   Otherwise, preemption disabled if the SG_MITER_ATOMIC is set.
+ *
+ * Returns:
+ *   true if @miter contains the valid mapping.  false if end of sg
+ *   list is reached.
+ */
+static bool sg_miter_skip(struct sg_mapping_iter *miter, off_t offset)
+{
+   sg_miter_stop(miter);
+
+   while (offset) {
+   off_t consumed;
+
+   if (!sg_miter_get_next_page(miter))
+   return false;
+
+   consumed = min_t(off_t, offset, miter-__remaining);
+   miter-__offset += consumed;
+   miter-__remaining -= consumed;
+   offset -= consumed;
+   }
+
+   return true;
+}
+
+/**
  * sg_miter_next - proceed mapping iterator to the next mapping
  * @miter: sg mapping iter to proceed
  *
@@ -561,14 +598,16 @@ EXPORT_SYMBOL(sg_miter_stop);
  * @nents:  Number of SG entries
  * @buf:Where to copy from
  * @buflen: The number of bytes to copy
- * @to_buffer:  transfer direction (non zero == from an sg 
list to a
- *  buffer, 0 == from a buffer to an sg list
+ * @skip:   Number of bytes to skip before copying
+ * @to_buffer:  transfer direction (true == from an sg list to a
+ *  buffer, false == from a buffer to an sg list
  *
  * Returns the number of copied bytes.
  *
  **/
 static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents,
-void *buf, size_t buflen, int to_buffer)
+void *buf, size_t buflen, off_t skip,
+bool to_buffer)
 {
unsigned int offset = 0;
struct sg_mapping_iter miter;
@@ -582,6 +621,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, 
unsigned int nents,
 
sg_miter_start(miter, sgl, nents, sg_flags);
 
+   if (!sg_miter_skip(miter, skip))
+   return false;
+
local_irq_save(flags);
 
while (sg_miter_next(miter)  offset  buflen) {
@@ -616,7 +658,7 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, 
unsigned int nents,
 size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents,
   void *buf, size_t buflen)
 {
-   return sg_copy_buffer(sgl, nents, buf, buflen, 0);
+   return sg_copy_buffer(sgl, nents, buf, buflen, 0, false);
 }
 EXPORT_SYMBOL(sg_copy_from_buffer);
 
@@ -633,6 +675,42 @@ EXPORT_SYMBOL(sg_copy_from_buffer);
 size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents,
 void *buf, size_t buflen)
 {
-   return sg_copy_buffer(sgl, nents, buf, buflen, 1);
+   return

[PATCH v3 1/4] lib/scatterlist: factor out sg_miter_get_next_page() from sg_miter_next()

[Akinobu Mita]

This function is used to proceed page iterator to the next page if
necessary, and will be used to implement the variants of
sg_copy_{from,to}_buffer() later.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Acked-by: Tejun Heo t...@kernel.org
Cc: Tejun Heo t...@kernel.org
Cc: Imre Deak imre.d...@intel.com
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Cc: linux-cry...@vger.kernel.org
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---

* Change from v2
- Add Acked-by line

 lib/scatterlist.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index a1cf8ca..c35b929 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -453,6 +453,28 @@ void sg_miter_start(struct sg_mapping_iter *miter, struct 
scatterlist *sgl,
 }
 EXPORT_SYMBOL(sg_miter_start);
 
+static bool sg_miter_get_next_page(struct sg_mapping_iter *miter)
+{
+   if (!miter-__remaining) {
+   struct scatterlist *sg;
+   unsigned long pgoffset;
+
+   if (!__sg_page_iter_next(miter-piter))
+   return false;
+
+   sg = miter-piter.sg;
+   pgoffset = miter-piter.sg_pgoffset;
+
+   miter-__offset = pgoffset ? 0 : sg-offset;
+   miter-__remaining = sg-offset + sg-length -
+   (pgoffset  PAGE_SHIFT) - miter-__offset;
+   miter-__remaining = min_t(unsigned long, miter-__remaining,
+  PAGE_SIZE - miter-__offset);
+   }
+
+   return true;
+}
+
 /**
  * sg_miter_next - proceed mapping iterator to the next mapping
  * @miter: sg mapping iter to proceed
@@ -478,22 +500,9 @@ bool sg_miter_next(struct sg_mapping_iter *miter)
 * Get to the next page if necessary.
 * __remaining, __offset is adjusted by sg_miter_stop
 */
-   if (!miter-__remaining) {
-   struct scatterlist *sg;
-   unsigned long pgoffset;
-
-   if (!__sg_page_iter_next(miter-piter))
-   return false;
-
-   sg = miter-piter.sg;
-   pgoffset = miter-piter.sg_pgoffset;
+   if (!sg_miter_get_next_page(miter))
+   return false;
 
-   miter-__offset = pgoffset ? 0 : sg-offset;
-   miter-__remaining = sg-offset + sg-length -
-   (pgoffset  PAGE_SHIFT) - miter-__offset;
-   miter-__remaining = min_t(unsigned long, miter-__remaining,
-  PAGE_SIZE - miter-__offset);
-   }
miter-page = sg_page_iter_page(miter-piter);
miter-consumed = miter-length = miter-__remaining;
 
-- 
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

[PATCH v2 0/4] introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()

[Akinobu Mita]

This patch set introduces sg_pcopy_from_buffer() and sg_pcopy_to_buffer(),
which copy data between a linear buffer and an SG list.

The only difference between sg_pcopy_{from,to}_buffer() and
sg_copy_{from,to}_buffer() is an additional argument that specifies
the number of bytes to skip the SG list before copying.

The main reason for introducing these functions is to fix a problem
in scsi_debug module.  And there is a local function in crypto/talitos
module, which can be replaced by sg_pcopy_to_buffer().

* Changes from v1
- Separate the change that factors out sg_miter_get_next_page() from
  the patch introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()
- Add function comment for internal function sg_miter_seek()
- Simplify the assignment of sdb-resid in fill_from_dev_buffer() in
  scsi_debug

Akinobu Mita (4):
  lib/scatterlist: factor out sg_miter_get_next_page() from
sg_miter_next()
  lib/scatterlist: introduce sg_pcopy_from_buffer() and
sg_pcopy_to_buffer()
  crypto: talitos: use sg_pcopy_to_buffer()
  scsi_debug: fix do_device_access() with wrap around range

 drivers/crypto/talitos.c|  60 +
 drivers/scsi/scsi_debug.c   |  48 +
 include/linux/scatterlist.h |   5 ++
 lib/scatterlist.c   | 127 +---
 4 files changed, 150 insertions(+), 90 deletions(-)

Cc: Tejun Heo t...@kernel.org
Cc: Imre Deak imre.d...@intel.com
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Cc: linux-cry...@vger.kernel.org
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org

-- 
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

[PATCH v2 1/4] lib/scatterlist: factor out sg_miter_get_next_page() from sg_miter_next()

[Akinobu Mita]

This function is used to proceed page iterator to the next page if
necessary, and will be used to implement the variants of
sg_copy_{from,to}_buffer() later.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Tejun Heo t...@kernel.org
Cc: Imre Deak imre.d...@intel.com
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Cc: linux-cry...@vger.kernel.org
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---

* New patch from v2

 lib/scatterlist.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index a1cf8ca..c35b929 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -453,6 +453,28 @@ void sg_miter_start(struct sg_mapping_iter *miter, struct 
scatterlist *sgl,
 }
 EXPORT_SYMBOL(sg_miter_start);
 
+static bool sg_miter_get_next_page(struct sg_mapping_iter *miter)
+{
+   if (!miter-__remaining) {
+   struct scatterlist *sg;
+   unsigned long pgoffset;
+
+   if (!__sg_page_iter_next(miter-piter))
+   return false;
+
+   sg = miter-piter.sg;
+   pgoffset = miter-piter.sg_pgoffset;
+
+   miter-__offset = pgoffset ? 0 : sg-offset;
+   miter-__remaining = sg-offset + sg-length -
+   (pgoffset  PAGE_SHIFT) - miter-__offset;
+   miter-__remaining = min_t(unsigned long, miter-__remaining,
+  PAGE_SIZE - miter-__offset);
+   }
+
+   return true;
+}
+
 /**
  * sg_miter_next - proceed mapping iterator to the next mapping
  * @miter: sg mapping iter to proceed
@@ -478,22 +500,9 @@ bool sg_miter_next(struct sg_mapping_iter *miter)
 * Get to the next page if necessary.
 * __remaining, __offset is adjusted by sg_miter_stop
 */
-   if (!miter-__remaining) {
-   struct scatterlist *sg;
-   unsigned long pgoffset;
-
-   if (!__sg_page_iter_next(miter-piter))
-   return false;
-
-   sg = miter-piter.sg;
-   pgoffset = miter-piter.sg_pgoffset;
+   if (!sg_miter_get_next_page(miter))
+   return false;
 
-   miter-__offset = pgoffset ? 0 : sg-offset;
-   miter-__remaining = sg-offset + sg-length -
-   (pgoffset  PAGE_SHIFT) - miter-__offset;
-   miter-__remaining = min_t(unsigned long, miter-__remaining,
-  PAGE_SIZE - miter-__offset);
-   }
miter-page = sg_page_iter_page(miter-piter);
miter-consumed = miter-length = miter-__remaining;
 
-- 
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

[PATCH v2 2/4] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()

[Akinobu Mita]

The only difference between sg_pcopy_{from,to}_buffer() and
sg_copy_{from,to}_buffer() is an additional argument that specifies
the number of bytes to skip the SG list before copying.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Tejun Heo t...@kernel.org
Cc: Imre Deak imre.d...@intel.com
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Cc: linux-cry...@vger.kernel.org
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---

* Change from v2
- Separate the change that factors out sg_miter_get_next_page()
- Add function comment for internal function sg_miter_seek()

 include/linux/scatterlist.h |  5 +++
 lib/scatterlist.c   | 88 ++---
 2 files changed, 88 insertions(+), 5 deletions(-)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 2680677..adae88f 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -244,6 +244,11 @@ size_t sg_copy_from_buffer(struct scatterlist *sgl, 
unsigned int nents,
 size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents,
 void *buf, size_t buflen);
 
+size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents,
+   void *buf, size_t buflen, off_t skip);
+size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
+ void *buf, size_t buflen, off_t skip);
+
 /*
  * Maximum number of entries that will be allocated in one piece, if
  * a list larger than this is required then chaining will be utilized.
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index c35b929..f167392 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -476,6 +476,43 @@ static bool sg_miter_get_next_page(struct sg_mapping_iter 
*miter)
 }
 
 /**
+ * sg_miter_seek - reposition mapping iterator
+ * @miter: sg mapping iter to be seeked
+ * @offset: number of bytes to plus the current location
+ *
+ * Description:
+ *   Sets the offset of @miter to its current location plus @offset bytes.
+ *
+ * Notes:
+ *   This function must be called just after sg_miter_start() or 
sg_miter_stop()
+ *
+ * Context:
+ *   Don't care.
+ *
+ * Returns:
+ *   true if @miter contains the valid mapping.  false if end of sg
+ *   list is reached.
+ */
+static bool sg_miter_seek(struct sg_mapping_iter *miter, off_t offset)
+{
+   WARN_ON(miter-addr);
+
+   while (offset) {
+   off_t consumed;
+
+   if (!sg_miter_get_next_page(miter))
+   return false;
+
+   consumed = min_t(off_t, offset, miter-__remaining);
+   miter-__offset += consumed;
+   miter-__remaining -= consumed;
+   offset -= consumed;
+   }
+
+   return true;
+}
+
+/**
  * sg_miter_next - proceed mapping iterator to the next mapping
  * @miter: sg mapping iter to proceed
  *
@@ -561,14 +598,16 @@ EXPORT_SYMBOL(sg_miter_stop);
  * @nents:  Number of SG entries
  * @buf:Where to copy from
  * @buflen: The number of bytes to copy
- * @to_buffer:  transfer direction (non zero == from an sg 
list to a
- *  buffer, 0 == from a buffer to an sg list
+ * @skip:   Number of bytes to skip before copying
+ * @to_buffer:  transfer direction (true == from an sg list to a
+ *  buffer, false == from a buffer to an sg list
  *
  * Returns the number of copied bytes.
  *
  **/
 static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents,
-void *buf, size_t buflen, int to_buffer)
+void *buf, size_t buflen, off_t skip,
+bool to_buffer)
 {
unsigned int offset = 0;
struct sg_mapping_iter miter;
@@ -582,6 +621,9 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, 
unsigned int nents,
 
sg_miter_start(miter, sgl, nents, sg_flags);
 
+   if (!sg_miter_seek(miter, skip))
+   return false;
+
local_irq_save(flags);
 
while (sg_miter_next(miter)  offset  buflen) {
@@ -616,7 +658,7 @@ static size_t sg_copy_buffer(struct scatterlist *sgl, 
unsigned int nents,
 size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents,
   void *buf, size_t buflen)
 {
-   return sg_copy_buffer(sgl, nents, buf, buflen, 0);
+   return sg_copy_buffer(sgl, nents, buf, buflen, 0, false);
 }
 EXPORT_SYMBOL(sg_copy_from_buffer);
 
@@ -633,6 +675,42 @@ EXPORT_SYMBOL(sg_copy_from_buffer);
 size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents,
 void *buf, size_t buflen)
 {
-   return sg_copy_buffer(sgl, nents, buf, buflen, 1);
+   return sg_copy_buffer(sgl, nents, buf, buflen, 0, true);
 }
 EXPORT_SYMBOL(sg_copy_to_buffer

[PATCH 2/2] [SCSI] ufs: fix DMA mask setting

[Akinobu Mita]

If the controller doesn't support 64-bit addressing mode, it must not
set the DMA mask to 64-bit.  But it's unconditionally trying to set to
64-bit without checking 64-bit addressing support in the controller
capabilities.

It was correctly checked before commit 3b1d05807a9a68c6d0580e9248247a774a4d3be6
([SCSI] ufs: Segregate PCI Specific Code), this aims to restores
the correct behaviour.

To achieve this in a generic way, firstly we should push down the DMA
mask setting routine ufshcd_set_dma_mask() from PCI glue driver to core
driver in order to do it for both PCI glue driver and Platform glue
driver.  Secondly, we should change pci_ DMA mapping API to dma_ DMA
mapping API because core driver is independent of glue drivers.

Lastly, we need to relax dma_set_mask(dev, DMA_BIT_MASK(32)) error check
for platform devices on ARM, which do not have a valid dma_mask pointer.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd-pci.c | 26 --
 drivers/scsi/ufs/ufshcd.c | 25 +
 2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 5cb1d75..6835520 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -101,26 +101,6 @@ static void ufshcd_pci_remove(struct pci_dev *pdev)
 }
 
 /**
- * ufshcd_set_dma_mask - Set dma mask based on the controller
- *  addressing capability
- * @pdev: PCI device structure
- *
- * Returns 0 for success, non-zero for failure
- */
-static int ufshcd_set_dma_mask(struct pci_dev *pdev)
-{
-   int err;
-
-   if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(64))
-!pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)))
-   return 0;
-   err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
-   if (!err)
-   err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
-   return err;
-}
-
-/**
  * ufshcd_pci_probe - probe routine of the driver
  * @pdev: pointer to PCI device handle
  * @id: PCI device id
@@ -156,12 +136,6 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
goto out_release_regions;
}
 
-   err = ufshcd_set_dma_mask(pdev);
-   if (err) {
-   dev_err(pdev-dev, set dma mask failed\n);
-   goto out_iounmap;
-   }
-
err = ufshcd_init(pdev-dev, hba, mmio_base, pdev-irq);
if (err) {
dev_err(pdev-dev, Initialization failed\n);
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 492f2b7..7f5399c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1598,6 +1598,25 @@ void ufshcd_remove(struct ufs_hba *hba)
 EXPORT_SYMBOL_GPL(ufshcd_remove);
 
 /**
+ * ufshcd_set_dma_mask - Set dma mask based on the controller
+ *  addressing capability
+ * @hba: per adapter instance
+ *
+ * Returns 0 for success, non-zero for failure
+ */
+static int ufshcd_set_dma_mask(struct ufs_hba *hba)
+{
+   if (hba-capabilities  MASK_64_ADDRESSING_SUPPORT) {
+   if (!dma_set_mask(hba-dev, DMA_BIT_MASK(64)) 
+   !dma_set_coherent_mask(hba-dev, DMA_BIT_MASK(64)))
+   return 0;
+   }
+   dma_set_mask(hba-dev, DMA_BIT_MASK(32));
+
+   return dma_set_coherent_mask(hba-dev, DMA_BIT_MASK(32));
+}
+
+/**
  * ufshcd_init - Driver initialization routine
  * @dev: pointer to device handle
  * @hba_handle: driver private handle
@@ -1645,6 +1664,12 @@ int ufshcd_init(struct device *dev, struct ufs_hba 
**hba_handle,
/* Get UFS version supported by the controller */
hba-ufs_version = ufshcd_get_ufs_version(hba);
 
+   err = ufshcd_set_dma_mask(hba);
+   if (err) {
+   dev_err(hba-dev, set dma mask failed\n);
+   goto out_disable;
+   }
+
/* Allocate memory for host memory space */
err = ufshcd_memory_alloc(hba);
if (err) {
-- 
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

Re: [PATCH 1/3] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()

[Akinobu Mita]

2013/6/6 Imre Deak imre.d...@intel.com:
 Looks ok to me, perhaps adding the seek functionality to the mapping
 iterator would make things more generic and the mapping iterator more
 resemble the page iterator. So we'd have a new sg_miter_start_offset and
 call it here something like:

 sg_miter_start_offset(miter, sgl, nents, sg_flags, skip);

I also thought something like this could be a new interface for sg_miter_* API,
but I haven't found any places where it can be used yet.  That's why I made
it a local function and didn't create new interface.

But putting good function comment like other sg_miter_* API for new
local function
is harmless and it helps someone who wants interface like this in the future.
So I'll do so in next version.
--
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 1/3] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()

[Akinobu Mita]

2013/6/7 Tejun Heo t...@kernel.org:
 Hello,

 On Thu, Jun 06, 2013 at 09:52:56PM +0900, Akinobu Mita wrote:
 +static bool sg_miter_get_next_page(struct sg_mapping_iter *miter)
 +{
 + if (!miter-__remaining) {
 + struct scatterlist *sg;
 + unsigned long pgoffset;
 +
 + if (!__sg_page_iter_next(miter-piter))
 + return false;
 +
 + sg = miter-piter.sg;
 + pgoffset = miter-piter.sg_pgoffset;
 +
 + miter-__offset = pgoffset ? 0 : sg-offset;
 + miter-__remaining = sg-offset + sg-length -
 + (pgoffset  PAGE_SHIFT) - miter-__offset;
 + miter-__remaining = min_t(unsigned long, miter-__remaining,
 +PAGE_SIZE - miter-__offset);
 + }
 +
 + return true;
 +}

 It'd be better if separating out this function is a separate patch.
 Mixing factoring out something and adding new stuff makes the patch a
 bit harder to read.

OK, sounds good. I'll do so in next version.

But I feel sg_miter_get_next_page() is not very appropriate name, because
it gets the next page only if necessary.  If I can find more appropriate name,
I'll rename it.

 +static bool sg_miter_seek(struct sg_mapping_iter *miter, off_t offset)
 +{
 + WARN_ON(miter-addr);
 +
 + while (offset) {
 + off_t consumed;
 +
 + if (!sg_miter_get_next_page(miter))
 + return false;
 +
 + consumed = min_t(off_t, offset, miter-__remaining);
 + miter-__offset += consumed;
 + miter-__remaining -= consumed;
 + offset -= consumed;
 + }
 +
 + return true;
 +}

 While I think the above should work at the beginning, what if @miter
 is in the middle of iteration and __remaining isn't zero?

sg_miter_seek() should work as far as it is called just after sg_miter_start(),
or just after sg_miter_stop() in the middle of iteration (Although I only
tested the former case).  I omitted a function comment in excuse of the static
function, but I should write something I said above.
--
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 v3 0/6] scsi_debug: bug fixes and cleanups for data integrity support

[Akinobu Mita]

2013/6/7 Martin K. Petersen martin.peter...@oracle.com:
 Akinobu == Akinobu Mita akinobu.m...@gmail.com writes:

 Akinobu So Martin and Douglas, can I have your ACKs on this patch
 Akinobu series for now?

 Only concern is the one I mentioned before: You're mapping and unmapping
 the ppage for every 8 bytes of protection information. That seems a bit
 excessive.

I understand your concern.  That could be cured by changing the outer loop from
'scsi_for_each_sg' to 'scsi_for_each_prot_sg'.  I'll try to do this and see
how it looks.

 However, I personally don't care too much about 32-bit platforms, so...

 Acked-by: Martin K. Petersen martin.peter...@oracle.com

Thanks.
--
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 3/3] scsi_debug: fix do_device_access() with wrap around range

[Akinobu Mita]

do_device_access() is a function that abstracts copying SG list from/to
ramdisk storage (fake_storep).

It must deal with the ranges exceeding actual fake_storep size, because
such ranges are valid if virtual_gb is set greater than zero, and they
should be treated as fake_storep is repeatedly mirrored up to virtual size.

Unfortunately, it can't deal with the range which wraps around the end of
fake_storep. A wrap around range is copied by two sg_copy_{from,to}_buffer()
calls, but sg_copy_{from,to}_buffer() can't copy from/to in the middle of
SG list, therefore the second call can't copy correctly.

This fixes it by using sg_pcopy_{from,to}_buffer() that can copy from/to
the middle of SG list.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 43 ---
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 21239b3..c1efaf8 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1614,24 +1614,48 @@ static int check_device_access_params(struct 
sdebug_dev_info *devi,
return 0;
 }
 
+/* Returns number of bytes copied or -1 if error. */
 static int do_device_access(struct scsi_cmnd *scmd,
struct sdebug_dev_info *devi,
unsigned long long lba, unsigned int num, int write)
 {
int ret;
unsigned long long block, rest = 0;
-   int (*func)(struct scsi_cmnd *, unsigned char *, int);
+   struct scsi_data_buffer *sdb;
+   enum dma_data_direction dir;
+   size_t (*func)(struct scatterlist *, unsigned int, void *, size_t,
+  off_t);
+
+   if (write) {
+   sdb = scsi_out(scmd);
+   dir = DMA_TO_DEVICE;
+   func = sg_pcopy_to_buffer;
+   } else {
+   sdb = scsi_in(scmd);
+   dir = DMA_FROM_DEVICE;
+   func = sg_pcopy_from_buffer;
+   }
 
-   func = write ? fetch_to_dev_buffer : fill_from_dev_buffer;
+   if (!sdb-length)
+   return 0;
+   if (!(scsi_bidi_cmnd(scmd) || scmd-sc_data_direction == dir))
+   return -1;
 
block = do_div(lba, sdebug_store_sectors);
if (block + num  sdebug_store_sectors)
rest = block + num - sdebug_store_sectors;
 
-   ret = func(scmd, fake_storep + (block * scsi_debug_sector_size),
-  (num - rest) * scsi_debug_sector_size);
-   if (!ret  rest)
-   ret = func(scmd, fake_storep, rest * scsi_debug_sector_size);
+   ret = func(sdb-table.sgl, sdb-table.nents,
+  fake_storep + (block * scsi_debug_sector_size),
+  (num - rest) * scsi_debug_sector_size, 0);
+   if (ret != (num - rest) * scsi_debug_sector_size)
+   return ret;
+
+   if (rest) {
+   ret += func(sdb-table.sgl, sdb-table.nents,
+   fake_storep, rest * scsi_debug_sector_size,
+   (num - rest) * scsi_debug_sector_size);
+   }
 
return ret;
 }
@@ -1888,7 +1912,12 @@ static int resp_read(struct scsi_cmnd *SCpnt, unsigned 
long long lba,
read_lock_irqsave(atomic_rw, iflags);
ret = do_device_access(SCpnt, devip, lba, num, 0);
read_unlock_irqrestore(atomic_rw, iflags);
-   return ret;
+   if (ret == -1)
+   return DID_ERROR  16;
+
+   scsi_in(SCpnt)-resid = scsi_bufflen(SCpnt) - ret;
+
+   return 0;
 }
 
 static void dump_sector(unsigned char *buf, int len)
-- 
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

[PATCH 0/3] introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()

[Akinobu Mita]

This patch set introduces sg_pcopy_from_buffer() and sg_pcopy_to_buffer(),
which copy data between a linear buffer and an SG list.

The only difference between sg_pcopy_{from,to}_buffer() and
sg_copy_{from,to}_buffer() is an additional argument that specifies
the number of bytes to skip the SG list before copying.

The main reason for introducing these functions is to fix a problem
in scsi_debug module.  And there is a local function in crypto/talitos
module, which can be replaced by sg_pcopy_to_buffer().

Akinobu Mita (3):
  lib/scatterlist: introduce sg_pcopy_from_buffer() and
sg_pcopy_to_buffer()
  crypto: talitos: use sg_pcopy_to_buffer()
  scsi_debug: fix do_device_access() with wrap around range

 drivers/crypto/talitos.c|  60 +---
 drivers/scsi/scsi_debug.c   |  43 ++---
 include/linux/scatterlist.h |   5 ++
 lib/scatterlist.c   | 109 
 4 files changed, 131 insertions(+), 86 deletions(-)

Cc: Tejun Heo t...@kernel.org
Cc: Imre Deak imre.d...@intel.com
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Cc: linux-cry...@vger.kernel.org
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org

-- 
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

[PATCH 1/3] lib/scatterlist: introduce sg_pcopy_from_buffer() and sg_pcopy_to_buffer()

[Akinobu Mita]

The only difference between sg_pcopy_{from,to}_buffer() and
sg_copy_{from,to}_buffer() is an additional argument that specifies
the number of bytes to skip the SG list before copying.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Tejun Heo t...@kernel.org
Cc: Imre Deak imre.d...@intel.com
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Cc: linux-cry...@vger.kernel.org
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: linux-scsi@vger.kernel.org
---
 include/linux/scatterlist.h |   5 ++
 lib/scatterlist.c   | 109 
 2 files changed, 94 insertions(+), 20 deletions(-)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 5951e3f..f5dee42 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -241,6 +241,11 @@ size_t sg_copy_from_buffer(struct scatterlist *sgl, 
unsigned int nents,
 size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents,
 void *buf, size_t buflen);
 
+size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents,
+   void *buf, size_t buflen, off_t skip);
+size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
+ void *buf, size_t buflen, off_t skip);
+
 /*
  * Maximum number of entries that will be allocated in one piece, if
  * a list larger than this is required then chaining will be utilized.
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index a1cf8ca..3b40b72 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -453,6 +453,47 @@ void sg_miter_start(struct sg_mapping_iter *miter, struct 
scatterlist *sgl,
 }
 EXPORT_SYMBOL(sg_miter_start);
 
+static bool sg_miter_get_next_page(struct sg_mapping_iter *miter)
+{
+   if (!miter-__remaining) {
+   struct scatterlist *sg;
+   unsigned long pgoffset;
+
+   if (!__sg_page_iter_next(miter-piter))
+   return false;
+
+   sg = miter-piter.sg;
+   pgoffset = miter-piter.sg_pgoffset;
+
+   miter-__offset = pgoffset ? 0 : sg-offset;
+   miter-__remaining = sg-offset + sg-length -
+   (pgoffset  PAGE_SHIFT) - miter-__offset;
+   miter-__remaining = min_t(unsigned long, miter-__remaining,
+  PAGE_SIZE - miter-__offset);
+   }
+
+   return true;
+}
+
+static bool sg_miter_seek(struct sg_mapping_iter *miter, off_t offset)
+{
+   WARN_ON(miter-addr);
+
+   while (offset) {
+   off_t consumed;
+
+   if (!sg_miter_get_next_page(miter))
+   return false;
+
+   consumed = min_t(off_t, offset, miter-__remaining);
+   miter-__offset += consumed;
+   miter-__remaining -= consumed;
+   offset -= consumed;
+   }
+
+   return true;
+}
+
 /**
  * sg_miter_next - proceed mapping iterator to the next mapping
  * @miter: sg mapping iter to proceed
@@ -478,22 +519,9 @@ bool sg_miter_next(struct sg_mapping_iter *miter)
 * Get to the next page if necessary.
 * __remaining, __offset is adjusted by sg_miter_stop
 */
-   if (!miter-__remaining) {
-   struct scatterlist *sg;
-   unsigned long pgoffset;
-
-   if (!__sg_page_iter_next(miter-piter))
-   return false;
-
-   sg = miter-piter.sg;
-   pgoffset = miter-piter.sg_pgoffset;
+   if (!sg_miter_get_next_page(miter))
+   return false;
 
-   miter-__offset = pgoffset ? 0 : sg-offset;
-   miter-__remaining = sg-offset + sg-length -
-   (pgoffset  PAGE_SHIFT) - miter-__offset;
-   miter-__remaining = min_t(unsigned long, miter-__remaining,
-  PAGE_SIZE - miter-__offset);
-   }
miter-page = sg_page_iter_page(miter-piter);
miter-consumed = miter-length = miter-__remaining;
 
@@ -552,14 +580,16 @@ EXPORT_SYMBOL(sg_miter_stop);
  * @nents:  Number of SG entries
  * @buf:Where to copy from
  * @buflen: The number of bytes to copy
- * @to_buffer:  transfer direction (non zero == from an sg 
list to a
- *  buffer, 0 == from a buffer to an sg list
+ * @skip:   Number of bytes to skip before copying
+ * @to_buffer:  transfer direction (true == from an sg list to a
+ *  buffer, false == from a buffer to an sg list
  *
  * Returns the number of copied bytes.
  *
  **/
 static size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents,
-void *buf, size_t buflen, int to_buffer)
+void *buf, size_t buflen, off_t skip

Re: [PATCH v3 0/6] scsi_debug: bug fixes and cleanups for data integrity support

[Akinobu Mita]

2013/5/29 Martin K. Petersen martin.peter...@oracle.com:
 I have some patches pending as part of my next DIF/DIX update that makes
 some of these things more palatable at the block/SCSI level. Akinobu
 voiced interest in finishing the scsi_debug work on top of my code.

Yes.  I'm interested in that work.  Before I start working on it, I would
like to fix the problems which I found recently with virtual_gb option in
scsi_debug.  Because the change is not small and may touch the DIX/DIF
support code, too.

So Martin and Douglas, can I have your ACKs on this patch series for now?
--
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] [SCSI] ufs: fix register address in UIC error interrupt handling

[Akinobu Mita]

In UIC error interrupt handling, it checks if UIC data link layer error
code indicates PA_INIT_ERROR in order to determine whether a fatal error
handling is needed or not.

But the code tries to read UIC data link layer error code from wrong
REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER, it should be
REG_UIC_ERROR_CODE_DATA_LINK_LAYER.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c32a478..492f2b7 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1246,7 +1246,7 @@ static void ufshcd_err_handler(struct ufs_hba *hba)
if (hba-errors  UIC_ERROR) {
 
reg = readl(hba-mmio_base +
-   REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
+   REG_UIC_ERROR_CODE_DATA_LINK_LAYER);
if (reg  UIC_DATA_LINK_LAYER_ERROR_PA_INIT)
goto fatal_eh;
}
-- 
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

[PATCH v3 0/6] scsi_debug: bug fixes and cleanups for data integrity support

[Akinobu Mita]

This patch set includes bug fixes which I hit when I was tried testing
the data integrity support in scsi_debug on x86_32.

And it also includes cleanups which helps increasing readability and
further bug fixing in data integrity support.

* Changes from v2
- Add new bug fix patch for UNMAP command support
- Change the way to fix for the patch fix invalid address passed to
  kunmap_atomic()
- Reduce more lines of code for the patch reduce duplication between
  prot_verify_read and prot_verify_writ

* Changes from v1
- Split the patch fix data integrity support on highmem machine into
  two separate patches.
- Add new cleanup patch reduce duplication between prot_verify_read and
  prot_verify_write.

Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org

Akinobu Mita (6):
  scsi_debug: fix invalid address passed to kunmap_atomic()
  scsi_debug: fix incorrectly nested kmap_atomic()
  scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1
  scsi_debug: invalidate protection info for unmapped region
  scsi_debug: simplify offset calculation for dif_storep
  scsi_debug: reduce duplication between prot_verify_read and
prot_verify_write

 drivers/scsi/scsi_debug.c | 176 +++---
 1 file changed, 72 insertions(+), 104 deletions(-)

-- 
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

[PATCH v3 1/6] scsi_debug: fix invalid address passed to kunmap_atomic()

[Akinobu Mita]

In the function prot_verify_write(), the kmap address 'daddr' is
incremented in the loop for each data page.  Finally 'daddr' reaches
the next page boundary in the end of the loop, and the invalid address
is passed to kunmap_atomic().

Fix the issue by not incrementing 'daddr' in the loop and offsetting it
by the loop counter on demand.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---

* Change from v2
- It was not very clear that incrementing 'daddr' in the loop and restoring
  the original value by subtracting the sum of increments.  Instead of
  doing that, fix the issue by not incrementing 'daddr' in the loop and
  offsetting it by the loop counter on demand.

 drivers/scsi/scsi_debug.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 0a537a0..d51bddd 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1899,7 +1899,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
daddr = kmap_atomic(sg_page(dsgl)) + dsgl-offset;
 
/* For each sector-sized chunk in data page */
-   for (j = 0 ; j  dsgl-length ; j += scsi_debug_sector_size) {
+   for (j = 0; j  dsgl-length; j += scsi_debug_sector_size) {
 
/* If we're at the end of the current
 * protection page advance to the next one
@@ -1917,11 +1917,11 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
switch (scsi_debug_guard) {
case 1:
-   csum = ip_compute_csum(daddr,
+   csum = ip_compute_csum(daddr + j,
   scsi_debug_sector_size);
break;
case 0:
-   csum = cpu_to_be16(crc_t10dif(daddr,
+   csum = cpu_to_be16(crc_t10dif(daddr + j,
  scsi_debug_sector_size));
break;
default:
@@ -1938,7 +1938,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
   be16_to_cpu(sdt-guard_tag),
   be16_to_cpu(csum));
ret = 0x01;
-   dump_sector(daddr, scsi_debug_sector_size);
+   dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
@@ -1949,7 +1949,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
   %s: REF check failed on sector %lu\n,
   __func__, (unsigned long)sector);
ret = 0x03;
-   dump_sector(daddr, scsi_debug_sector_size);
+   dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
@@ -1959,7 +1959,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
   %s: REF check failed on sector %lu\n,
   __func__, (unsigned long)sector);
ret = 0x03;
-   dump_sector(daddr, scsi_debug_sector_size);
+   dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
@@ -1977,7 +1977,6 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
start_sec++;
ei_lba++;
-   daddr += scsi_debug_sector_size;
ppage_offset += sizeof(struct sd_dif_tuple);
}
 
-- 
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

[PATCH v3 2/6] scsi_debug: fix incorrectly nested kmap_atomic()

[Akinobu Mita]

In the function prot_verify_write(), kmap_atomic()/kunmap_atomic() for
data page and kmap_atomic()/kunmap_atomic() for protection information
page are not nested each other.

It worked perfectly before commit 3e4d3af501cccdc8a8cca41bdbe57d54ad7e7e73
(mm: stack based kmap_atomic()).  Because the kmap_atomic slot KM_IRQ0
was used for data page and the slot KM_IRQ1 was used for protection page.

But KM_types are gone and kmap_atomic() is using stack based implementation.
So two different kmap_atomic() usages must be strictly nested now.

This change ensures kmap_atomic() usage is strictly nested.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
Acked-by: Douglas Gilbert dgilb...@interlog.com
---

* No changes from v2

 drivers/scsi/scsi_debug.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d51bddd..bcf73e4 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1891,12 +1891,12 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
BUG_ON(scsi_sg_count(SCpnt) == 0);
BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
 
-   paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
ppage_offset = 0;
 
/* For each data page */
scsi_for_each_sg(SCpnt, dsgl, scsi_sg_count(SCpnt), i) {
daddr = kmap_atomic(sg_page(dsgl)) + dsgl-offset;
+   paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
 
/* For each sector-sized chunk in data page */
for (j = 0; j  dsgl-length; j += scsi_debug_sector_size) {
@@ -1980,19 +1980,18 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ppage_offset += sizeof(struct sd_dif_tuple);
}
 
+   kunmap_atomic(paddr);
kunmap_atomic(daddr);
}
 
-   kunmap_atomic(paddr);
-
dix_writes++;
 
return 0;
 
 out:
dif_errors++;
-   kunmap_atomic(daddr);
kunmap_atomic(paddr);
+   kunmap_atomic(daddr);
return ret;
 }
 
-- 
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

[PATCH v3 3/6] scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1

[Akinobu Mita]

The protection info dif_storep is allocated only when parameter dif is
not zero.  But it will be accessed when reading or writing to the storage
installed with parameter dix is not zero.

So kernel crashes if scsi_debug module is loaded with parameters dix=1 and
dif=0.

This fixes it by making dif_storep available if parameter dix is not zero
instead of checking if parameter dif is not zero.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
Acked-by: Douglas Gilbert dgilb...@interlog.com
Acked-by: Martin K. Petersen martin.peter...@oracle.com
---

* No changes from v1

 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index bcf73e4..e83e661 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3372,7 +3372,7 @@ static int __init scsi_debug_init(void)
if (scsi_debug_num_parts  0)
sdebug_build_parts(fake_storep, sz);
 
-   if (scsi_debug_dif) {
+   if (scsi_debug_dix) {
int dif_size;
 
dif_size = sdebug_store_sectors * sizeof(struct sd_dif_tuple);
-- 
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

[PATCH v3 4/6] scsi_debug: invalidate protection info for unmapped region

[Akinobu Mita]

When UNMAP command is issued with the data integrity support enabled,
the protection info for the unmapped region is remain unchanged.
So READ command for the region later on causes data integrity failure.

This fixes it by invalidating protection info for the unmapped region
by filling with 0xff pattern.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---

* New patch from v3

 drivers/scsi/scsi_debug.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index e83e661..83efec2 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2064,6 +2064,11 @@ static void unmap_region(sector_t lba, unsigned int len)
   scsi_debug_sector_size *
   scsi_debug_unmap_granularity);
}
+   if (dif_storep) {
+   memset(dif_storep + lba, 0xff,
+  sizeof(*dif_storep) *
+  scsi_debug_unmap_granularity);
+   }
}
lba = map_index_to_lba(index + 1);
}
-- 
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

[PATCH v3 5/6] scsi_debug: simplify offset calculation for dif_storep

[Akinobu Mita]

dif_storep is declared as pointer to unsigned char type.  But it is
actually used to store vmalloced array of struct sd_dif_tuple.

This changes the type of dif_storep to the pointer to struct sd_dif_tuple.
It simplifies offset calculation for dif_storep and enables to remove
hardcoded size of struct sd_dif_tuple.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
Acked-by: Douglas Gilbert dgilb...@interlog.com
---

* No changes from v1

 drivers/scsi/scsi_debug.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 83efec2..fc8b3aa 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -258,7 +258,7 @@ struct sdebug_queued_cmd {
 static struct sdebug_queued_cmd queued_arr[SCSI_DEBUG_CANQUEUE];
 
 static unsigned char * fake_storep;/* ramdisk storage */
-static unsigned char *dif_storep;  /* protection info */
+static struct sd_dif_tuple *dif_storep;/* protection info */
 static void *map_storep;   /* provisioning map */
 
 static unsigned long map_size;
@@ -277,11 +277,6 @@ static char sdebug_proc_name[] = scsi_debug;
 
 static struct bus_type pseudo_lld_bus;
 
-static inline sector_t dif_offset(sector_t sector)
-{
-   return sector  3;
-}
-
 static struct device_driver sdebug_driverfs_driver = {
.name   = sdebug_proc_name,
.bus= pseudo_lld_bus,
@@ -1727,7 +1722,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
start_sec = do_div(tmp_sec, sdebug_store_sectors);
 
-   sdt = (struct sd_dif_tuple *)(dif_storep + dif_offset(start_sec));
+   sdt = dif_storep + start_sec;
 
for (i = 0 ; i  sectors ; i++) {
u16 csum;
@@ -1782,16 +1777,17 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ei_lba++;
}
 
-   resid = sectors * 8; /* Bytes of protection data to copy into sgl */
+   /* Bytes of protection data to copy into sgl */
+   resid = sectors * sizeof(*dif_storep);
sector = start_sec;
 
scsi_for_each_prot_sg(SCpnt, psgl, scsi_prot_sg_count(SCpnt), i) {
int len = min(psgl-length, resid);
 
paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
-   memcpy(paddr, dif_storep + dif_offset(sector), len);
+   memcpy(paddr, dif_storep + sector, len);
 
-   sector += len  3;
+   sector += len / sizeof(*dif_storep);
if (sector = sdebug_store_sectors) {
/* Force wrap */
tmp_sec = sector;
@@ -1968,7 +1964,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 * correctness we need to verify each sector
 * before writing it to stable storage
 */
-   memcpy(dif_storep + dif_offset(sector), sdt, 8);
+   memcpy(dif_storep + sector, sdt, sizeof(*sdt));
 
sector++;
 
-- 
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

[PATCH v3 6/6] scsi_debug: reduce duplication between prot_verify_read and prot_verify_write

[Akinobu Mita]

In order to reduce code duplication between prot_verify_read() and
prot_verify_write(), this moves common code into the new functions.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---

* Change from v2
- Change dif_verify() interface and it reduces more lines of code

 drivers/scsi/scsi_debug.c | 139 ++
 1 file changed, 54 insertions(+), 85 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index fc8b3aa..9790d56 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1710,6 +1710,52 @@ static int do_device_access(struct scsi_cmnd *scmd,
return ret;
 }
 
+static u16 dif_compute_csum(const void *buf, int len)
+{
+   u16 csum;
+
+   switch (scsi_debug_guard) {
+   case 1:
+   csum = ip_compute_csum(buf, len);
+   break;
+   case 0:
+   csum = cpu_to_be16(crc_t10dif(buf, len));
+   break;
+   default:
+   BUG();
+   }
+   return csum;
+}
+
+static int dif_verify(struct sd_dif_tuple *sdt, const void *data,
+ sector_t sector, u32 ei_lba)
+{
+   u16 csum = dif_compute_csum(data, scsi_debug_sector_size);
+
+   if (sdt-guard_tag != csum) {
+   pr_err(%s: GUARD check failed on sector %lu rcvd 0x%04x, data 
0x%04x\n,
+   __func__,
+   (unsigned long)sector,
+   be16_to_cpu(sdt-guard_tag),
+   be16_to_cpu(csum));
+   return 0x01;
+   }
+   if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION 
+   be32_to_cpu(sdt-ref_tag) != (sector  0x)) {
+   pr_err(%s: REF check failed on sector %lu\n,
+   __func__, (unsigned long)sector);
+   return 0x03;
+   }
+   if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION 
+   be32_to_cpu(sdt-ref_tag) != ei_lba) {
+   pr_err(%s: REF check failed on sector %lu\n,
+   __func__, (unsigned long)sector);
+   dif_errors++;
+   return 0x03;
+   }
+   return 0;
+}
+
 static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
unsigned int sectors, u32 ei_lba)
 {
@@ -1725,53 +1771,19 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
sdt = dif_storep + start_sec;
 
for (i = 0 ; i  sectors ; i++) {
-   u16 csum;
+   int ret;
 
if (sdt[i].app_tag == 0x)
continue;
 
sector = start_sec + i;
 
-   switch (scsi_debug_guard) {
-   case 1:
-   csum = ip_compute_csum(fake_storep +
-  sector * scsi_debug_sector_size,
-  scsi_debug_sector_size);
-   break;
-   case 0:
-   csum = crc_t10dif(fake_storep +
- sector * scsi_debug_sector_size,
- scsi_debug_sector_size);
-   csum = cpu_to_be16(csum);
-   break;
-   default:
-   BUG();
-   }
-
-   if (sdt[i].guard_tag != csum) {
-   printk(KERN_ERR %s: GUARD check failed on sector %lu \
-   rcvd 0x%04x, data 0x%04x\n, __func__,
-  (unsigned long)sector,
-  be16_to_cpu(sdt[i].guard_tag),
-  be16_to_cpu(csum));
+   ret = dif_verify(sdt[i],
+fake_storep + sector * scsi_debug_sector_size,
+sector, ei_lba);
+   if (ret) {
dif_errors++;
-   return 0x01;
-   }
-
-   if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION 
-   be32_to_cpu(sdt[i].ref_tag) != (sector  0x)) {
-   printk(KERN_ERR %s: REF check failed on sector %lu\n,
-  __func__, (unsigned long)sector);
-   dif_errors++;
-   return 0x03;
-   }
-
-   if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION 
-   be32_to_cpu(sdt[i].ref_tag) != ei_lba) {
-   printk(KERN_ERR %s: REF check failed on sector %lu\n,
-  __func__, (unsigned long)sector);
-   dif_errors++;
-   return 0x03;
+   return ret;
}
 
ei_lba

[PATCH] ufshcd-pltfrm: add missing empty slot in ufs_of_match[]

[Akinobu Mita]

of_match_table member in struct device_driver must be terminated by
empty slot as a sentinel.

Signed-off-by: Akinobu Mita m...@fixstars.com
Cc: Vinayak Holikatti vinholika...@gmail.com
Cc: Santosh Y santos...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 03319ac..0014e28 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -190,6 +190,7 @@ static int ufshcd_pltfrm_remove(struct platform_device 
*pdev)
 
 static const struct of_device_id ufs_of_match[] = {
{ .compatible = jedec,ufs-1.1},
+   {},
 };
 
 static const struct dev_pm_ops ufshcd_dev_pm_ops = {
-- 
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

[PATCH v2 0/5] scsi_debug: bug fixes and cleanups for data integrity support

[Akinobu Mita]

This patch set includes bug fixes which I hit when I was tried testing
the data integrity support in scsi_debug on x86_32.

And it also includes cleanups which helps increasing readability and
further bug fixing in data integrity support.

* Changes from v1
- Split the patch fix data integrity support on highmem machine into
  two separate patches.
- Add new cleanup patch reduce duplication between prot_verify_read and
  prot_verify_write.

Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org

Akinobu Mita (5):
  scsi_debug: fix invalid address passed to kunmap_atomic()
  scsi_debug: fix incorrectly nested kmap_atomic()
  scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1
  scsi_debug: simplify offset calculation for dif_storep
  scsi_debug: reduce duplication between prot_verify_read and
prot_verify_write

 drivers/scsi/scsi_debug.c | 164 +++---
 1 file changed, 66 insertions(+), 98 deletions(-)

-- 
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

[PATCH v2 1/5] scsi_debug: fix invalid address passed to kunmap_atomic()

[Akinobu Mita]

In the function prot_verify_write(), the kmap address 'daddr' is
incremented in the loop for each data page.  Finally 'daddr' reaches
the next page boundary in the end of the loop, and the invalid address
is passed to kunmap_atomic().

Fix it by passing original value to kunmap_atomic().

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
Acked-by: Douglas Gilbert dgilb...@interlog.com
---

* New patch from v2
- Splitted from the patch fix data integrity support on highmem machine in v1

 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 0a537a0..caf6a94 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1981,7 +1981,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ppage_offset += sizeof(struct sd_dif_tuple);
}
 
-   kunmap_atomic(daddr);
+   kunmap_atomic(daddr - dsgl-length);
}
 
kunmap_atomic(paddr);
-- 
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

[PATCH v2 2/5] scsi_debug: fix incorrectly nested kmap_atomic()

[Akinobu Mita]

In the function prot_verify_write(), kmap_atomic()/kunmap_atomic() for
data page and kmap_atomic()/kunmap_atomic() for protection information
page are not nested each other.

It worked perfectly before commit 3e4d3af501cccdc8a8cca41bdbe57d54ad7e7e73
(mm: stack based kmap_atomic()).  Because the kmap_atomic slot KM_IRQ0
was used for data page and the slot KM_IRQ1 was used for protection page.

But KM_types are gone and kmap_atomic() is using stack based implementation.
So two different kmap_atomic() usages must be strictly nested now.

This change ensures kmap_atomic() usage is strictly nested.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
Acked-by: Douglas Gilbert dgilb...@interlog.com
---

* New patch from v2
- Splitted from the patch fix data integrity support on highmem machine in v1

 drivers/scsi/scsi_debug.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index caf6a94..0a428b6 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1891,12 +1891,12 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
BUG_ON(scsi_sg_count(SCpnt) == 0);
BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
 
-   paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
ppage_offset = 0;
 
/* For each data page */
scsi_for_each_sg(SCpnt, dsgl, scsi_sg_count(SCpnt), i) {
daddr = kmap_atomic(sg_page(dsgl)) + dsgl-offset;
+   paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
 
/* For each sector-sized chunk in data page */
for (j = 0 ; j  dsgl-length ; j += scsi_debug_sector_size) {
@@ -1981,19 +1981,18 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ppage_offset += sizeof(struct sd_dif_tuple);
}
 
+   kunmap_atomic(paddr);
kunmap_atomic(daddr - dsgl-length);
}
 
-   kunmap_atomic(paddr);
-
dix_writes++;
 
return 0;
 
 out:
dif_errors++;
-   kunmap_atomic(daddr);
kunmap_atomic(paddr);
+   kunmap_atomic(daddr);
return ret;
 }
 
-- 
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

[PATCH v2 3/5] scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1

[Akinobu Mita]

The protection info dif_storep is allocated only when parameter dif is
not zero.  But it will be accessed when reading or writing to the storage
installed with parameter dix is not zero.

So kernel crashes if scsi_debug module is loaded with parameters dix=1 and
dif=0.

This fixes it by making dif_storep available if parameter dix is not zero
instead of checking if parameter dif is not zero.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
Acked-by: Douglas Gilbert dgilb...@interlog.com
Acked-by: Martin K. Petersen martin.peter...@oracle.com
---

* No changes from v1

 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 0a428b6..631e9ab 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3373,7 +3373,7 @@ static int __init scsi_debug_init(void)
if (scsi_debug_num_parts  0)
sdebug_build_parts(fake_storep, sz);
 
-   if (scsi_debug_dif) {
+   if (scsi_debug_dix) {
int dif_size;
 
dif_size = sdebug_store_sectors * sizeof(struct sd_dif_tuple);
-- 
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

[PATCH v2 4/5] scsi_debug: simplify offset calculation for dif_storep

[Akinobu Mita]

dif_storep is declared as pointer to unsigned char type.  But it is
actually used to store vmalloced array of struct sd_dif_tuple.

This changes the type of dif_storep to the pointer to struct sd_dif_tuple.
It simplifies offset calculation for dif_storep and enables to remove
hardcoded size of struct sd_dif_tuple.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
Acked-by: Douglas Gilbert dgilb...@interlog.com
---

* No changes from v1

 drivers/scsi/scsi_debug.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 631e9ab..152ead6 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -258,7 +258,7 @@ struct sdebug_queued_cmd {
 static struct sdebug_queued_cmd queued_arr[SCSI_DEBUG_CANQUEUE];
 
 static unsigned char * fake_storep;/* ramdisk storage */
-static unsigned char *dif_storep;  /* protection info */
+static struct sd_dif_tuple *dif_storep;/* protection info */
 static void *map_storep;   /* provisioning map */
 
 static unsigned long map_size;
@@ -277,11 +277,6 @@ static char sdebug_proc_name[] = scsi_debug;
 
 static struct bus_type pseudo_lld_bus;
 
-static inline sector_t dif_offset(sector_t sector)
-{
-   return sector  3;
-}
-
 static struct device_driver sdebug_driverfs_driver = {
.name   = sdebug_proc_name,
.bus= pseudo_lld_bus,
@@ -1727,7 +1722,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
start_sec = do_div(tmp_sec, sdebug_store_sectors);
 
-   sdt = (struct sd_dif_tuple *)(dif_storep + dif_offset(start_sec));
+   sdt = dif_storep + start_sec;
 
for (i = 0 ; i  sectors ; i++) {
u16 csum;
@@ -1782,16 +1777,17 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ei_lba++;
}
 
-   resid = sectors * 8; /* Bytes of protection data to copy into sgl */
+   /* Bytes of protection data to copy into sgl */
+   resid = sectors * sizeof(*dif_storep);
sector = start_sec;
 
scsi_for_each_prot_sg(SCpnt, psgl, scsi_prot_sg_count(SCpnt), i) {
int len = min(psgl-length, resid);
 
paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
-   memcpy(paddr, dif_storep + dif_offset(sector), len);
+   memcpy(paddr, dif_storep + sector, len);
 
-   sector += len  3;
+   sector += len / sizeof(*dif_storep);
if (sector = sdebug_store_sectors) {
/* Force wrap */
tmp_sec = sector;
@@ -1968,7 +1964,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 * correctness we need to verify each sector
 * before writing it to stable storage
 */
-   memcpy(dif_storep + dif_offset(sector), sdt, 8);
+   memcpy(dif_storep + sector, sdt, sizeof(*sdt));
 
sector++;
 
-- 
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

[PATCH v2 5/5] scsi_debug: reduce duplication between prot_verify_read and prot_verify_write

[Akinobu Mita]

In order to reduce code duplication between prot_verify_read() and
prot_verify_write(), this moves common code into the new functions.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---

* New patch from v2

 drivers/scsi/scsi_debug.c | 135 +++---
 1 file changed, 54 insertions(+), 81 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 152ead6..bd14e12 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1710,6 +1710,50 @@ static int do_device_access(struct scsi_cmnd *scmd,
return ret;
 }
 
+static u16 dif_compute_csum(const void *buf, int len)
+{
+   u16 csum;
+
+   switch (scsi_debug_guard) {
+   case 1:
+   csum = ip_compute_csum(buf, len);
+   break;
+   case 0:
+   csum = cpu_to_be16(crc_t10dif(buf, len));
+   break;
+   default:
+   BUG();
+   }
+   return csum;
+}
+
+static int dif_verify(struct sd_dif_tuple *sdt, u16 csum, sector_t sector,
+ u32 ei_lba)
+{
+   if (sdt-guard_tag != csum) {
+   pr_err(%s: GUARD check failed on sector %lu rcvd 0x%04x, data 
0x%04x\n,
+   __func__,
+   (unsigned long)sector,
+   be16_to_cpu(sdt-guard_tag),
+   be16_to_cpu(csum));
+   return 0x01;
+   }
+   if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION 
+   be32_to_cpu(sdt-ref_tag) != (sector  0x)) {
+   pr_err(%s: REF check failed on sector %lu\n,
+   __func__, (unsigned long)sector);
+   return 0x03;
+   }
+   if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION 
+   be32_to_cpu(sdt-ref_tag) != ei_lba) {
+   pr_err(%s: REF check failed on sector %lu\n,
+   __func__, (unsigned long)sector);
+   dif_errors++;
+   return 0x03;
+   }
+   return 0;
+}
+
 static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
unsigned int sectors, u32 ei_lba)
 {
@@ -1726,52 +1770,21 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
for (i = 0 ; i  sectors ; i++) {
u16 csum;
+   int ret;
 
if (sdt[i].app_tag == 0x)
continue;
 
sector = start_sec + i;
 
-   switch (scsi_debug_guard) {
-   case 1:
-   csum = ip_compute_csum(fake_storep +
-  sector * scsi_debug_sector_size,
-  scsi_debug_sector_size);
-   break;
-   case 0:
-   csum = crc_t10dif(fake_storep +
- sector * scsi_debug_sector_size,
- scsi_debug_sector_size);
-   csum = cpu_to_be16(csum);
-   break;
-   default:
-   BUG();
-   }
+   csum = dif_compute_csum(fake_storep +
+   sector * scsi_debug_sector_size,
+   scsi_debug_sector_size);
 
-   if (sdt[i].guard_tag != csum) {
-   printk(KERN_ERR %s: GUARD check failed on sector %lu \
-   rcvd 0x%04x, data 0x%04x\n, __func__,
-  (unsigned long)sector,
-  be16_to_cpu(sdt[i].guard_tag),
-  be16_to_cpu(csum));
+   ret = dif_verify(sdt[i], csum, sector, ei_lba);
+   if (ret) {
dif_errors++;
-   return 0x01;
-   }
-
-   if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION 
-   be32_to_cpu(sdt[i].ref_tag) != (sector  0x)) {
-   printk(KERN_ERR %s: REF check failed on sector %lu\n,
-  __func__, (unsigned long)sector);
-   dif_errors++;
-   return 0x03;
-   }
-
-   if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION 
-   be32_to_cpu(sdt[i].ref_tag) != ei_lba) {
-   printk(KERN_ERR %s: REF check failed on sector %lu\n,
-  __func__, (unsigned long)sector);
-   dif_errors++;
-   return 0x03;
+   return ret;
}
 
ei_lba++;
@@ -1911,50 +1924,10 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec

Re: [PATCH 2/3] scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1

[Akinobu Mita]

2013/4/25 Martin K. Petersen martin.peter...@oracle.com:
 Akinobu == Akinobu Mita akinobu.m...@gmail.com writes:

 Akinobu The protection info dif_storep is allocated only when parameter
 Akinobu dif is not zero.  But it will be accessed when reading or
 Akinobu writing to the storage installed with parameter dix is not
 Akinobu zero.

 Akinobu So kernel crashes if scsi_debug module is loaded with
 Akinobu parameters dix=1 and dif=0.

 The full story is that scsi_debug does not support DIF and DIX correctly
 by virtue of simultaneously being the HBA and the target. And since
 there is no actual data transfer between the HBA and the target the
 notion of DIF is weak at best.

 I did look into making scsi_debug do the right thing but it's quite a
 bit of code and I lost interest about halfway through the effort. If
 you'd like to fix this properly I can probably find the patch to use as
 baseline?

I'm interested in the patch.  So could you provide it?
--
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 1/3] scsi_debug: fix data integrity support on highmem machine

[Akinobu Mita]

2013/4/25 Martin K. Petersen martin.peter...@oracle.com:
 Akinobu == Akinobu Mita akinobu.m...@gmail.com writes:

 Akinobu kmap_atomic() is now using stack based implementation and
 Akinobu doesn't take the KM_type argument anymore.  So nesting
 Akinobu kmap_atomic() calls must be properly stacked.

 Akinobu This fixes nesting kmap_atomic() calls for scsi_sglist and
 Akinobu scsi_prot_sglist in prot_verify_write().

 I don't see how the prog_sglist is incorrectly nested? Also, with your
 code you also end up mapping and unmapping the protection page for every
 data segment. The two scatterlists have different cadence.

I meant that the unmapped address by kunmap_atomic() should be the
address returned by the last kmap_atomic() call which is not yet unmapped.
Because kmap_atomic was changed to stack based implementation.

Specifically,

/* For each data page */
scsi_for_each_sg(SCpnt, dsgl, scsi_sg_count(SCpnt), i) {
daddr = kmap_atomic(sg_page(dsgl)) + dsgl-offset;

This kmapped address 'daddr' will not unmapped during the next for-loop.

/* For each sector-sized chunk in data page */
for (j = 0 ; j  dsgl-length ; j += scsi_debug_sector_size) {

/* If we're at the end of the current
 * protection page advance to the next one
 */
if (ppage_offset = psgl-length) {
kunmap_atomic(paddr);
...

But this kunmap_atomic() for the first time in this loop is called.
The last two kmap_atomic() and kunmap_atomic() do not correspond and
it breaks kmap_atomic/kunmap_atomic stack.
--
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 0/3] scsi_debug: fix data integrity support

[Akinobu Mita]

When I tried testing the data integrity support in scsi_debug on x86_32,
I got CONFIG_DEBUG_HIGHMEM warnings and protection errors.  This was
triggered due to misused kmap_atomic/kunmap_atomic.

And then, while I was testing the fix of the above issue with several
combination with module parameters dix and dif, I found that doing
'modprobe scsi_debug dif=0 dix=1' causes kernel crash.

This patch set includes these fixes and cleanup which is related to data
integrity support.

Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org

Akinobu Mita (3):
  scsi_debug: fix data integrity support on highmem machine
  scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1
  scsi_debug: simplify offset calculation for dif_storep

 drivers/scsi/scsi_debug.c | 29 -
 1 file changed, 12 insertions(+), 17 deletions(-)

-- 
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

[PATCH 1/3] scsi_debug: fix data integrity support on highmem machine

[Akinobu Mita]

kmap_atomic() is now using stack based implementation and doesn't take
the KM_type argument anymore.  So nesting kmap_atomic() calls must be
properly stacked.

This fixes nesting kmap_atomic() calls for scsi_sglist and scsi_prot_sglist
in prot_verify_write().

This also fixes another issue that invalid kmap address is used for
kunmap_atomic(): the kmap address 'daddr' is incremented in the loop for
each data page, and it can reach the next page boundary.

These problems trigger CONFIG_DEBUG_HIGHMEM warnings, protection errors,
and kernel crash when doing I/O for the storage installed by
'modprobe scsi_debug dif=1 dix=1' on x86_32 with highmem.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index aea4c2e..8fd30a5 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1814,12 +1814,12 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
BUG_ON(scsi_sg_count(SCpnt) == 0);
BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
 
-   paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
ppage_offset = 0;
 
/* For each data page */
scsi_for_each_sg(SCpnt, dsgl, scsi_sg_count(SCpnt), i) {
daddr = kmap_atomic(sg_page(dsgl)) + dsgl-offset;
+   paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
 
/* For each sector-sized chunk in data page */
for (j = 0 ; j  dsgl-length ; j += scsi_debug_sector_size) {
@@ -1904,19 +1904,18 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ppage_offset += sizeof(struct sd_dif_tuple);
}
 
-   kunmap_atomic(daddr);
+   kunmap_atomic(paddr);
+   kunmap_atomic(daddr - dsgl-length);
}
 
-   kunmap_atomic(paddr);
-
dix_writes++;
 
return 0;
 
 out:
dif_errors++;
-   kunmap_atomic(daddr);
kunmap_atomic(paddr);
+   kunmap_atomic(daddr);
return ret;
 }
 
-- 
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

[PATCH 2/3] scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1

[Akinobu Mita]

The protection info dif_storep is allocated only when parameter dif is
not zero.  But it will be accessed when reading or writing to the storage
installed with parameter dix is not zero.

So kernel crashes if scsi_debug module is loaded with parameters dix=1 and
dif=0.

This fixes it by making dif_storep available if parameter dix is not zero
instead of checking if parameter dif is not zero.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 8fd30a5..ffbdb4c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3306,7 +3306,7 @@ static int __init scsi_debug_init(void)
if (scsi_debug_num_parts  0)
sdebug_build_parts(fake_storep, sz);
 
-   if (scsi_debug_dif) {
+   if (scsi_debug_dix) {
int dif_size;
 
dif_size = sdebug_store_sectors * sizeof(struct sd_dif_tuple);
-- 
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

[PATCH 3/3] scsi_debug: simplify offset calculation for dif_storep

[Akinobu Mita]

dif_storep is declared as pointer to unsigned char type.  But it is
actually used to store vmalloced array of struct sd_dif_tuple.

This changes the type of dif_storep to the pointer to struct sd_dif_tuple.
It simplifies offset calculation for dif_storep and enables to remove
hardcoded size of struct sd_dif_tuple.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index ffbdb4c..e69cbc2 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -258,7 +258,7 @@ struct sdebug_queued_cmd {
 static struct sdebug_queued_cmd queued_arr[SCSI_DEBUG_CANQUEUE];
 
 static unsigned char * fake_storep;/* ramdisk storage */
-static unsigned char *dif_storep;  /* protection info */
+static struct sd_dif_tuple *dif_storep;/* protection info */
 static void *map_storep;   /* provisioning map */
 
 static unsigned long map_size;
@@ -277,11 +277,6 @@ static char sdebug_proc_name[] = scsi_debug;
 
 static struct bus_type pseudo_lld_bus;
 
-static inline sector_t dif_offset(sector_t sector)
-{
-   return sector  3;
-}
-
 static struct device_driver sdebug_driverfs_driver = {
.name   = sdebug_proc_name,
.bus= pseudo_lld_bus,
@@ -1653,7 +1648,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
start_sec = do_div(tmp_sec, sdebug_store_sectors);
 
-   sdt = (struct sd_dif_tuple *)(dif_storep + dif_offset(start_sec));
+   sdt = dif_storep + start_sec;
 
for (i = 0 ; i  sectors ; i++) {
u16 csum;
@@ -1708,16 +1703,17 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ei_lba++;
}
 
-   resid = sectors * 8; /* Bytes of protection data to copy into sgl */
+   /* Bytes of protection data to copy into sgl */
+   resid = sectors * sizeof(*dif_storep);
sector = start_sec;
 
scsi_for_each_prot_sg(SCpnt, psgl, scsi_prot_sg_count(SCpnt), i) {
int len = min(psgl-length, resid);
 
paddr = kmap_atomic(sg_page(psgl)) + psgl-offset;
-   memcpy(paddr, dif_storep + dif_offset(sector), len);
+   memcpy(paddr, dif_storep + sector, len);
 
-   sector += len  3;
+   sector += len / sizeof(*dif_storep);
if (sector = sdebug_store_sectors) {
/* Force wrap */
tmp_sec = sector;
@@ -1891,7 +1887,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 * correctness we need to verify each sector
 * before writing it to stable storage
 */
-   memcpy(dif_storep + dif_offset(sector), sdt, 8);
+   memcpy(dif_storep + sector, sdt, sizeof(*sdt));
 
sector++;
 
-- 
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

[PATCH 0/6] scsi_debug: fix logical block provisioning support

[Akinobu Mita]

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: 0x  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.

I looked around the logical block provisioning support in scsi_debug,
and I found several problems there.  This patch series tries to fix
these problems and it is broken into small patches as much as possible
for ease of review.

Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com

Akinobu Mita (6):
  scsi_debug: call map_region() and unmap_region() only when needed
  scsi_debug: prohibit scsi_debug_unmap_granularity ==
scsi_debug_unmap_alignment
  scsi_debug: clear correct memory region when LBPRZ is enabled
  scsi_debug: add translation functions between LBA and index of
provisioning map
  scsi_debug: fix initialization of provisioning map
  scsi_debug: fix logical block provisioning support

 drivers/scsi/scsi_debug.c | 97 ---
 1 file changed, 50 insertions(+), 47 deletions(-)

-- 
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

[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 akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
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

[PATCH 2/6] scsi_debug: prohibit scsi_debug_unmap_granularity == scsi_debug_unmap_alignment

[Akinobu Mita]

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 akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

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

[PATCH 3/6] scsi_debug: clear correct memory region when LBPRZ is enabled

[Akinobu Mita]

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 akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 5c32140..4b5d388 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;
}
-- 
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

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

[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 akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
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

[PATCH 5/6] scsi_debug: fix initialization of provisioning map

[Akinobu Mita]

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.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index c6de36c..6581549 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3419,8 +3419,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, 0xU);
 
@@ -3439,9 +3437,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);
@@ -3452,7 +3449,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)
-- 
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

[PATCH 6/6] scsi_debug: fix logical block provisioning support

[Akinobu Mita]

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: 0x  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 added
previously.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Douglas Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 55 ++-
 1 file changed, 21 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 6581549..154d987 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2016,22 +2016,20 @@ static sector_t map_index_to_lba(unsigned long index)
 
 static unsigned int map_state(sector_t lba, unsigned int *num)
 {
-   unsigned int granularity, alignment, mapped;
-   sector_t block, next, end;
+   sector_t end;
+   unsigned int mapped;
+   unsigned long index;
+   unsigned long next;
 
-   granularity = scsi_debug_unmap_granularity;
-   alignment = granularity - scsi_debug_unmap_alignment;
-   block = lba + alignment;
-   do_div(block, granularity);
-
-   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;
@@ -2039,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);
}
 }
 
-- 
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

[PATCH -v3 15/23] scsi: rename random32() to prandom_u32()

[Akinobu Mita]

Use more preferable function name which implies using a pseudo-random
number generator.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Robert Love robert.w.l...@intel.com
Cc: de...@open-fcoe.org
Cc: James Smart james.sm...@emulex.com
Cc: Andrew Vasquez andrew.vasq...@qlogic.com
Cc: linux-dri...@qlogic.com
Cc: linux-scsi@vger.kernel.org
---

No change from v2

 drivers/scsi/fcoe/fcoe_ctlr.c| 4 ++--
 drivers/scsi/lpfc/lpfc_hbadisc.c | 6 +++---
 drivers/scsi/qla2xxx/qla_attr.c  | 7 +--
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 08c3bc3..43bb558 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -2161,7 +2161,7 @@ static void fcoe_ctlr_vn_restart(struct fcoe_ctlr *fip)
 
if (fip-probe_tries  FIP_VN_RLIM_COUNT) {
fip-probe_tries++;
-   wait = random32() % FIP_VN_PROBE_WAIT;
+   wait = prandom_u32() % FIP_VN_PROBE_WAIT;
} else
wait = FIP_VN_RLIM_INT;
mod_timer(fip-timer, jiffies + msecs_to_jiffies(wait));
@@ -2794,7 +2794,7 @@ static void fcoe_ctlr_vn_timeout(struct fcoe_ctlr *fip)
  fcoe_all_vn2vn, 0);
fip-port_ka_time = jiffies +
 msecs_to_jiffies(FIP_VN_BEACON_INT +
-   (random32() % FIP_VN_BEACON_FUZZ));
+   (prandom_u32() % FIP_VN_BEACON_FUZZ));
}
if (time_before(fip-port_ka_time, next_time))
next_time = fip-port_ka_time;
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index d7096ad..bfda184 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -1732,7 +1732,7 @@ lpfc_check_pending_fcoe_event(struct lpfc_hba *phba, 
uint8_t unreg_fcf)
  * use through a sequence of @fcf_cnt eligible FCF records with equal
  * probability. To perform integer manunipulation of random numbers with
  * size unit32_t, the lower 16 bits of the 32-bit random number returned
- * from random32() are taken as the random random number generated.
+ * from prandom_u32() are taken as the random random number generated.
  *
  * Returns true when outcome is for the newly read FCF record should be
  * chosen; otherwise, return false when outcome is for keeping the previously
@@ -1744,7 +1744,7 @@ lpfc_sli4_new_fcf_random_select(struct lpfc_hba *phba, 
uint32_t fcf_cnt)
uint32_t rand_num;
 
/* Get 16-bit uniform random number */
-   rand_num = (0x  random32());
+   rand_num = 0x  prandom_u32();
 
/* Decision with probability 1/fcf_cnt */
if ((fcf_cnt * rand_num)  0x)
@@ -2380,7 +2380,7 @@ lpfc_mbx_cmpl_fcf_scan_read_fcf_rec(struct lpfc_hba 
*phba, LPFC_MBOXQ_t *mboxq)
phba-fcf.eligible_fcf_cnt = 1;
/* Seeding the random number generator for random selection */
seed = (uint32_t)(0x  jiffies);
-   srandom32(seed);
+   prandom_seed(seed);
}
spin_unlock_irq(phba-hbalock);
goto read_next_fcf;
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 1d82eef..04bf7b8 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -1940,8 +1940,11 @@ qla24xx_vport_delete(struct fc_vport *fc_vport)
 
/* No pending activities shall be there on the vha now */
if (ql2xextended_error_logging  ql_dbg_user)
-   msleep(random32()%10);  /* Just to see if something falls on
-   * the net we have placed below */
+   msleep(prandom_u32() % 10);
+   /*
+* Just to see if something falls on the net we have placed
+* below
+*/
 
BUG_ON(atomic_read(vha-vref_count));
 
-- 
1.8.1.2

--
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] scsi: fix typo s/scsi_dispatch_cmnd/scsi_dispatch_cmd/

[Akinobu Mita]

Logging message contains a mistake in its own function name.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 2c0d0ec..39acb6a 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -765,7 +765,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
}
 
  out:
-   SCSI_LOG_MLQUEUE(3, printk(leaving scsi_dispatch_cmnd()\n));
+   SCSI_LOG_MLQUEUE(3, printk(leaving %s()\n, __func__));
return rtn;
 }
 
-- 
1.8.1

--
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/5] pmcraid: fix a typo in error message

[Akinobu Mita]

Remove duplicate with in error message.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anil Ravindranath anil_ravindran...@pmc-sierra.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/pmcraid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index b46f5e9..073ff48 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -6106,7 +6106,7 @@ static int __init pmcraid_init(void)
 
if (IS_ERR(pmcraid_class)) {
error = PTR_ERR(pmcraid_class);
-   pmcraid_err(failed to register with with sysfs, error = %x\n,
+   pmcraid_err(failed to register with sysfs, error = %x\n,
error);
goto out_unreg_chrdev;
}
-- 
1.8.1

--
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 2/5] pmcraid: fix pmcraid_netlink_init() error path in module_init

[Akinobu Mita]

pmcraid_netlink_init() error path in module_init doesn't destroy
a struct class that was created just before.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anil Ravindranath anil_ravindran...@pmc-sierra.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/pmcraid.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 073ff48..db77bf9 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -6114,18 +6114,20 @@ static int __init pmcraid_init(void)
error = pmcraid_netlink_init();
 
if (error)
-   goto out_unreg_chrdev;
+   goto out_class_destroy;
 
error = pci_register_driver(pmcraid_driver);
 
-   if (error == 0)
-   goto out_init;
+   if (error)
+   goto out_netlink_release;
 
-   pmcraid_err(failed to register pmcraid driver, error = %x\n,
-error);
-   class_destroy(pmcraid_class);
-   pmcraid_netlink_release();
+   return 0;
 
+out_netlink_release:
+   pmcraid_err(failed to register pmcraid driver, error = %x\n, error);
+   pmcraid_netlink_release();
+out_class_destroy:
+   class_destroy(pmcraid_class);
 out_unreg_chrdev:
unregister_chrdev_region(MKDEV(pmcraid_major, 0), PMCRAID_MAX_ADAPTERS);
 
-- 
1.8.1

--
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 3/5] pmcraid: make pmcraid_minor static

[Akinobu Mita]

The bitmap pmcraid_minor is only used in this file.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anil Ravindranath anil_ravindran...@pmc-sierra.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/pmcraid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index db77bf9..583ec82 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -76,7 +76,7 @@ static atomic_t pmcraid_adapter_count = ATOMIC_INIT(0);
  */
 static unsigned int pmcraid_major;
 static struct class *pmcraid_class;
-DECLARE_BITMAP(pmcraid_minor, PMCRAID_MAX_ADAPTERS);
+static DECLARE_BITMAP(pmcraid_minor, PMCRAID_MAX_ADAPTERS);
 
 /*
  * Module parameters
-- 
1.8.1

--
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/5] pmcraid: check for exceeding the max adapters limit

[Akinobu Mita]

Add proper check for running out of bitmap slot and disallow exceeding
the max adapters limit.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anil Ravindranath anil_ravindran...@pmc-sierra.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/pmcraid.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 583ec82..e3b68d9 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -5372,8 +5372,10 @@ static unsigned short pmcraid_get_minor(void)
 {
int minor;
 
-   minor = find_first_zero_bit(pmcraid_minor, sizeof(pmcraid_minor));
-   __set_bit(minor, pmcraid_minor);
+   minor = find_first_zero_bit(pmcraid_minor, PMCRAID_MAX_ADAPTERS);
+   if (minor  PMCRAID_MAX_ADAPTERS)
+   __set_bit(minor, pmcraid_minor);
+
return minor;
 }
 
@@ -5399,6 +5401,9 @@ static int pmcraid_setup_chrdev(struct pmcraid_instance 
*pinstance)
int error;
 
minor = pmcraid_get_minor();
+   if (minor = PMCRAID_MAX_ADAPTERS)
+   return -EBUSY;
+
cdev_init(pinstance-cdev, pmcraid_fops);
pinstance-cdev.owner = THIS_MODULE;
 
-- 
1.8.1

--
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 5/5] pmcraid: check error from device_create()

[Akinobu Mita]

Fix an unchecked error from device_create() and convert to use standard
goto based unwinding for error cleanup.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: Anil Ravindranath anil_ravindran...@pmc-sierra.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/pmcraid.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index e3b68d9..d3034a8 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -5399,6 +5399,7 @@ static int pmcraid_setup_chrdev(struct pmcraid_instance 
*pinstance)
 {
int minor;
int error;
+   struct device *device;
 
minor = pmcraid_get_minor();
if (minor = PMCRAID_MAX_ADAPTERS)
@@ -5408,12 +5409,23 @@ static int pmcraid_setup_chrdev(struct pmcraid_instance 
*pinstance)
pinstance-cdev.owner = THIS_MODULE;
 
error = cdev_add(pinstance-cdev, MKDEV(pmcraid_major, minor), 1);
-
if (error)
-   pmcraid_release_minor(minor);
-   else
-   device_create(pmcraid_class, NULL, MKDEV(pmcraid_major, minor),
+   goto out_release_minor;
+
+   device = device_create(pmcraid_class, NULL, MKDEV(pmcraid_major, minor),
  NULL, %s%u, PMCRAID_DEVFILE, minor);
+   if (IS_ERR(device)) {
+   error = PTR_ERR(device);
+   goto out_cdev_del;
+   }
+
+   return 0;
+
+out_cdev_del:
+   cdev_del(pinstance-cdev);
+out_release_minor:
+   pmcraid_release_minor(minor);
+
return error;
 }
 
-- 
1.8.1

--
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 -v2 16/26] scsi: rename random32() to prandom_u32()

[Akinobu Mita]

Use more preferable function name which implies using a pseudo-random
number generator.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Robert Love robert.w.l...@intel.com
Cc: de...@open-fcoe.org
Cc: James Smart james.sm...@emulex.com
Cc: Andrew Vasquez andrew.vasq...@qlogic.com
Cc: linux-dri...@qlogic.com
Cc: linux-scsi@vger.kernel.org
---

No change from v1

 drivers/scsi/fcoe/fcoe_ctlr.c| 4 ++--
 drivers/scsi/lpfc/lpfc_hbadisc.c | 6 +++---
 drivers/scsi/qla2xxx/qla_attr.c  | 7 +--
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 4a909d7..d870ccc 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -2153,7 +2153,7 @@ static void fcoe_ctlr_vn_restart(struct fcoe_ctlr *fip)
 
if (fip-probe_tries  FIP_VN_RLIM_COUNT) {
fip-probe_tries++;
-   wait = random32() % FIP_VN_PROBE_WAIT;
+   wait = prandom_u32() % FIP_VN_PROBE_WAIT;
} else
wait = FIP_VN_RLIM_INT;
mod_timer(fip-timer, jiffies + msecs_to_jiffies(wait));
@@ -2786,7 +2786,7 @@ static void fcoe_ctlr_vn_timeout(struct fcoe_ctlr *fip)
  fcoe_all_vn2vn, 0);
fip-port_ka_time = jiffies +
 msecs_to_jiffies(FIP_VN_BEACON_INT +
-   (random32() % FIP_VN_BEACON_FUZZ));
+   (prandom_u32() % FIP_VN_BEACON_FUZZ));
}
if (time_before(fip-port_ka_time, next_time))
next_time = fip-port_ka_time;
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index d7096ad..bfda184 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -1732,7 +1732,7 @@ lpfc_check_pending_fcoe_event(struct lpfc_hba *phba, 
uint8_t unreg_fcf)
  * use through a sequence of @fcf_cnt eligible FCF records with equal
  * probability. To perform integer manunipulation of random numbers with
  * size unit32_t, the lower 16 bits of the 32-bit random number returned
- * from random32() are taken as the random random number generated.
+ * from prandom_u32() are taken as the random random number generated.
  *
  * Returns true when outcome is for the newly read FCF record should be
  * chosen; otherwise, return false when outcome is for keeping the previously
@@ -1744,7 +1744,7 @@ lpfc_sli4_new_fcf_random_select(struct lpfc_hba *phba, 
uint32_t fcf_cnt)
uint32_t rand_num;
 
/* Get 16-bit uniform random number */
-   rand_num = (0x  random32());
+   rand_num = 0x  prandom_u32();
 
/* Decision with probability 1/fcf_cnt */
if ((fcf_cnt * rand_num)  0x)
@@ -2380,7 +2380,7 @@ lpfc_mbx_cmpl_fcf_scan_read_fcf_rec(struct lpfc_hba 
*phba, LPFC_MBOXQ_t *mboxq)
phba-fcf.eligible_fcf_cnt = 1;
/* Seeding the random number generator for random selection */
seed = (uint32_t)(0x  jiffies);
-   srandom32(seed);
+   prandom_seed(seed);
}
spin_unlock_irq(phba-hbalock);
goto read_next_fcf;
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 83d7984..3658e9f 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -1933,8 +1933,11 @@ qla24xx_vport_delete(struct fc_vport *fc_vport)
 
/* No pending activities shall be there on the vha now */
if (ql2xextended_error_logging  ql_dbg_user)
-   msleep(random32()%10);  /* Just to see if something falls on
-   * the net we have placed below */
+   msleep(prandom_u32() % 10);
+   /*
+* Just to see if something falls on the net we have placed
+* below
+*/
 
BUG_ON(atomic_read(vha-vref_count));
 
-- 
1.7.11.7

--
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 16/29] scsi: rename random32() to prandom_u32()

[Akinobu Mita]

Use more preferable function name which implies using a pseudo-random
number generator.

Signed-off-by: Akinobu Mita akinobu.m...@gmail.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Robert Love robert.w.l...@intel.com
Cc: de...@open-fcoe.org
Cc: James Smart james.sm...@emulex.com
Cc: Andrew Vasquez andrew.vasq...@qlogic.com
Cc: linux-dri...@qlogic.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/fcoe/fcoe_ctlr.c| 4 ++--
 drivers/scsi/lpfc/lpfc_hbadisc.c | 6 +++---
 drivers/scsi/qla2xxx/qla_attr.c  | 7 +--
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 4a909d7..d870ccc 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -2153,7 +2153,7 @@ static void fcoe_ctlr_vn_restart(struct fcoe_ctlr *fip)
 
if (fip-probe_tries  FIP_VN_RLIM_COUNT) {
fip-probe_tries++;
-   wait = random32() % FIP_VN_PROBE_WAIT;
+   wait = prandom_u32() % FIP_VN_PROBE_WAIT;
} else
wait = FIP_VN_RLIM_INT;
mod_timer(fip-timer, jiffies + msecs_to_jiffies(wait));
@@ -2786,7 +2786,7 @@ static void fcoe_ctlr_vn_timeout(struct fcoe_ctlr *fip)
  fcoe_all_vn2vn, 0);
fip-port_ka_time = jiffies +
 msecs_to_jiffies(FIP_VN_BEACON_INT +
-   (random32() % FIP_VN_BEACON_FUZZ));
+   (prandom_u32() % FIP_VN_BEACON_FUZZ));
}
if (time_before(fip-port_ka_time, next_time))
next_time = fip-port_ka_time;
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index d7096ad..bfda184 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -1732,7 +1732,7 @@ lpfc_check_pending_fcoe_event(struct lpfc_hba *phba, 
uint8_t unreg_fcf)
  * use through a sequence of @fcf_cnt eligible FCF records with equal
  * probability. To perform integer manunipulation of random numbers with
  * size unit32_t, the lower 16 bits of the 32-bit random number returned
- * from random32() are taken as the random random number generated.
+ * from prandom_u32() are taken as the random random number generated.
  *
  * Returns true when outcome is for the newly read FCF record should be
  * chosen; otherwise, return false when outcome is for keeping the previously
@@ -1744,7 +1744,7 @@ lpfc_sli4_new_fcf_random_select(struct lpfc_hba *phba, 
uint32_t fcf_cnt)
uint32_t rand_num;
 
/* Get 16-bit uniform random number */
-   rand_num = (0x  random32());
+   rand_num = 0x  prandom_u32();
 
/* Decision with probability 1/fcf_cnt */
if ((fcf_cnt * rand_num)  0x)
@@ -2380,7 +2380,7 @@ lpfc_mbx_cmpl_fcf_scan_read_fcf_rec(struct lpfc_hba 
*phba, LPFC_MBOXQ_t *mboxq)
phba-fcf.eligible_fcf_cnt = 1;
/* Seeding the random number generator for random selection */
seed = (uint32_t)(0x  jiffies);
-   srandom32(seed);
+   prandom_seed(seed);
}
spin_unlock_irq(phba-hbalock);
goto read_next_fcf;
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 83d7984..3658e9f 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -1933,8 +1933,11 @@ qla24xx_vport_delete(struct fc_vport *fc_vport)
 
/* No pending activities shall be there on the vha now */
if (ql2xextended_error_logging  ql_dbg_user)
-   msleep(random32()%10);  /* Just to see if something falls on
-   * the net we have placed below */
+   msleep(prandom_u32() % 10);
+   /*
+* Just to see if something falls on the net we have placed
+* below
+*/
 
BUG_ON(atomic_read(vha-vref_count));
 
-- 
1.7.11.7

--
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] sr: fix error handling in module_init

[Akinobu Mita]

Sweep registered blkdev when scsi_register_driver has failed.

Cc: Jens Axboe [EMAIL PROTECTED]
Signed-off-by: Akinobu Mita [EMAIL PROTECTED]

Index: 2.6-rc/drivers/scsi/sr.c
===
--- 2.6-rc.orig/drivers/scsi/sr.c
+++ 2.6-rc/drivers/scsi/sr.c
@@ -885,7 +885,11 @@ static int __init init_sr(void)
rc = register_blkdev(SCSI_CDROM_MAJOR, sr);
if (rc)
return rc;
-   return scsi_register_driver(sr_template.gendrv);
+   rc = scsi_register_driver(sr_template.gendrv);
+   if (rc)
+   unregister_blkdev(SCSI_CDROM_MAJOR, sr);
+
+   return rc;
 }
 
 static void __exit exit_sr(void)
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fix possible race with scsi command pool

[Akinobu Mita]

Hello!

There is a place where the reference count of scsi_host_cmd_pool is
incremented without acquiring host_cmd_pool_mutex.

Signed-off-by: Akinobu Mita [EMAIL PROTECTED]

--- 2.6-mm/drivers/scsi/scsi.c.orig 2005-01-20 22:40:20.0 +0900
+++ 2.6-mm/drivers/scsi/scsi.c  2005-01-20 22:41:16.0 +0900
@@ -360,9 +360,9 @@ int scsi_setup_command_freelist(struct S
return 0;
 
  fail2:
+   down(host_cmd_pool_mutex);
if (!--pool-users)
kmem_cache_destroy(pool-slab);
-   return -ENOMEM;
  fail:
up(host_cmd_pool_mutex);
return -ENOMEM;


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