Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest5 patches 12 repliesIn review
Patches in v1 No applies recorded yet
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [patchset ver2 0/5] Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE
Thank you Christoph, Randy, Alen for your comments. Here is ver 2 incorporating all your suggestions.
Quoted 5 lines · Christoph Hellwig
I think just struct "struct
scsi_eh_save*save" is descriptive enough and almost fits on a line as well.. Also continuation of the prototype is indented by two tabs normally.
I think you can kill the old prefixes in the struct, they're saved per defintion.
Done
Quoted 3 lines · Alan Stern
A trivial problem is that quilt complains about patch 3, which leaves extra whitespace at the end of line 591 in
transport.c.
Thanks
Quoted 4 lines
More seriously, why make the caller define a static
generic_sensearray? Why not put the array inscsi_eh_prep_cmnd(), where it can be used whenevercopy_senseis nonzero?
I hope you like my solution
Quoted 3 lines
The line initializing the sense buffer to zero should be inside the
copy_senseconditional block.
Thanks
Quoted 4 lines
The code setting the LUN bits in scmd[1] is wrong. It should be like the code in
scsi_dispatch_cmd(); i.e., those bits should not be set if thescsi_levelisSCSI_UNKNOWN.
Thanks, good catch
Quoted 6 lines
Finally, a really serious problem. The code sets the buffer length for the REQUEST SENSE command to be the length of
scmd->sense_buffer, which is 96. But many USB devices won't work properly unless the requesed sense data length is 18. The appropriate adjustment must be added totransport.cin patch 3.
I was afraid of that at the beginning. But testing both with a Seagate USB sata hard-disk and Sandisk 2Gb usb-stick. They both returned a short read of 56 but other wise were happy. So I thought it is leftovers from The time the sense buffer was driver allocated. Also the standard recommends a short-read behavior. But I have taken your advise to hart and changed the code accordingly. Please check me out.
----------------------------------------------------------------
In motivation to abstract scsi_cmnd members and insulate drivers/transports from scsi_cmnd internals. The last place left was the REQUEST_SENSE sequence when done synchronous, by drivers.
Also all these drivers would do a use_sg==0 command invocation, preventing from doing cleanups on these drivers/transports. So this patchset is left-overs from Christoph's 2.6.18 cleanups.
In this patchset I have re-factored scsi_error to make it possible for drivers to use an abstract (easy to use, I hope) API for invoking a REQUEST_SENSE command. The API is declared in scsi/scsi_eh.h
[patch 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd()
Move some code around and add new fixtures here. So next patch is left a Mechanical breakup of code.
[patch 2/5] scsi_error: Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE
Here new API is introduced in scsi/scsi_eh.h and mechanical move of code to the new functions.
[PATCH 3/5] usb: transport.c use scsi_eh API in REQUEST_SENSE execution
Use above API for drivers/usb/storage/transport.c. Now that last User of use_sg==0, we can do the USB storage cleanups.
[PATCH 4/5] NCR5380: Use scsi_eh API for REQUEST_SENSE invocation
All NCR5380 family of drivers used to send a REQUEST_SENSE command.
[PATCH 5/5] arm: fas216 Use scsi_eh API for REQUEST_SENSE invocation
drivers/scsi/arm/fas216.c need change also.
Please Test as much as possible, and report any problems/differences.
Boaz Harrosh