* [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).