linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] scsi_debug: fix bugs with certain module parameters
@ 2013-07-15 11:52 Akinobu Mita
  2013-07-15 11:52 ` [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts() Akinobu Mita
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Akinobu Mita @ 2013-07-15 11:52 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert,
	Martin K. Petersen

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" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts()
  2013-07-15 11:52 [PATCH 0/4] scsi_debug: fix bugs with certain module parameters Akinobu Mita
@ 2013-07-15 11:52 ` Akinobu Mita
  2013-07-25 17:10   ` Martin Peschke
  2013-07-15 11:52 ` [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0 Akinobu Mita
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Akinobu Mita @ 2013-07-15 11:52 UTC (permalink / raw)
  To: linux-scsi; +Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert

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.mita@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0
  2013-07-15 11:52 [PATCH 0/4] scsi_debug: fix bugs with certain module parameters Akinobu Mita
  2013-07-15 11:52 ` [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts() Akinobu Mita
@ 2013-07-15 11:52 ` Akinobu Mita
  2013-08-19 14:16   ` Akinobu Mita
  2013-07-15 11:52 ` [PATCH 3/4] scsi_debug: fix WRITE_SAME with virtual_gb > 0 Akinobu Mita
  2013-07-15 11:52 ` [PATCH 4/4] scsi_debug: fix out of range access by Get_LBA_status " Akinobu Mita
  3 siblings, 1 reply; 13+ messages in thread
From: Akinobu Mita @ 2013-07-15 11:52 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert,
	Martin K. Petersen

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.mita@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: "Martin K. Petersen" <martin.petersen@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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] scsi_debug: fix WRITE_SAME with virtual_gb > 0
  2013-07-15 11:52 [PATCH 0/4] scsi_debug: fix bugs with certain module parameters Akinobu Mita
  2013-07-15 11:52 ` [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts() Akinobu Mita
  2013-07-15 11:52 ` [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0 Akinobu Mita
@ 2013-07-15 11:52 ` Akinobu Mita
  2013-07-15 11:52 ` [PATCH 4/4] scsi_debug: fix out of range access by Get_LBA_status " Akinobu Mita
  3 siblings, 0 replies; 13+ messages in thread
From: Akinobu Mita @ 2013-07-15 11:52 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert,
	Martin K. Petersen

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.mita@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: "Martin K. Petersen" <martin.petersen@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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] scsi_debug: fix out of range access by Get_LBA_status with virtual_gb > 0
  2013-07-15 11:52 [PATCH 0/4] scsi_debug: fix bugs with certain module parameters Akinobu Mita
                   ` (2 preceding siblings ...)
  2013-07-15 11:52 ` [PATCH 3/4] scsi_debug: fix WRITE_SAME with virtual_gb > 0 Akinobu Mita
@ 2013-07-15 11:52 ` Akinobu Mita
  3 siblings, 0 replies; 13+ messages in thread
From: Akinobu Mita @ 2013-07-15 11:52 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert,
	Martin K. Petersen

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.mita@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: "Martin K. Petersen" <martin.petersen@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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts()
  2013-07-15 11:52 ` [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts() Akinobu Mita
@ 2013-07-25 17:10   ` Martin Peschke
  2013-07-25 20:16     ` Douglas Gilbert
  2013-07-26 17:07     ` Akinobu Mita
  0 siblings, 2 replies; 13+ messages in thread
From: Martin Peschke @ 2013-07-25 17:10 UTC (permalink / raw)
  To: Akinobu Mita; +Cc: linux-scsi, James E.J. Bottomley, Douglas Gilbert

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.mita@gmail.com>
> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
> Cc: Douglas Gilbert <dgilbert@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-scsi&m=137051617907423&w=2
Good luck!

Acked-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>


-- 
Linux on System z Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts()
  2013-07-25 17:10   ` Martin Peschke
@ 2013-07-25 20:16     ` Douglas Gilbert
  2013-07-26 17:07     ` Akinobu Mita
  1 sibling, 0 replies; 13+ messages in thread
From: Douglas Gilbert @ 2013-07-25 20:16 UTC (permalink / raw)
  To: Martin Peschke; +Cc: Akinobu Mita, linux-scsi, James E.J. Bottomley

On 13-07-25 01:10 PM, Martin Peschke wrote:
> 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.mita@gmail.com>
>> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
>> Cc: Douglas Gilbert <dgilbert@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-scsi&m=137051617907423&w=2
> Good luck!
>
> Acked-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>

(Re-)Acked-by: Douglas Gilbert <dgilbert@interlog.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts()
  2013-07-25 17:10   ` Martin Peschke
  2013-07-25 20:16     ` Douglas Gilbert
@ 2013-07-26 17:07     ` Akinobu Mita
  2013-07-29 10:53       ` Martin Peschke
  1 sibling, 1 reply; 13+ messages in thread
From: Akinobu Mita @ 2013-07-26 17:07 UTC (permalink / raw)
  To: Martin Peschke; +Cc: linux-scsi, James E.J. Bottomley, Douglas Gilbert

2013/7/26 Martin Peschke <mpeschke@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.mita@gmail.com>
>> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
>> Cc: Douglas Gilbert <dgilbert@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-scsi&m=137051617907423&w=2
> Good luck!
>
> Acked-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>

Thanks.  I found this problem from sparse warning noticed by 0-DAY kernel
build testing.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts()
  2013-07-26 17:07     ` Akinobu Mita
@ 2013-07-29 10:53       ` Martin Peschke
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Peschke @ 2013-07-29 10:53 UTC (permalink / raw)
  To: Akinobu Mita; +Cc: linux-scsi, James E.J. Bottomley, Douglas Gilbert

On Sat, 2013-07-27 at 02:07 +0900, Akinobu Mita wrote:
> 2013/7/26 Martin Peschke <mpeschke@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.mita@gmail.com>
> >> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
> >> Cc: Douglas Gilbert <dgilbert@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-scsi&m=137051617907423&w=2
> > Good luck!
> >
> > Acked-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>
> 
> Thanks.  I found this problem from sparse warning noticed by 0-DAY kernel
> build testing.

It surfaced as a real problem on System z (big endian).
That is why I can confirm that the above patch works.

Thanks,
Martin


-- 
Linux on System z Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0
  2013-07-15 11:52 ` [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0 Akinobu Mita
@ 2013-08-19 14:16   ` Akinobu Mita
  2013-08-20 20:56     ` Douglas Gilbert
  0 siblings, 1 reply; 13+ messages in thread
From: Akinobu Mita @ 2013-08-19 14:16 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert,
	Martin K. Petersen

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.mita@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.mita@gmail.com>
> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
> Cc: Douglas Gilbert <dgilbert@interlog.com>
> Cc: "Martin K. Petersen" <martin.petersen@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
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0
  2013-08-19 14:16   ` Akinobu Mita
@ 2013-08-20 20:56     ` Douglas Gilbert
  2013-08-21 13:58       ` Akinobu Mita
  2013-08-22  1:29       ` Martin K. Petersen
  0 siblings, 2 replies; 13+ messages in thread
From: Douglas Gilbert @ 2013-08-20 20:56 UTC (permalink / raw)
  To: Akinobu Mita; +Cc: linux-scsi, James E.J. Bottomley, Martin K. Petersen

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 <dgilbert@interlog.com>

> 2013/7/15 Akinobu Mita <akinobu.mita@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.mita@gmail.com>
>> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
>> Cc: Douglas Gilbert <dgilbert@interlog.com>
>> Cc: "Martin K. Petersen" <martin.petersen@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0
  2013-08-20 20:56     ` Douglas Gilbert
@ 2013-08-21 13:58       ` Akinobu Mita
  2013-08-22  1:29       ` Martin K. Petersen
  1 sibling, 0 replies; 13+ messages in thread
From: Akinobu Mita @ 2013-08-21 13:58 UTC (permalink / raw)
  To: Douglas Gilbert; +Cc: linux-scsi, James E.J. Bottomley, Martin K. Petersen

2013/8/21 Douglas Gilbert <dgilbert@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 <dgilbert@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: 0x0000000000000001  blocks: 16383  mapped

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0
  2013-08-20 20:56     ` Douglas Gilbert
  2013-08-21 13:58       ` Akinobu Mita
@ 2013-08-22  1:29       ` Martin K. Petersen
  1 sibling, 0 replies; 13+ messages in thread
From: Martin K. Petersen @ 2013-08-22  1:29 UTC (permalink / raw)
  To: dgilbert; +Cc: Akinobu Mita, linux-scsi, James E.J. Bottomley,
	Martin K. Petersen

>>>>> "Doug" == Douglas Gilbert <dgilbert@interlog.com> writes:

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

Doug> Acked-by: Douglas Gilbert <dgilbert@interlog.com>

I believe I already did, but:

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-08-22  1:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-15 11:52 [PATCH 0/4] scsi_debug: fix bugs with certain module parameters Akinobu Mita
2013-07-15 11:52 ` [PATCH 1/4] scsi_debug: fix endianness bug in sdebug_build_parts() Akinobu Mita
2013-07-25 17:10   ` Martin Peschke
2013-07-25 20:16     ` Douglas Gilbert
2013-07-26 17:07     ` Akinobu Mita
2013-07-29 10:53       ` Martin Peschke
2013-07-15 11:52 ` [PATCH 2/4] scsi_debug: fix logical block provisioning support when unmap_alignment != 0 Akinobu Mita
2013-08-19 14:16   ` Akinobu Mita
2013-08-20 20:56     ` Douglas Gilbert
2013-08-21 13:58       ` Akinobu Mita
2013-08-22  1:29       ` Martin K. Petersen
2013-07-15 11:52 ` [PATCH 3/4] scsi_debug: fix WRITE_SAME with virtual_gb > 0 Akinobu Mita
2013-07-15 11:52 ` [PATCH 4/4] scsi_debug: fix out of range access by Get_LBA_status " Akinobu Mita

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).