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

2013-06-08 Thread Akinobu Mita
2013/6/7 Martin K. Petersen :
>> "Akinobu" == Akinobu Mita  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 

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


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

2013-06-06 Thread Martin K. Petersen
> "Akinobu" == Akinobu Mita  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.

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

Acked-by: Martin K. Petersen 

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


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

2013-06-02 Thread Douglas Gilbert

On 13-06-01 10:51 PM, Akinobu Mita wrote:

2013/5/29 Martin K. Petersen :

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?


Done.

Just doing some debugging using scsi_debug and noticed
that 'blockdev --rereadpt /dev/sdb' causes a SCSI
REPORT SUPPORTED OPERATION CODES command to be issued.
Perhaps we could add that one (and ... SUPPORTED TMFs) to
scsi_debug's code.

I was also testing setting the SWP bit in the Control
mode page and that works, even though that field is
marked as 'changeable=n'. Since scsi_debug allows all
fields in that page to be changed, perhaps they could
all be marked as changeable.

Doug Gilbert


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

2013-06-02 Thread Douglas Gilbert

On 13-05-26 04:01 AM, Akinobu Mita wrote:

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" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
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(-)


Acked-by: Douglas Gilbert 

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

2013-06-01 Thread Akinobu Mita
2013/5/29 Martin K. Petersen :
> 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


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

2013-05-28 Thread Martin K. Petersen
> "Doug" == Douglas Gilbert  writes:

Doug> No PI, why not? This was tested on lk 3.9.4 with your patches
Doug> applied.

I've been communicating with Akinobu offline about this.

The reason is that I really only implemented DIX support in
scsi_debug. The dif module parameter is only there to select the
reported protection type.

The main problem with scsi_debug is that it's both initiator and target
in one. And there isn't any notion of a data transfer between the
initiator and the target. We just fill out the scatterlists with stuff
from the in-memory buffer.

To support DIX/DIF correctly we'd have to have the notion of a handoff
between the front and back of scsi_debug. And we'd have to support all
combinations of DIX and DIF with the relevant slicing and dicing of data
and PI buffers.

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.

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


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

2013-05-28 Thread Douglas Gilbert

Perhaps I'm missing something but I can never get
DIF stuff to do what I want in scsi_debug. It was
like this before your patches as well.

Assume this set up:
# lsscsi -gp
[1:0:0:0]diskATA  ST3320620AS  3.AA  /dev/sda   /dev/sg0   - 
  -
[6:0:9:0]diskSEAGATE  ST33000650SS 0002  /dev/sdb   /dev/sg2 
DIF/Type1  -
[7:0:0:0]diskLinuxscsi_debug   0004  /dev/sdc   /dev/sg3 
DIF/Type1  -


So 6:0:9:0 is a real disk formatted with protection type 1
and 7:0:0:0 is simulating the same thing (with scsi_debug).
Now I try to read the first block with RDPROTECT=1 so in
the data-out buffer I expect the LB plus the protection
info. That as 512+8 bytes. So now I use ddpt to read the
first block plus PI from the Seagate disk:

# ddpt if=/dev/sg2 bs=512 verbose=3 protect=1 count=1
  
Output file not specified so no copy, just reading input
READ cdb: 28 20 00 00 00 00 00 00 01 00
1+0 records in
0+0 records out
time to read data: 0.000574 secs at 892.0 KB/sec

Good, asked for 520 bytes and got them. But when I try
that with scsi_debug disk:

# ddpt if=/dev/sg3 bs=512 verbose=3 protect=1 count=1
...
Output file not specified so no copy, just reading input
READ cdb: 28 20 00 00 00 00 00 00 01 00
READ: pass-through requested 520 bytes but got 512 bytes
1+0 records in
0+0 records out
>> Non-zero sum of residual counts=8
time to read data: 0.000833 secs at 614.6 KB/sec

No PI, why not? This was tested on lk 3.9.4 with your patches
applied.

Doug Gilbert


On 13-05-26 04:01 AM, Akinobu Mita wrote:

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" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
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(-)



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

2013-05-26 Thread 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" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
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