new scsi sense handling
Subsystems (37)
There are 3 usages of sense handling in drivers
1. sense is available in driver internal structure and is mem-copied to upper level
2. A CHECK_CONDITION status was returned and the driver uses the scsi_eh_prep_cmnd() for a REQUEST_SENSE invocation to the target. Then returning the sense in scsi_eh_return_cmnd(). A variation on this is when the driver does nothing the queue is frozen an the scsi watchdog timer does the above.
3. The underline host adapter does the REQUEST_SENSE and a pre-allocated and DMA mapped sense buffer receives the sense information from HW.
Now since all IO requests come with a sense set at request->sense the buffer at scsi_cmnd->sense_buffer can go away, and case 1 above can copy it's bits directly into it.
Inspection of all users of blk_execute_rq_nowait() shows that all users setup a sense buffer, but submitted code puts a BUG_ON to make sure new code does not break that.
for cases 2 and 3 above two new members in scsi host template will tell scsi-midlayer what the driver wants to do.
.sense_buffsize - will instruct a mempool_t to be allocated per host with sense buffers
of ".sense_buffsize" size.
.pre_allocate_sense - If set tells the midlayer to pre-allocate a sense buffer for each
command. When set the code will behave like today. If the sense allocation fails the command allocation also fails. (With one extra sense at mempool)
If .pre_allocate_sense is not set, then mempool_t is resized to accommodate one reserved buffer for each target. So a call to scsi_eh_prep_cmnd() will not fail to allocate a sense buffer even in low memory condition.
Case 1 drivers call - scsi_eh_cpy_sense() - to transfer their sense information to upper
layer. The drivers are completely abstracted from any future changes at the scsi and block sense handling.
Case 3 drivers call the new scsi_make_sense()/scsi_return_sense() API to retrieve
a pointer to an extra DMAable sense buffer/return it to free store. scsi_return_sense() will call scsi_eh_cpy_sense() to set new sense information into upper layer before freeing the buffer back to the mempool. If the driver properly set the .pre_allocate_sense flag & .sense_buffsize at host template then these calls are guaranteed to succeed.
Case 2 drivers continue to call scsi_eh_prep_cmnd()/scsi_eh_restore_cmnd() to reuse the
failing command for REQUEST_SENSE. Inside these functions scsi_error.c will call above scsi_make_sense()/scsi_return_sense() for the actual sense buffer allocation. In this case there is a guarantied, one reserved buffer per scsi_device. (.pre_allocate_sense not set). Drivers need to set proper size at .sense_buffsize in host template, for this to work.
Upper layer and ULDs that need to inspect the sense information can get to it
using the scsi_sense() accessor.
Submitted for Review and comments (RFC) a patcset for that effect.
- first patch will introduce the new API implemented over existing members:
scsi_eh: Define new API for sense handling
- first group of patches are the case 1 drivers:
scsi-drivers: Move to new sense API. The Trevial case
scsi-drivers: more drivers use new scsi_eh_cpy_sense() firewire & ieee1394: Simple convert to new scsi_eh_cpy_sense.
dpt_i2o: Use new scsi_eh_cpy_sense()
gdth: Use of scsi_eh API and sense accessors
qla2xxx: convert to new scsi_eh_cpy_sense API
isd200, transport: Use scsi_eh_cpy_sense API
ultrastor: Use scsi_eh_cpy_sense() API
usb/microtek: No special handling for REQUEST_SENSE command please
- second group are case 2 drivers:
libata: Use scsi_eh API for REQUEST_SENSE invocation 53c700: Use scsi_eh API for REQUEST_SENSE invocation
aic7xxx_old: Use scsi_eh API for REQUEST_SENSE invocation
dc395x: Use scsi_eh API for REQUEST_SENSE invocation
tmscsim: Use scsi_eh API for REQUEST_SENSE invocation Add .sense_buffsize to drivers that use scsi_eh_prep_cmnd
- third group are case 3:
BusLogic: Use of scsi_make_sense() API for DMA-able sense buffer
eata: Use of scsi_make_sense() API for DMA-able sense buffer
initio: Use of scsi_make_sense() API for DMA-able sense buffer
u14-34f: Use of scsi_make_sense() API for DMA-able sense buffer
- Then, some patches for the upper layer:
scsi_tgt: use of sense accessors
scsi upper layer use of sense accessors
- And finally, block and scsi-midlayer switch to new handling.
block: Minor changes to sense handling
scsi: New sense handling
These patches are over latest scsi-misc. They are completely *untested* and are for discussion only. I will debug and update in the near future but only if it is accepted in principle first.
Lots of maintainers' ACKs are needed and a good deal of testing time in mm so I think this is only 2.6.26 at the earliest.
Comment away ;)
Volunteers for testing needed!
Boaz
RFC PATCH 1/24 scsi_eh: Define new API for sense handling
This patch defines a new API for sense handling. All drivers will be converted to this API, before the sense handling implementation will change. API is as follows:
void scsi_eh_cpy_sense(struct scsi_cmnd *cmd, void* sense,
unsigned sense_bytes);
To be used by drivers, when they have sense-bits
and wants to send them to upper layer. Max size
need not be a concern, If upper layer does not have
enough space it will be automatically truncated.
u8 *scsi_make_sense(struct scsi_cmnd *cmd);
To be used by drivers, and scsi-midlayer. Returns a DMA-able sense buffer. Must be returned by scsi_return_sense(). It should never fail if .pre_allocate_sense && .sense_buffsize in host template where properly set.
the buffer is of shost->sense_buffsize long.
void *scsi_return_sense(struct scsi_cmnd *cmd, u8 *sb);
Frees and returns the sense to the upper layer, copying only what's necessary.
void scsi_eh_reset_sense(struct scsi_cmnd *cmd)
Should not be used or necessary.
const u8 *scsi_sense(struct scsi_cmnd *cmd)
Used by ULDs and for inspecting the returned sense, can not be modified. It is only valid after a call to scsi_eh_cpy_sense() or a call to scsi_return_sense(). Before that it will/should return an empty buffer.
New members at scsi host template:
.sense_buffsize - if a driver calls scsi_make_sense() or
scsi_eh_prep_cmnd(), This value should be none zero indicating the max sense size, the driver supports. In most cases it should be SCSI_SENSE_BUFFERSIZE.
If this value is zero the driver will only call scsi_eh_cpy_sense().
.pre_allocate_sense - if a Driver calls scsi_make_sense()
in .queuecommand for every cmnd, this should be set to true. In which case scsi_make_sense() will not fail because midlayer will fail the command allocation. If the drivers calls scsi_eh_prep_cmnd() then sense_buffsize is not Zero but this here is set to false.
--- drivers/scsi/scsi.c | 12 ++++++++++++ drivers/scsi/scsi_error.c | 7 +++++++ include/scsi/scsi_eh.h | 17 +++++++++++++++++ include/scsi/scsi_host.h | 15 +++++++++++++++ 4 files changed, 51 insertions(+), 0 deletions(-)
drivers/scsi/scsi.c +12 -0
@@ -290,6 +290,18 @@ void scsi_put_command(struct scsi_cmnd *cmd) }EXPORT_SYMBOL (scsi_put_command); +u8 *scsi_make_sense (struct scsi_cmnd *cmd) +{ +return cmd->sense_buffer; +} +EXPORT_SYMBOL (scsi_make_sense); + +void scsi_return_sense (struct scsi_cmnd *cmd,u8 *sb) +{ +BUG_ON (cmd->sense_buffer != sb); +} +EXPORT_SYMBOL (scsi_return_sense); +/** * scsi_setup_command_freelist - Setup the command freelist for a scsi host. * @shost: host to allocate the freelist for.
drivers/scsi/scsi_error.c +7 -0
@@ -588,6 +588,13 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)scsi_try_host_reset (scmd); } +void scsi_eh_cpy_sense (struct scsi_cmnd *cmd,void *sense,unsigned sense_bytes) +{ +unsigned len =min_t (unsigned, sense_bytes,SCSI_SENSE_BUFFERSIZE ); +memcpy (cmd->sense_buffer, sense, len); +} +EXPORT_SYMBOL (scsi_eh_cpy_sense); +/** * scsi_eh_prep_cmnd - Save a scsi command info as part of error recory * @scmd: SCSI command structure to hijack
include/scsi/scsi_eh.h +17 -0
@@ -87,4 +87,21 @@ extern void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,extern void scsi_eh_restore_cmnd (struct scsi_cmnd * scmd,struct scsi_eh_save *ses); +extern void scsi_eh_cpy_sense (struct scsi_cmnd *cmd,void *sense, +unsigned sense_bytes); + +extern u8 *scsi_make_sense (struct scsi_cmnd *cmd); +extern void scsi_return_sense (struct scsi_cmnd *cmd,u8 *sb); + +/*FIXME: don't use, it's temporary */ +static inline void scsi_eh_reset_sense (struct scsi_cmnd *cmd) +{ +memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); +} + +static inline const u8 *scsi_sense (struct scsi_cmnd *cmd) +{ +return cmd->sense_buffer; +} +#endif /* _SCSI_SCSI_EH_H */
include/scsi/scsi_host.h +15 -0
@@ -443,6 +443,18 @@ struct scsi_host_template { unsigned ordered_tag:1;/* +* IF sense_buffsize != 0 then this bit asks to pre-allocate a sense +* buffer for each command. +*/ + unsigned pre_allocate_sense:1; + +/* +* IF sense_buffsize != 0 then a mem_cache_pool is allocated for the +* host with buffers of this size, with 1 pre-allocated buffer. +*/ +unsigned sense_buffsize; + +/* * Countdown for host blocking with no commands outstanding */ unsigned int max_host_blocked;
@@ -609,6 +621,9 @@ struct Scsi_Host {/* Asynchronous scan in progress */ unsigned async_scan:1; +/* actual allocated sense_buffsize for this host */ +unsigned sense_buffsize; +/* * Optional work queue to be utilized by the transport */
Signature · 1 line
1.5.3.3
James Bottomley
maintainerRe: [PATCH 1/24][RFC] scsi_eh: Define new API for sense handling
Quoted 40 lines · Boaz Harrosh
This patch defines a new API for sense handling. All drivers will
be converted to this API, before the sense handling implementation will change. API is as follows:void scsi_eh_cpy_sense(struct
scsi_cmnd*cmd, void* sense,
unsignedsense_bytes);
To be used by drivers, when they have sense-bits
and wants to send them to upper layer. Max size
need not be a concern, If upper layer does not have
enough space it will be automatically truncated.u8 *scsi_make_sense(struct
scsi_cmnd*cmd);
To be used by drivers, and scsi-midlayer. Returns a DMA-able sense buffer. Must be returned byscsi_return_sense(). It should never fail if .pre_allocate_sense && .sense_buffsize in host template where properly set.
the buffer is ofshost->sense_buffsizelong.void *scsi_return_sense(struct
scsi_cmnd*cmd, u8 *sb);
Frees and returns the sense to the upper layer, copying only what's necessary.void
scsi_eh_reset_sense(struct scsi_cmnd *cmd)
Should not be used or necessary.const u8 *scsi_sense(struct
scsi_cmnd*cmd)
Used by ULDs and for inspecting the returned sense, can not be modified. It is only valid after a call toscsi_eh_cpy_sense()or a call toscsi_return_sense(). Before that it will/should return an empty buffer.New members at scsi host template:
.sense_buffsize - if a driver callsscsi_make_sense()or
scsi_eh_prep_cmnd(), This value should be none zero indicating the max sense size, the driver supports. In most cases it should beSCSI_SENSE_BUFFERSIZE.
If this value is zero the driver will only callscsi_eh_cpy_sense().
Quoted 8 lines
.pre_allocate_sense - if a Driver calls
scsi_make_sense()
in .queuecommand for every cmnd, this should be set to true. In which casescsi_make_sense()will not fail because midlayer will fail the command allocation. If the drivers callsscsi_eh_prep_cmnd()thensense_buffsizeis not Zero but this here is set to false.
My initial reaction to this is that you're doing too many contortions to ensure something we don't particularly care about: whether we can allocate a sense buffer atomically or not.
What all this code should be doing is simply allocating the sense buffer in scsi_eh_prep_cmnd() using tomo's existing slab (and GFP_ATOMIC) if that fails, we need a return from scsi_eh_prep_cmnd() telling us. At that point, the driver should abandon the auto request sense attempt and instead just return the CC/UA without the DRIVER_SENSE bit set which will trigger the eh to collect the sense for us.
Ideally, doing it this way might mean we could even dump the sense_buffer pointer from the command (although I don't see that as necessary).
This solves the 99% case without getting into preallocation contortions.
James
Re: [PATCH 1/24][RFC] scsi_eh: Define new API for sense handling
Quoted 50 lines · James Bottomley
Quoted 40 lines · Boaz Harrosh
This patch defines a new API for sense handling. All drivers will
be converted to this API, before the sense handling implementation will change. API is as follows:void scsi_eh_cpy_sense(struct
scsi_cmnd*cmd, void* sense,
unsignedsense_bytes);
To be used by drivers, when they have sense-bits
and wants to send them to upper layer. Max size
need not be a concern, If upper layer does not have
enough space it will be automatically truncated.u8 *scsi_make_sense(struct
scsi_cmnd*cmd);
To be used by drivers, and scsi-midlayer. Returns a DMA-able sense buffer. Must be returned byscsi_return_sense(). It should never fail if .pre_allocate_sense && .sense_buffsize in host template where properly set.
the buffer is ofshost->sense_buffsizelong.void *scsi_return_sense(struct
scsi_cmnd*cmd, u8 *sb);
Frees and returns the sense to the upper layer, copying only what's necessary.void
scsi_eh_reset_sense(struct scsi_cmnd *cmd)
Should not be used or necessary.const u8 *scsi_sense(struct
scsi_cmnd*cmd)
Used by ULDs and for inspecting the returned sense, can not be modified. It is only valid after a call toscsi_eh_cpy_sense()or a call toscsi_return_sense(). Before that it will/should return an empty buffer.New members at scsi host template:
.sense_buffsize - if a driver callsscsi_make_sense()or
scsi_eh_prep_cmnd(), This value should be none zero indicating the max sense size, the driver supports. In most cases it should beSCSI_SENSE_BUFFERSIZE.
If this value is zero the driver will only callscsi_eh_cpy_sense().Quoted 8 lines
.pre_allocate_sense - if a Driver calls
scsi_make_sense()
in .queuecommand for every cmnd, this should be set to true. In which casescsi_make_sense()will not fail because midlayer will fail the command allocation. If the drivers callsscsi_eh_prep_cmnd()thensense_buffsizeis not Zero but this here is set to false.
Quoted 3 lines
My initial reaction to this is that you're doing too many contortions to ensure something we don't particularly care about: whether we can allocate a sense buffer atomically or not.
I hope that now, once you've actually seen the implementation, my motivation is clearer. Perhaps I explained it badly, but the actual code is pretty simple and contortions is not how I would describe it. The API above is just a way for drivers to say how they intend to behave, and the midlayer will accommodate easily. None of the solutions are hard and they are all simpler then what exists today. The only added complexity introduced is the initial choice.
Quoted 3 lines
What all this code should be doing is simply allocating the sense buffer in
scsi_eh_prep_cmnd()using tomo's existing slab (andGFP_ATOMIC)
This is what we are doing. Only allocating the sense buffer in the very unlikely event of the call to scsi_eh_prep_cmnd(). So we are in agreement here.
Quoted 6 lines
if
that fails, we need a return fromscsi_eh_prep_cmnd()telling us. At that point, the driver should abandon the auto request sense attempt and instead just return the CC/UA without theDRIVER_SENSEbit set which will trigger the eh to collect the sense for us.
This is a nightmare and a serious regression. It will cause an IO deadlock in the event of an IO error during an IO-to-free-memory scenario.
The memory footprint of a system running with my patchset, after the very first request, is the same as with the current (Post Tomo) code. Only thing is, my system will preallocate a bit more memory, 96 bytes, per device scanned. This happens anyway, currently, with Tomo's code as soon as the device is used the first time.
Preallocating the sense buffer during initialization eliminates the need to allocate it for every command, providing considerable performance and memory consumption benefits. All that without compromising robustness in the event of an IO error on heavily loaded systems.
Quoted 6 lines
Ideally, doing it this way might mean we could even dump the
sense_bufferpointer from the command (although I don't see that as necessary).This solves the 99% case without getting into preallocation contortions.
after the final patch you can see that I have ditched the sense_buffer pointer without sacrificing anything in reliability, and absolutely got rid of any sense allocation in the 99% of successful IO.
Quoted 2 lines
James
I apologize for not explaining myself well. I think the point of this patchset was missed. (I know it is me, because it happens to me often)
My primary motivation was as follows.
1. Get rid of the sense_buffer at scsi layer, and go directly to block layer sense buffer. But do this only once with a simple accessor that will let me have freedom of implementation later. (And simplify code, catch bugs and have a central point of control)
2. In the less common scenario of these drivers that need to DMA into the sense buffer, Only allocate the buffer in the error path but make sure that the system stays as robust as today. With hopefully no memory footprint penalty.
3. Since I need to change a bunch of drivers, and there are these rare drivers that absolutely need a DMA-able sense buffer before hand, and since there are more then two of them, so for the sake of reusable code: Have utility functions for these drivers as well. Even more so, if the same exact code can also be reused by the other code paths, but with a different usage model.
4. I had one more motivation which is not immediately apparent but is included in this patchset, that is the: "Bigger than 96 bytes sense buffer". The scsi protocol defines the maximum possible sense buffer to be of 260 bytes. One-byte 4-bytes-aligned length specifier at offset 7 of header. hence 252+8. And guess what protocol needs these kind of sizes? you guessed right OSD. With submitted code an OSD ULD (like bsg) would allocate a 260 bytes buffer put 260 at request->max_sense_len. The iSCSI initiator that does a scsi_eh_cpy_sense() directly from network buffer at specified size will just copy all of it. Thanks to new request->max_sense_len and the new scsi_eh_cpy_sense() that does the right job. All that with a minimal change (cleanup) to iscsi driver.
And hence what came out.
The patchset presented is just a pragmatic solution to the constraints and motivations presented above, with "minimum change and maximum safety at drivers" mindset. When I began this work I tended to do more work at the driver level, but very fast I got lazy and scared from a "gdth effect", and settled on a good solid infrastructure at the mid-layer but optional so it will only affect the more needy drivers.
Any new Ideas are welcome.
There are a couple of things to talk about, specifically the two WARN_ONs, one at scsi_sense(), and one at scsi_make_sense() (See last patch) but lets do this later. See how above is received first.
Thanks
Boaz
James Bottomley
maintainerRe: [PATCH 1/24][RFC] scsi_eh: Define new API for sense handling
Quoted 55 lines · Boaz Harrosh
Quoted 50 lines · James Bottomley
Quoted 40 lines · Boaz Harrosh
This patch defines a new API for sense handling. All drivers will
be converted to this API, before the sense handling implementation will change. API is as follows:void scsi_eh_cpy_sense(struct
scsi_cmnd*cmd, void* sense,
unsignedsense_bytes);
To be used by drivers, when they have sense-bits
and wants to send them to upper layer. Max size
need not be a concern, If upper layer does not have
enough space it will be automatically truncated.u8 *scsi_make_sense(struct
scsi_cmnd*cmd);
To be used by drivers, and scsi-midlayer. Returns a DMA-able sense buffer. Must be returned byscsi_return_sense(). It should never fail if .pre_allocate_sense && .sense_buffsize in host template where properly set.
the buffer is ofshost->sense_buffsizelong.void *scsi_return_sense(struct
scsi_cmnd*cmd, u8 *sb);
Frees and returns the sense to the upper layer, copying only what's necessary.void
scsi_eh_reset_sense(struct scsi_cmnd *cmd)
Should not be used or necessary.const u8 *scsi_sense(struct
scsi_cmnd*cmd)
Used by ULDs and for inspecting the returned sense, can not be modified. It is only valid after a call toscsi_eh_cpy_sense()or a call toscsi_return_sense(). Before that it will/should return an empty buffer.New members at scsi host template:
.sense_buffsize - if a driver callsscsi_make_sense()or
scsi_eh_prep_cmnd(), This value should be none zero indicating the max sense size, the driver supports. In most cases it should beSCSI_SENSE_BUFFERSIZE.
If this value is zero the driver will only callscsi_eh_cpy_sense().Quoted 8 lines
.pre_allocate_sense - if a Driver calls
scsi_make_sense()
in .queuecommand for every cmnd, this should be set to true. In which casescsi_make_sense()will not fail because midlayer will fail the command allocation. If the drivers callsscsi_eh_prep_cmnd()thensense_buffsizeis not Zero but this here is set to false.Quoted 3 lines
My initial reaction to this is that you're doing too many contortions to ensure something we don't particularly care about: whether we can allocate a sense buffer atomically or not.
Quoted 7 lines
I hope that now, once you've actually seen the implementation, my motivation is clearer. Perhaps I explained it badly, but the actual code is pretty simple and contortions is not how I would describe it. The API above is just a way for drivers to say how they intend to behave, and the midlayer will accommodate easily. None of the solutions are hard and they are all simpler then what exists today. The only added complexity introduced is the initial choice.
Well, actually now it's clearer that this is at least three patch sets rolled into one:
1. Alter the sense allocation method (The one I'm not keen on)
2. Allow the sense buffer size to be increased
3. A block change to allow variable sense lengths.
So, confining the remarks to 1. only
Your API could do with a bit of name tuning. scsi_make_sense() is definitely an API certainly linux people have been yearning after for a while ... I'm just not sure you satisfy them in that regard ...
The usual API naming would either be _get/_put based or _alloc/_free based.
Quoted 7 lines
Quoted 3 lines
What all this code should be doing is simply allocating the sense buffer in
scsi_eh_prep_cmnd()using tomo's existing slab (andGFP_ATOMIC)This is what we are doing. Only allocating the sense buffer in the very unlikely event of the call to
scsi_eh_prep_cmnd(). So we are in agreement here.
No, you're not ... you're preallocating in the command get for most hosts that don't have an existing sense buffer in their descriptor structure. For the others, I think the logic is flawed ... see below.
Quoted 9 lines
Quoted 6 lines
if
that fails, we need a return fromscsi_eh_prep_cmnd()telling us. At that point, the driver should abandon the auto request sense attempt and instead just return the CC/UA without theDRIVER_SENSEbit set which will trigger the eh to collect the sense for us.This is a nightmare and a serious regression. It will cause an IO deadlock in the event of an IO error during an IO-to-free-memory scenario.
Why? If that were true, hosts that don't do auto request sense would lock up today which they don't seem to be doing.
The point about only allocating sense at use is that for huge queue depths we don't have hundreds of unnecessary sense buffer allocations.
mempool backed slab allocations are guaranteed to succeed at least to the mempool depth; however, you only really need a depth of one for forward progress. After that, you're allocating from a slab at GFP_ATOMIC which succeeds in most cases.
So your 99% case is a single sense allocation which goes through the fast path. The 1% case is where you need two or more sense buffers in flight at once and if the GFP_ATOMIC slab allocation fails (probably only 1 time in 100 or so) you fall back to the slow path and make the eh collect sense.
Quoted 4 lines
The memory footprint of a system running with my patchset, after the very first request, is the same as with the current (Post Tomo) code. Only thing is, my system will preallocate a bit more memory, 96 bytes, per device scanned. This happens anyway, currently, with Tomo's code as soon as the device is used the first time.
It looks to me like your pools are host based rather than global, so that's one extra sense buffer per *host* rather than device, isn't it? Regardless, (and straying into 2. which I didn't really want to do) if we're going to increase the sense buffer size, it strikes me there are really only two values anyone would be interested in: 96 and 260, so we could just have global pools providing those two sizes. Plus, for preallocation, since you're dependent on the non mempool backed slab allocation of commands before you allocate the sense, using mempool backing doesn't really buy anything except complexity.
Quoted 4 lines
Preallocating the sense buffer during initialization eliminates the need to allocate it for every command, providing considerable performance and memory consumption benefits. All that without compromising robustness in the event of an IO error on heavily loaded systems.
OK, perhaps I've missed something. scsi_make_sense() does the sense allocation and in scsi_get_command() you have
+ if (cmd->device->host->hostt->pre_allocate_sense) {
+ u8 *sb;
+
+ sb = scsi_make_sense(cmd);
+ if (unlikely(!sb)) {
+ scsi_put_command(cmd);
+ cmd = NULL;
+ }
That looks like one allocation per command to me when this pre_allocate_sense flag is set.
Quoted 10 lines
Quoted 6 lines
Ideally, doing it this way might mean we could even dump the
sense_bufferpointer from the command (although I don't see that as necessary).This solves the 99% case without getting into preallocation contortions.
after the final patch you can see that I have ditched the
sense_bufferpointer without sacrificing anything in reliability, and absolutely got rid of any sense allocation in the 99% of successful IO.
Only for drivers which already separately allocate sense buffers ... which is good, don't get me wrong. I just want the benefits for the remaining drivers.
Quoted 11 lines
Quoted 2 lines
James
I apologize for not explaining myself well. I think the point of this patchset was missed. (I know it is me, because it happens to me often)
My primary motivation was as follows.
1. Get rid of thesense_bufferat scsi layer, and go directly to block layer sense buffer. But do this only once with a simple accessor that will let me have freedom of implementation later. (And simplify code, catch bugs and have a central point of control)
Yes, that's fine.
Quoted 3 lines
2. In the less common scenario of these drivers that need to DMA into the sense buffer, Only allocate the buffer in the error path but make sure that the system stays as robust as today. With hopefully no memory footprint penalty.
OK, this is another piece I'm not keen on. It seems to me you're relying on the mempool to always succeed. The problem is that the contingent allegiance condition is cleared as soon as sense is requested. This means that other in-flight commands can now generate sense requests. You're not guaranteed that the next interrupt is the actual sense returning instead of another unreleated command returned by the releasing of the contingent allegiance condition ... and if that one needs sense, you can now run out of buffers and NULL deref in the code.
Quoted 5 lines
3. Since I need to change a bunch of drivers, and there are these rare drivers that absolutely need a DMA-able sense buffer before hand, and since there are more then two of them, so for the sake of reusable code: Have utility functions for these drivers as well. Even more so, if the same exact code can also be reused by the other code paths, but with a different usage model.
Just combine your points 2 and 3. Always alloc sense in scsi_eh_prep_cmnd() at GFP_ATOMIC with mempool backing (or really, with a prefilled slab is probably good enough) and return failure from scsi_eh_prep_cmnd() if you can't. It will simplify the code enormously ... really!
Quoted 19 lines
4. I had one more motivation which is not immediately apparent but is included in this patchset, that is the: "Bigger than 96 bytes sense buffer". The scsi protocol defines the maximum possible sense buffer to be of 260 bytes. One-byte 4-bytes-aligned length specifier at offset 7 of header. hence 252+8. And guess what protocol needs these kind of sizes? you guessed right OSD. With submitted code an OSD ULD (like bsg) would allocate a 260 bytes buffer put 260 at
request->max_sense_len. The iSCSI initiator that does ascsi_eh_cpy_sense()directly from network buffer at specified size will just copy all of it. Thanks to newrequest->max_sense_lenand the newscsi_eh_cpy_sense()that does the right job. All that with a minimal change (cleanup) to iscsi driver.And hence what came out.
The patchset presented is just a pragmatic solution to the constraints and motivations presented above, with "minimum change and maximum safety at drivers" mindset. When I began this work I tended to do more work at the driver level, but very fast I got lazy and scared from a "gdth effect", and settled on a good solid infrastructure at the mid-layer but optional so it will only affect the more needy drivers.
Quoted 5 lines
Any new Ideas are welcome.
There are a couple of things to talk about, specifically the two
WARN_ONs, one atscsi_sense(), and one atscsi_make_sense()(See last patch) but lets do this later. See how above is received first.
James
RFC PATCH 2/24 scsi-drivers: Move to new sense API. The Trevial case
In reply to Boaz Harrosh All these drivers are trevialy converted from memcpy into command's sense_buffer from a driver private area, to the new scsi_eh_cpy_sense() API. Some also do amaturistic sense editing or printing.
FIXME: weed out these drivers that this patch is a bugfix for.
(copy more than what they have at private area)
The list of converted drivers:
arch/ia64/hp/sim/simscsi.c drivers/block/cciss_scsi.c drivers/infiniband/ulp/srp/ib_srp.c drivers/message/fusion/mptscsih.c drivers/message/i2o/i2o_scsi.c drivers/scsi/3w-9xxx.c drivers/scsi/a100u2w.c drivers/scsi/aha1542.c drivers/scsi/aha1740.c drivers/scsi/atp870u.c drivers/scsi/hptiop.c drivers/scsi/ibmvscsi/ibmvscsi.c drivers/scsi/ibmvscsi/ibmvstgt.c drivers/scsi/ide-scsi.c drivers/scsi/libiscsi.c drivers/scsi/libsas/sas_scsi_host.c drivers/scsi/lpfc/lpfc_scsi.c drivers/scsi/megaraid.c drivers/scsi/megaraid/megaraid_sas.c drivers/scsi/ncr53c8xx.c drivers/scsi/qlogicpti.c drivers/scsi/scsi_debug.c drivers/scsi/sym53c8xx_2/sym_glue.c
--- arch/ia64/hp/sim/simscsi.c | 8 ++++++-- drivers/block/cciss_scsi.c | 8 +++----- drivers/infiniband/ulp/srp/ib_srp.c | 6 +++--- drivers/message/fusion/mptscsih.c | 9 +++++---- drivers/message/i2o/i2o_scsi.c | 4 ++-- drivers/scsi/3w-9xxx.c | 5 ++++- drivers/scsi/a100u2w.c | 4 ++-- drivers/scsi/aha1542.c | 4 ++-- drivers/scsi/aha1740.c | 4 ++-- drivers/scsi/atp870u.c | 1 - drivers/scsi/hptiop.c | 6 +++--- drivers/scsi/ibmvscsi/ibmvscsi.c | 4 +--- drivers/scsi/ibmvscsi/ibmvstgt.c | 2 +- drivers/scsi/ide-scsi.c | 4 +++- drivers/scsi/libiscsi.c | 6 ++---- drivers/scsi/libsas/sas_scsi_host.c | 3 +-- drivers/scsi/lpfc/lpfc_scsi.c | 9 ++++----- drivers/scsi/megaraid.c | 14 +++++++++----- drivers/scsi/megaraid/megaraid_sas.c | 6 +++--- drivers/scsi/ncr53c8xx.c | 9 ++++----- drivers/scsi/qlogicpti.c | 4 ++-- drivers/scsi/scsi_debug.c | 5 ++--- drivers/scsi/sym53c8xx_2/sym_glue.c | 5 ++--- 23 files changed, 66 insertions(+), 64 deletions(-)
arch/ia64/hp/sim/simscsi.c +6 -2
@@ -326,9 +326,13 @@ simscsi_queuecommand (struct scsi_cmnd *sc, void (*done)(struct scsi_cmnd *)) } }if (sc->result ==DID_BAD_TARGET ) { +u8 sense_buffer[3 ]; sc->result |=DRIVER_SENSE <<24 ; - sc->sense_buffer[0 ] =0x70 ; - sc->sense_buffer[2 ] =0x00 ; + + sense_buffer[0 ] =0x70 ; + sense_buffer[1 ] =0 ; + sense_buffer[2 ] =0x00 ; +scsi_eh_cpy_sense (sc, sense_buffer,sizeof (sense_buffer)); }if (atomic_read (&num_reqs) >=SIMSCSI_REQ_QUEUE_LEN ) {panic ("Attempt to queue command while command is pending!!" );
drivers/block/cciss_scsi.c +3 -5
@@ -37,7 +37,8 @@#include <scsi/scsi_cmnd.h> #include <scsi/scsi_device.h> -#include <scsi/scsi_host.h> +#include <scsi/scsi_host.h> +#include <scsi/scsi_eh.h> #include "cciss_scsi.h"
@@ -579,10 +580,7 @@ complete_scsi_command( CommandList_struct *cp, int timeout, __u32 tag)/* copy the sense data whether we need to or not. */ -memcpy (cmd->sense_buffer, ei->SenseInfo, - ei->SenseLen >SCSI_SENSE_BUFFERSIZE ? -SCSI_SENSE_BUFFERSIZE : - ei->SenseLen); +scsi_eh_cpy_sense (cmd, ei->SenseInfo, ei->SenseLen);scsi_set_resid (cmd, ei->ResidualCnt);if (ei->CommandStatus !=0 )
drivers/infiniband/ulp/srp/ib_srp.c +3 -3
@@ -48,6 +48,7 @@#include <scsi/scsi_dbg.h> #include <scsi/srp.h> #include <scsi/scsi_transport_srp.h> +#include <scsi/scsi_eh.h> #include <rdma/ib_cache.h>
@@ -798,10 +799,9 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp) scmnd->result = rsp->status;if (rsp->flags &SRP_RSP_FLAG_SNSVALID ) { -memcpy (scmnd->sense_buffer, rsp->data + +scsi_eh_cpy_sense (scmnd, rsp->data +be32_to_cpu (rsp->resp_data_len), -min_t (int,be32_to_cpu (rsp->sense_data_len), -SCSI_SENSE_BUFFERSIZE )); +be32_to_cpu (rsp->sense_data_len)); }if (rsp->flags & (SRP_RSP_FLAG_DOOVER |SRP_RSP_FLAG_DOUNDER ))
drivers/message/fusion/mptscsih.c +5 -4
@@ -61,6 +61,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/scsi_dbg.h> +#include <scsi/scsi_eh.h> #include "mptbase.h" #include "mptscsih.h"
@@ -606,9 +607,9 @@ mptscsih_info_scsiio(MPT_ADAPTER *ioc, struct scsi_cmnd *sc, SCSIIOReply_t * pSc pScsiReply->SCSIState);if (pScsiReply->SCSIState &MPI_SCSI_STATE_AUTOSENSE_VALID ) { - skey = sc->sense_buffer[2 ] &0x0F ; - asc = sc->sense_buffer[12 ]; - ascq = sc->sense_buffer[13 ]; + skey =scsi_sense (sc)[2 ] &0x0F ; + asc =scsi_sense (sc)[12 ]; + ascq =scsi_sense (sc)[13 ];printk (MYIOC_s_DEBUG_FMT"\t[sense_key,asc,ascq]: " "[0x%02x,0x%02x,0x%02x]\n" , ioc->name, skey, asc, ascq);
@@ -2501,7 +2502,7 @@ mptscsih_copy_sense_data(struct scsi_cmnd *sc, MPT_SCSI_HOST *hd, MPT_FRAME_HDR/* Copy the sense received into the scsi command block. */ req_index =le16_to_cpu (mf->u.frame.hwhdr.msgctxu.fld.req_idx); sense_data = ((u8 *)ioc->sense_buf_pool + (req_index *MPT_SENSE_BUFFER_ALLOC )); -memcpy (sc->sense_buffer, sense_data,SNS_LEN (sc)); +scsi_eh_cpy_sense (sc, sense_data,MPT_SENSE_BUFFER_ALLOC );/* Log SMART data (asc = 0x5D, non-IM case only) if required. */
drivers/message/i2o/i2o_scsi.c +2 -2
@@ -65,6 +65,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_device.h> #include <scsi/scsi_cmnd.h> +#include <scsi/scsi_eh.h> #include <scsi/sg.h> #define OSM_NAME "scsi-osm"
@@ -369,8 +370,7 @@ static int i2o_scsi_reply(struct i2o_controller *c, u32 m,* the SCSI layer handle the error */ if (cmd->result) -memcpy (cmd->sense_buffer, &msg->body[3 ], -min (SCSI_SENSE_BUFFERSIZE ,40 )); +scsi_eh_cpy_sense (cmd, &msg->body[3 ],40 );/* only output error code if AdapterStatus is not HBA_SUCCESS */ if ((error >>8 ) &0xff )
drivers/scsi/3w-9xxx.c +4 -1
@@ -91,6 +91,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/scsi_cmnd.h> +#include <scsi/scsi_eh.h> #include "3w-9xxx.h" /* Globals */
@@ -992,7 +993,9 @@ static int twa_fill_sense(TW_Device_Extension *tw_dev, int request_id, int copy_ }if (copy_sense) { -memcpy (tw_dev->srb[request_id]->sense_buffer, full_command_packet->header.sense_data,TW_SENSE_DATA_LENGTH ); +scsi_eh_cpy_sense (tw_dev->srb[request_id], + full_command_packet->header.sense_data, +TW_SENSE_DATA_LENGTH ); tw_dev->srb[request_id]->result = (full_command_packet->command.newcommand.status <<1 ); retval =TW_ISR_DONT_RESULT ; goto out;
drivers/scsi/a100u2w.c +2 -2
@@ -79,6 +79,7 @@#include <scsi/scsi_cmnd.h> #include <scsi/scsi_device.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_eh.h> #include "a100u2w.h"
@@ -1028,8 +1029,7 @@ static void inia100_scb_handler(struct orc_host *host, struct orc_scb *scb) }if (scb->tastat ==2 ) {/* Check condition */ -memcpy ((unsigned char *) &cmd->sense_buffer[0 ], - (unsigned char *) &escb->sglist[0 ],SENSE_SIZE ); +scsi_eh_cpy_sense (cmd, &escb->sglist[0 ],SENSE_SIZE ); } cmd->result = scb->tastat | (scb->hastat <<16 );scsi_dma_unmap (cmd);
drivers/scsi/aha1542.c +2 -2
@@ -535,8 +535,8 @@ static void aha1542_intr_handle(struct Scsi_Host *shost, void *dev_id)Adaptec automatically fetches it, and there is no guarantee that we will still have it in the cdb when we come back */ if (ccb[mbo].tarstat ==2 ) -memcpy (SCtmp->sense_buffer, &ccb[mbo].cdb[ccb[mbo].cdblen], -SCSI_SENSE_BUFFERSIZE ); +scsi_eh_cpy_sense (SCtmp, &ccb[mbo].cdb[ccb[mbo].cdblen], + ~0 );/*FIXME: What is the card's max space here*/ /* is there mail :-) */
drivers/scsi/aha1740.c +2 -2
@@ -285,8 +285,8 @@ static irqreturn_t aha1740_intr_handle(int irq, void *dev_id)guarantee that we will still have it in the cdb when we come back */ if ( (adapstat &G2INTST_MASK ) ==G2INTST_CCBERROR ) { -memcpy (SCtmp->sense_buffer, ecbptr->sense, -SCSI_SENSE_BUFFERSIZE ); +scsi_eh_cpy_sense (SCtmp, ecbptr->sense, +sizeof (ecbptr->sense)); errstatus =aha1740_makecode (ecbptr->sense,ecbptr->status); }else errstatus =0 ;
drivers/scsi/atp870u.c +0 -1
@@ -613,7 +613,6 @@ static int atp870u_queuecommand(struct scsi_cmnd * req_p,struct Scsi_Host *host; c =scmd_channel (req_p); - req_p->sense_buffer[0 ]=0 ;scsi_set_resid (req_p,0 );if (scmd_channel (req_p) >1 ) { req_p->result =0x00040000 ;
drivers/scsi/hptiop.c +3 -3
@@ -34,6 +34,7 @@#include <scsi/scsi.h> #include <scsi/scsi_tcq.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_eh.h> #include "hptiop.h"
@@ -573,9 +574,8 @@ static void hptiop_finish_scsi_req(struct hptiop_hba *hba, u32 tag,scsi_set_resid (scp,scsi_bufflen (scp) -le32_to_cpu (req->dataxfer_length)); scp->result =SAM_STAT_CHECK_CONDITION ; -memcpy (scp->sense_buffer, &req->sg_list, -min_t (size_t,SCSI_SENSE_BUFFERSIZE , -le32_to_cpu (req->dataxfer_length))); +scsi_eh_cpy_sense (scp, &req->sg_list, +le32_to_cpu (req->dataxfer_length));break ;default :
drivers/scsi/ibmvscsi/ibmvscsi.c +1 -3
@@ -688,9 +688,7 @@ static void handle_cmd_rsp(struct srp_event_struct *evt_struct)if (cmnd) { cmnd->result = rsp->status;if (((cmnd->result >>1 ) &0x1f ) ==CHECK_CONDITION ) -memcpy (cmnd->sense_buffer, - rsp->data, - rsp->sense_data_len); +scsi_eh_cpy_sense (cmnd, rsp->data, rsp->sense_data_len);unmap_cmd_data (&evt_struct->iu.srp.cmd, evt_struct, evt_struct->hostdata->dev);
drivers/scsi/ibmvscsi/ibmvstgt.c +1 -1
@@ -166,7 +166,7 @@ static int send_rsp(struct iu_entry *iue, struct scsi_cmnd *sc,if (sc) { iu->srp.rsp.flags |=SRP_RSP_FLAG_SNSVALID ; iu->srp.rsp.sense_data_len =SCSI_SENSE_BUFFERSIZE ; -memcpy (sense, sc->sense_buffer,SCSI_SENSE_BUFFERSIZE ); +memcpy (sense,scsi_sense (sc),SCSI_SENSE_BUFFERSIZE ); }else { iu->srp.rsp.status =SAM_STAT_CHECK_CONDITION ; iu->srp.rsp.flags |=SRP_RSP_FLAG_SNSVALID ;
drivers/scsi/ide-scsi.c +3 -1
@@ -57,6 +57,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/sg.h> +#include <scsi/scsi_eh.h> #define IDESCSI_DEBUG_LOG 0
@@ -333,7 +334,8 @@ static int idescsi_end_request (ide_drive_t *drive, int uptodate, int nrsecs)printk ("ide-scsi: %s: wrap up check %lu, rst = " , drive->name, opc->scsi_cmd->serial_number);ide_scsi_hex_dump (pc->buffer,16 ); } -memcpy ((void *) opc->scsi_cmd->sense_buffer, pc->buffer,SCSI_SENSE_BUFFERSIZE ); +scsi_eh_cpy_sense (opc->scsi_cmd, pc->buffer, +SCSI_SENSE_BUFFERSIZE );kfree (pc->buffer);kfree (pc);kfree (rq);
drivers/scsi/libiscsi.c +2 -4
@@ -513,10 +513,8 @@ invalid_datalen:if (datalen < senselen) goto invalid_datalen; -memcpy (sc->sense_buffer, data +2 , -min_t (uint16_t, senselen,SCSI_SENSE_BUFFERSIZE )); -debug_scsi ("copied %d bytes of sense\n" , -min_t (uint16_t, senselen,SCSI_SENSE_BUFFERSIZE )); +scsi_eh_cpy_sense (sc, data +2 , senselen); +debug_scsi ("copied %d bytes of sense\n" , senselen); } if (scsi_bidi_cmnd(sc) &&
drivers/scsi/libsas/sas_scsi_host.c +1 -2
@@ -107,8 +107,7 @@ static void sas_scsi_task_done(struct sas_task *task) hs =DID_ABORT ;break ;case SAM_CHECK_COND : -memcpy (sc->sense_buffer, ts->buf, -min (SCSI_SENSE_BUFFERSIZE , ts->buf_valid_size)); +scsi_eh_cpy_sense (sc, ts->buf, ts->buf_valid_size); stat =SAM_CHECK_COND ;break ;default :
drivers/scsi/lpfc/lpfc_scsi.c +4 -5
@@ -28,6 +28,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/scsi_transport_fc.h> +#include <scsi/scsi_eh.h> #include "lpfc_version.h" #include "lpfc_hw.h"
@@ -424,14 +425,12 @@ lpfc_handle_fcp_err(struct lpfc_vport *vport, struct lpfc_scsi_buf *lpfc_cmd,if ((resp_info &SNS_LEN_VALID ) && fcprsp->rspSnsLen) {uint32_t snslen =be32_to_cpu (fcprsp->rspSnsLen); -if (snslen >SCSI_SENSE_BUFFERSIZE ) - snslen =SCSI_SENSE_BUFFERSIZE ;if (resp_info &RSP_LEN_VALID ) rsplen =be32_to_cpu (fcprsp->rspRspLen); -memcpy (cmnd->sense_buffer, &fcprsp->rspInfo0 + rsplen, snslen); +scsi_eh_cpy_sense (cmnd, &fcprsp->rspInfo0 + rsplen, snslen); } - lp = (uint32_t *)cmnd->sense_buffer; + lp = (uint32_t *)scsi_sense (cmnd);if (!scsi_status && (resp_info &RESID_UNDER )) logit =LOG_FCP ;
@@ -593,7 +592,7 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pIocbIn, }if (cmd->result || lpfc_cmd->fcp_rsp->rspSnsLen) { -uint32_t *lp = (uint32_t *)cmd->sense_buffer; +const uint32_t *lp = (const uint32_t *)scsi_sense (cmd);lpfc_printf_vlog (vport,KERN_INFO ,LOG_FCP , "0710 Iodone <%d/%d> cmd %p, error "
drivers/scsi/megaraid.c +9 -5
@@ -1572,8 +1572,7 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int nstatus, int status)if ( mbox->m_out.cmd ==MEGA_MBOXCMD_PASSTHRU || mbox->m_out.cmd ==MEGA_MBOXCMD_PASSTHRU64 ) { -memcpy (cmd->sense_buffer, pthru->reqsensearea, -14 ); +scsi_eh_cpy_sense (cmd, pthru->reqsensearea,14 ); cmd->result = (DRIVER_SENSE <<24 ) | (DID_OK << 16) |
@@ -1582,15 +1581,20 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int nstatus, int status)else {if (mbox->m_out.cmd ==MEGA_MBOXCMD_EXTPTHRU ) { -memcpy (cmd->sense_buffer, +scsi_eh_cpy_sense (cmd, epthru->reqsensearea,14 ); cmd->result = (DRIVER_SENSE <<24 ) | (DID_OK <<16 ) | (CHECK_CONDITION <<1 ); }else { - cmd->sense_buffer[0 ] =0x70 ; - cmd->sense_buffer[2 ] =ABORTED_COMMAND ; +u8 sense[4 ]; + +memset (sense,0 ,sizeof (sense)); + sense[0 ] =0x70 ; + sense[2 ] =ABORTED_COMMAND ; +scsi_eh_cpy_sense (cmd, sense, +sizeof (sense)); cmd->result |= (CHECK_CONDITION <<1 ); } }
drivers/scsi/megaraid/megaraid_sas.c +3 -3
@@ -45,6 +45,8 @@#include <scsi/scsi_cmnd.h> #include <scsi/scsi_device.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_eh.h> +#include "megaraid_sas.h" /*
@@ -1309,9 +1311,7 @@ megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd, cmd->scmd->result = (DID_OK <<16 ) | hdr->scsi_status;if (hdr->scsi_status ==SAM_STAT_CHECK_CONDITION ) { -memset (cmd->scmd->sense_buffer,0 , -SCSI_SENSE_BUFFERSIZE ); -memcpy (cmd->scmd->sense_buffer, cmd->sense, +scsi_eh_cpy_sense (cmd->scmd, cmd->sense, hdr->sense_len); cmd->scmd->result |=DRIVER_SENSE <<24 ;
drivers/scsi/ncr53c8xx.c +4 -5
@@ -123,6 +123,7 @@#include <scsi/scsi_tcq.h> #include <scsi/scsi_transport.h> #include <scsi/scsi_transport_spi.h> +#include <scsi/scsi_eh.h> #include "ncr53c8xx.h"
@@ -4962,12 +4963,10 @@ void ncr_complete (struct ncb *np, struct ccb *cp)/* ** Copy back sense data to caller's buffer. */ -memcpy (cmd->sense_buffer, cp->sense_buf, -min_t (size_t,SCSI_SENSE_BUFFERSIZE , -sizeof (cp->sense_buf))); +scsi_eh_cpy_sense (cmd, cp->sense_buf,sizeof (cp->sense_buf));if (DEBUG_FLAGS & (DEBUG_RESULT |DEBUG_TINY )) { -u_char *p = cmd->sense_buffer; +u_char *p = cp->sense_buf;int i;PRINT_ADDR (cmd,"sense data:" );for (i=0 ; i<14 ; i++)printk (" %x" , *p++);
@@ -5040,7 +5039,7 @@ void ncr_complete (struct ncb *np, struct ccb *cp)break ;case S_CHECK_COND :printk (" SENSE:" ); - p = (u_char *) &cmd->sense_buffer; + p = (u_char *)scsi_sense (cmd);for (i=0 ; i<14 ; i++)printk (" %x" , *p++);break ;
drivers/scsi/qlogicpti.c +2 -2
@@ -1141,8 +1141,8 @@ static struct scsi_cmnd *qlogicpti_intr_handler(struct qlogicpti *qpti) qpti->send_marker =1 ;if (sts->state_flags &SF_GOT_SENSE ) -memcpy (Cmnd->sense_buffer, sts->req_sense_data, -SCSI_SENSE_BUFFERSIZE ); +scsi_eh_cpy_sense (Cmnd, sts->req_sense_data, +sizeof (sts->req_sense_data));if (sts->hdr.entry_type ==ENTRY_STATUS ) Cmnd->result =
drivers/scsi/scsi_debug.c +2 -3
@@ -2379,9 +2379,8 @@ static int schedule_resp(struct scsi_cmnd * cmnd,if (cmnd && devip) {/* simulate autosense by this driver */ if (SAM_STAT_CHECK_CONDITION == (scsi_result &0xff )) -memcpy (cmnd->sense_buffer, devip->sense_buff, - (SCSI_SENSE_BUFFERSIZE >SDEBUG_SENSE_LEN ) ? -SDEBUG_SENSE_LEN :SCSI_SENSE_BUFFERSIZE ); +scsi_eh_cpy_sense (cmnd, devip->sense_buff, +SDEBUG_SENSE_LEN ); }if (delta_jiff <=0 ) {if (cmnd)
drivers/scsi/sym53c8xx_2/sym_glue.c +2 -3
@@ -46,6 +46,7 @@#include <scsi/scsi_tcq.h> #include <scsi/scsi_device.h> #include <scsi/scsi_transport.h> +#include <scsi/scsi_eh.h> #include "sym_glue.h" #include "sym_nvram.h"
@@ -207,9 +208,7 @@ void sym_set_cam_result_error(struct sym_hcb *np, struct sym_ccb *cp, int resid)/* * Bounce back the sense data to user. */ -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); -memcpy (cmd->sense_buffer, cp->sns_bbuf, -min (SCSI_SENSE_BUFFERSIZE ,SYM_SNS_BBUF_LEN )); +scsi_eh_cpy_sense (cmd, cp->sns_bbuf,SYM_SNS_BBUF_LEN );#if 0 /* * If the device reports a UNIT ATTENTION condition
Signature · 1 line
1.5.3.3
RFC PATCH 3/24 scsi-drivers: more drivers use new scsi_eh_cpy_sense()
In reply to Boaz Harrosh All below drivers had the sense information stored in some driver internal structure, which was copied or manipulated and set into sense_buffer. So use scsi_eh_cpy_sense() in it's place. In case of manipulation of sense data. a temporary buffer is used, then copied.
Some places that inspect the sense buffer are converted to use scsi_sense() accessor.
driver files changed:
drivers/s390/scsi/zfcp_fsf.c
drivers/scsi/3w-xxxx.c
drivers/scsi/aacraid/aachba.c
drivers/scsi/aic7xxx/aic79xx_osm.c
drivers/scsi/aic7xxx/aic7xxx_osm.c
drivers/scsi/arcmsr/arcmsr_hba.c
drivers/scsi/ipr.c
drivers/scsi/ips.c
drivers/scsi/megaraid/megaraid_mbox.c
drivers/scsi/ps3rom.c
drivers/scsi/qla4xxx/ql4_isr.c
drivers/scsi/stex.c--- drivers/s390/scsi/zfcp_fsf.c | 11 ++---- drivers/scsi/3w-xxxx.c | 20 +++++++++---- drivers/scsi/aacraid/aachba.c | 52 ++++++++++---------------------- drivers/scsi/aic7xxx/aic79xx_osm.c | 34 +++++++-------------- drivers/scsi/aic7xxx/aic7xxx_osm.c | 22 ++++---------- drivers/scsi/arcmsr/arcmsr_hba.c | 32 ++++++++++++-------- drivers/scsi/arm/fas216.c | 2 +- drivers/scsi/ipr.c | 17 ++++++----- drivers/scsi/ips.c | 13 ++++---- drivers/scsi/megaraid/megaraid_mbox.c | 24 ++++++++++----- drivers/scsi/ps3rom.c | 31 +++++++++++-------- drivers/scsi/qla1280.c | 26 ++++------------ drivers/scsi/qla4xxx/ql4_isr.c | 12 ++----- drivers/scsi/stex.c | 31 +++++++++---------- 14 files changed, 147 insertions(+), 180 deletions(-)
drivers/s390/scsi/zfcp_fsf.c +4 -7
@@ -4209,13 +4209,11 @@ zfcp_fsf_send_fcp_command_task_handler(struct zfcp_fsf_req *fsf_req)/* check for sense data */ if (unlikely (fcp_rsp_iu->validity.bits.fcp_sns_len_valid)) { +u8 *sense; sns_len =FSF_FCP_RSP_SIZE -sizeof (struct fcp_rsp_iu ) + fcp_rsp_iu->fcp_rsp_len;ZFCP_LOG_TRACE ("room for %i bytes sense data in QTCB\n" , sns_len); - sns_len =min (sns_len, (u32 )SCSI_SENSE_BUFFERSIZE ); -ZFCP_LOG_TRACE ("room for %i bytes sense data in SCSI command\n" , -SCSI_SENSE_BUFFERSIZE ); sns_len =min (sns_len, fcp_rsp_iu->fcp_sns_len);ZFCP_LOG_TRACE ("scpnt->result =0x%x, command was:\n" , scpnt->result);
@@ -4224,10 +4222,9 @@ zfcp_fsf_send_fcp_command_task_handler(struct zfcp_fsf_req *fsf_req)ZFCP_LOG_TRACE ("%i bytes sense data provided by FCP\n" , fcp_rsp_iu->fcp_sns_len); -memcpy (scpnt->sense_buffer, -zfcp_get_fcp_sns_info_ptr (fcp_rsp_iu), sns_len); -ZFCP_HEX_DUMP (ZFCP_LOG_LEVEL_TRACE , - (void *)scpnt->sense_buffer, sns_len); + sense =zfcp_get_fcp_sns_info_ptr (fcp_rsp_iu); +scsi_eh_cpy_sense (scpnt, sense, sns_len); +ZFCP_HEX_DUMP (ZFCP_LOG_LEVEL_TRACE , sense, sns_len); }/* check for overrun */
drivers/scsi/3w-xxxx.c +14 -6
@@ -214,6 +214,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/scsi_cmnd.h> +#include <scsi/scsi_eh.h> #include "3w-xxxx.h" /* Globals */
@@ -410,23 +411,30 @@ static int tw_decode_sense(TW_Device_Extension *tw_dev, int request_id, int fillif ((command->status ==0xc7 ) || (command->status ==0xcb )) {for (i =0 ; i <ARRAY_SIZE (tw_sense_table); i++) {if (command->flags == tw_sense_table[i][0 ]) { +u8 sense[14 ]; +struct scsi_cmnd *srb = + tw_dev->srb[request_id]; +memset (sense,0 ,sizeof (sense));/* Valid bit and 'current errors' */ - tw_dev->srb[request_id]->sense_buffer[0 ] = (0x1 <<7 |0x70 ); + sense[0 ] = (0x1 <<7 |0x70 );/* Sense key */ - tw_dev->srb[request_id]->sense_buffer[2 ] = tw_sense_table[i][1 ]; + sense[2 ] = tw_sense_table[i][1 ];/* Additional sense length */ - tw_dev->srb[request_id]->sense_buffer[7 ] =0xa ;/* 10 bytes */ + sense[7 ] =0xa ;/* 10 bytes */ /* Additional sense code */ - tw_dev->srb[request_id]->sense_buffer[12 ] = tw_sense_table[i][2 ]; + sense[12 ] = tw_sense_table[i][2 ];/* Additional sense code qualifier */ - tw_dev->srb[request_id]->sense_buffer[13 ] = tw_sense_table[i][3 ]; + sense[13 ] = tw_sense_table[i][3 ]; - tw_dev->srb[request_id]->result = (DID_OK <<16 ) | (CHECK_CONDITION <<1 ); +scsi_eh_cpy_sense (srb, sense, +sizeof (sense)); + srb->result = (DID_OK <<16 ) | + (CHECK_CONDITION <<1 );return TW_ISR_DONT_RESULT ;/* Special case for isr to not over-write result */ } }
drivers/scsi/aacraid/aachba.c +16 -36
135 lines not on this page · raw message
drivers/scsi/aic7xxx/aic79xx_osm.c +11 -23
@@ -1654,13 +1654,6 @@ ahd_done(struct ahd_softc *ahd, struct scb *scb) }ahd_linux_unmap_scb (ahd, scb); -/* -* Guard against stale sense data. -* The Linux mid-layer assumes that sense -* was retrieved anytime the first byte of -* the sense buffer looks "sane". -*/ - cmd->sense_buffer[0 ] =0 ;if (ahd_get_transaction_status (scb) ==CAM_REQ_INPROG ) {uint32_t amount_xferred;
@@ -1779,11 +1772,11 @@ ahd_linux_handle_scsi_status(struct ahd_softc *ahd,struct scsi_status_iu_header *siu;u_int sense_size;u_int sense_offset; +u8 *sense_buffer;if (scb->flags &SCB_SENSE ) { - sense_size =min (sizeof (struct scsi_sense_data ) - -ahd_get_sense_residual (scb), - (u_long )SCSI_SENSE_BUFFERSIZE ); + sense_size =sizeof (struct scsi_sense_data ) - +ahd_get_sense_residual (scb); sense_offset =0 ; }else {/*
@@ -1792,16 +1785,12 @@ ahd_linux_handle_scsi_status(struct ahd_softc *ahd,*/ siu = (struct scsi_status_iu_header *) scb->sense_data; - sense_size =min_t (size_t, -scsi_4btoul (siu->sense_length), -SCSI_SENSE_BUFFERSIZE ); + sense_size =scsi_4btoul (siu->sense_length); sense_offset =SIU_SENSE_OFFSET (siu); } - -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); -memcpy (cmd->sense_buffer, -ahd_get_sense_buf (ahd, scb) - + sense_offset, sense_size); + sense_buffer =ahd_get_sense_buf (ahd, scb) + + sense_offset; +scsi_eh_cpy_sense (cmd, sense_buffer, sense_size); cmd->result |= (DRIVER_SENSE <<24 );#ifdef AHD_DEBUG
@@ -1813,7 +1802,7 @@ ahd_linux_handle_scsi_status(struct ahd_softc *ahd,for (i =0 ; i < sense_size; i++) {if ((i &0xF ) ==0 )printf ("\n" ); -printf ("0x%x " , cmd->sense_buffer[i]); +printf ("0x%x " , sense_buffer[i]); }printf ("\n" ); }
@@ -1918,10 +1907,9 @@ ahd_linux_queue_cmd_complete(struct ahd_softc *ahd, struct scsi_cmnd *cmd)if ((cmd->result >>24 ) !=DRIVER_SENSE ) { do_fallback =1 ; }else { -struct scsi_sense_data *sense; - - sense = (struct scsi_sense_data *) - cmd->sense_buffer; +struct scsi_sense_data *sense = + (struct scsi_sense_data *)scsi_sense (cmd); +if (sense->extra_len >=5 && (sense->add_sense_code ==0x47 || sense->add_sense_code ==0x48 ))
drivers/scsi/aic7xxx/aic7xxx_osm.c +6 -16
@@ -1677,13 +1677,6 @@ ahc_done(struct ahc_softc *ahc, struct scb *scb) }ahc_linux_unmap_scb (ahc, scb); -/* -* Guard against stale sense data. -* The Linux mid-layer assumes that sense -* was retrieved anytime the first byte of -* the sense buffer looks "sane". -*/ - cmd->sense_buffer[0 ] =0 ;if (ahc_get_transaction_status (scb) ==CAM_REQ_INPROG ) {uint32_t amount_xferred;
@@ -1800,15 +1793,12 @@ ahc_linux_handle_scsi_status(struct ahc_softc *ahc, cmd = scb->io_ctx;if (scb->flags &SCB_SENSE ) {u_int sense_size; +u8 *sense_buffer; - sense_size =min (sizeof (struct scsi_sense_data ) - -ahc_get_sense_residual (scb), - (u_long )SCSI_SENSE_BUFFERSIZE ); -memcpy (cmd->sense_buffer, -ahc_get_sense_buf (ahc, scb), sense_size); -if (sense_size <SCSI_SENSE_BUFFERSIZE ) -memset (&cmd->sense_buffer[sense_size],0 , -SCSI_SENSE_BUFFERSIZE - sense_size); + sense_size =sizeof (struct scsi_sense_data ) + -ahc_get_sense_residual (scb); + sense_buffer = (u8 *)ahc_get_sense_buf (ahc, scb); +scsi_eh_cpy_sense (cmd, sense_buffer, sense_size); cmd->result |= (DRIVER_SENSE <<24 );#ifdef AHC_DEBUG if (ahc_debug &AHC_SHOW_SENSE ) {
@@ -1819,7 +1809,7 @@ ahc_linux_handle_scsi_status(struct ahc_softc *ahc,for (i =0 ; i < sense_size; i++) {if ((i &0xF ) ==0 )printf ("\n" ); -printf ("0x%x " , cmd->sense_buffer[i]); +printf ("0x%x " , sense_buffer[i]); }printf ("\n" ); }
drivers/scsi/arcmsr/arcmsr_hba.c +19 -13
@@ -69,6 +69,7 @@#include <scsi/scsi_device.h> #include <scsi/scsi_transport.h> #include <scsi/scsicam.h> +#include <scsi/scsi_eh.h> #include "arcmsr.h" MODULE_AUTHOR ("Erich Chen <[email protected]>" );
@@ -628,18 +629,15 @@ static void arcmsr_report_sense_info(struct CommandControlBlock *ccb) {struct scsi_cmnd *pcmd = ccb->pcmd; -struct SENSE_DATA *sensebuffer = (struct SENSE_DATA *)pcmd->sense_buffer; +struct SENSE_DATA *sensebuffer = + (struct SENSE_DATA *)ccb->arcmsr_cdb.SenseData; pcmd->result =DID_OK <<16 ; -if (sensebuffer) { -int sense_data_length = -sizeof (struct SENSE_DATA ) <SCSI_SENSE_BUFFERSIZE - ?sizeof (struct SENSE_DATA ) :SCSI_SENSE_BUFFERSIZE ; -memset (sensebuffer,0 ,SCSI_SENSE_BUFFERSIZE ); -memcpy (sensebuffer, ccb->arcmsr_cdb.SenseData, sense_data_length); - sensebuffer->ErrorCode =SCSI_SENSE_CURRENT_ERRORS ; - sensebuffer->Valid =1 ; - } + + sensebuffer->ErrorCode =SCSI_SENSE_CURRENT_ERRORS ; + sensebuffer->Valid =1 ; + +scsi_eh_cpy_sense (pcmd, sensebuffer,sizeof (sensebuffer)); }static u32 arcmsr_disable_outbound_ints (struct AdapterControlBlock *acb)
@@ -1442,8 +1440,10 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb, \ wqbuf_lastindex = acb->wqbuf_lastindex; wqbuf_firstindex = acb->wqbuf_firstindex;if (wqbuf_lastindex != wqbuf_firstindex) { +u8 sb[16 ];struct SENSE_DATA *sensebuffer = - (struct SENSE_DATA *)cmd->sense_buffer; + (struct SENSE_DATA *)sb; +memset (sensebuffer,0 ,sizeof (sensebuffer));arcmsr_post_ioctldata2iop (acb);/* has error report sensedata */ sensebuffer->ErrorCode =0x70 ;
@@ -1451,6 +1451,8 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb, \ sensebuffer->AdditionalSenseLength =0x0A ; sensebuffer->AdditionalSenseCode =0x20 ; sensebuffer->Valid =1 ; +scsi_eh_cpy_sense (cmd, sensebuffer, +sizeof (*sensebuffer)); retvalue =ARCMSR_MESSAGE_FAIL ; }else { my_empty_len = (wqbuf_firstindex-wqbuf_lastindex -1 )
@@ -1471,14 +1473,18 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb, \arcmsr_post_ioctldata2iop (acb); } }else { -/* has error report sensedata */ +u8 sb[16 ];struct SENSE_DATA *sensebuffer = - (struct SENSE_DATA *)cmd->sense_buffer; + (struct SENSE_DATA *)sb; +memset (sensebuffer,0 ,sizeof (sensebuffer)); +/* has error report sensedata */ sensebuffer->ErrorCode =0x70 ; sensebuffer->SenseKey =ILLEGAL_REQUEST ; sensebuffer->AdditionalSenseLength =0x0A ; sensebuffer->AdditionalSenseCode =0x20 ; sensebuffer->Valid =1 ; +scsi_eh_cpy_sense (cmd, sensebuffer, +sizeof (sensebuffer)); retvalue =ARCMSR_MESSAGE_FAIL ; } }
drivers/scsi/arm/fas216.c +1 -1
@@ -2009,7 +2009,7 @@ static void fas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt,* have valid data in the sense buffer that could * confuse the higher levels. */ -memset (SCpnt->sense_buffer,0 ,sizeof (SCpnt->sense_buffer)); +scsi_eh_reset_sense (SCpnt);//printk("scsi%d.%c: sense buffer: ", info->host->host_no, '0' + SCpnt->device->id); //{ int i; for (i = 0; i < 32; i++) printk("%02x ", SCpnt->sense_buffer[i]); printk("\n"); } /*
drivers/scsi/ipr.c +9 -8
@@ -4379,8 +4379,8 @@ static void ipr_erp_done(struct ipr_cmnd *ipr_cmd)scmd_printk (KERN_ERR , scsi_cmd,"Request Sense failed with IOASC: 0x%08X\n" , ioasc); }else { -memcpy (scsi_cmd->sense_buffer, ipr_cmd->sense_buffer, -SCSI_SENSE_BUFFERSIZE ); +scsi_eh_cpy_sense (scsi_cmd, ipr_cmd->sense_buffer, +sizeof (ipr_cmd->sense_buffer)); }if (res) {
@@ -4574,12 +4574,12 @@ static void ipr_dump_ioasa(struct ipr_ioa_cfg *ioa_cfg,static void ipr_gen_sense (struct ipr_cmnd *ipr_cmd) {u32 failing_lba; -u8 *sense_buf = ipr_cmd->scsi_cmd->sense_buffer; +u8 sense_buf[24 ];struct ipr_resource_entry *res = ipr_cmd->scsi_cmd->device->hostdata;struct ipr_ioasa *ioasa = &ipr_cmd->ioasa;u32 ioasc =be32_to_cpu (ioasa->ioasc); -memset (sense_buf,0 ,SCSI_SENSE_BUFFERSIZE ); +memset (sense_buf,0 ,sizeof (sense_buf));if (ioasc >=IPR_FIRST_DRIVER_IOASC )return ;
@@ -4652,6 +4652,8 @@ static void ipr_gen_sense(struct ipr_cmnd *ipr_cmd) sense_buf[7 ] =6 ;/* additional length */ } } +scsi_eh_cpy_sense (ipr_cmd->scsi_cmd, sense_buf,sizeof (sense_buf)); + }/**
@@ -4671,9 +4673,8 @@ static int ipr_get_autosense(struct ipr_cmnd *ipr_cmd)if ((be32_to_cpu (ioasa->ioasc_specific) &IPR_AUTOSENSE_VALID ) ==0 )return 0 ; -memcpy (ipr_cmd->scsi_cmd->sense_buffer, ioasa->auto_sense.data, -min_t (u16,be16_to_cpu (ioasa->auto_sense.auto_sense_len), -SCSI_SENSE_BUFFERSIZE )); +scsi_eh_cpy_sense (ipr_cmd->scsi_cmd, ioasa->auto_sense.data, +be16_to_cpu (ioasa->auto_sense.auto_sense_len));return 1 ; }
@@ -4836,7 +4837,7 @@ static int ipr_queuecommand(struct scsi_cmnd *scsi_cmd,* and the ioa_is_dead check can be removed */ if (unlikely (ioa_cfg->ioa_is_dead || !res)) { -memset (scsi_cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); +scsi_eh_reset_sense (scsi_cmd); scsi_cmd->result = (DID_NO_CONNECT <<16 ); scsi_cmd->scsi_done (scsi_cmd);return 0 ;
drivers/scsi/ips.c +7 -6
@@ -3431,13 +3431,13 @@ ips_map_status(ips_ha_t * ha, ips_scb_t * scb, ips_stat_t * sp) IPS_CMD_EXTENDED_DCDB_SG)) { tapeDCDB = (IPS_DCDB_TABLE_TAPE *) & scb->dcdb; -memcpy (scb->scsi_cmd->sense_buffer, +scsi_eh_cpy_sense (scb->scsi_cmd, tapeDCDB->sense_info, -SCSI_SENSE_BUFFERSIZE ); +sizeof (tapeDCDB->sense_info)); }else { -memcpy (scb->scsi_cmd->sense_buffer, +scsi_eh_cpy_sense (scb->scsi_cmd, scb->dcdb.sense_info, -SCSI_SENSE_BUFFERSIZE ); +sizeof (scb->dcdb.sense_info)); } device_error =2 ;/* check condition */ }
@@ -3571,7 +3571,7 @@ static int ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb) {int ret; -char *sp; +u8 sp[16 ];int device_error;IPS_DCDB_TABLE_TAPE *tapeDCDB;int TimeOut;
@@ -3816,13 +3816,14 @@ ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb)/* Set the Return Info to appear like the Command was */ /* attempted, a Check Condition occurred, and Sense */ /* Data indicating an Invalid CDB OpCode is returned. */ - sp = (char *) scb->scsi_cmd->sense_buffer; +memset (sp,0 ,sizeof (sp)); sp[0 ] =0x70 ;/* Error Code */ sp[2 ] =ILLEGAL_REQUEST ;/* Sense Key 5 Illegal Req. */ sp[7 ] =0x0A ;/* Additional Sense Length */ sp[12 ] =0x20 ;/* ASC = Invalid OpCode */ sp[13 ] =0x00 ;/* ASCQ */ +scsi_eh_cpy_sense (scb->scsi_cmd, sp,sizeof (sp)); device_error =2 ;/* Indicate Check Condition */ scb->scsi_cmd->result = device_error | (DID_OK <<16 );
drivers/scsi/megaraid/megaraid_mbox.c +16 -8
@@ -70,6 +70,7 @@* For history of changes, see Documentation/ChangeLog.megaraid */ +#include <scsi/scsi_eh.h> #include "megaraid_mbox.h" static int megaraid_init (void );
@@ -1616,9 +1617,13 @@ megaraid_mbox_build_cmd(adapter_t *adapter, struct scsi_cmnd *scp, int *busy) }if (scp->cmnd[1 ] &MEGA_SCSI_INQ_EVPD ) { - scp->sense_buffer[0 ] =0x70 ; - scp->sense_buffer[2 ] =ILLEGAL_REQUEST ; - scp->sense_buffer[12 ] =MEGA_INVALID_FIELD_IN_CDB ; +u8 sense[14 ]; + +memset (sense,0 ,sizeof (sense)); + sense[0 ] =0x70 ; + sense[2 ] =ILLEGAL_REQUEST ; + sense[12 ] =MEGA_INVALID_FIELD_IN_CDB ; +scsi_eh_cpy_sense (scp, sense,sizeof (sense)); scp->result =CHECK_CONDITION <<1 ;return NULL ; }
@@ -2365,8 +2370,7 @@ megaraid_mbox_dpc(unsigned long devp)if (mbox->cmd ==MBOXCMD_PASSTHRU || mbox->cmd ==MBOXCMD_PASSTHRU64 ) { -memcpy (scp->sense_buffer, pthru->reqsensearea, -14 ); +scsi_eh_cpy_sense (scp, pthru->reqsensearea,14 ); scp->result =DRIVER_SENSE <<24 |DID_OK <<16 |CHECK_CONDITION <<1 ;
@@ -2374,15 +2378,19 @@ megaraid_mbox_dpc(unsigned long devp)else {if (mbox->cmd ==MBOXCMD_EXTPTHRU ) { -memcpy (scp->sense_buffer, +scsi_eh_cpy_sense (scp, epthru->reqsensearea,14 ); scp->result =DRIVER_SENSE <<24 |DID_OK <<16 |CHECK_CONDITION <<1 ; }else { - scp->sense_buffer[0 ] =0x70 ; - scp->sense_buffer[2 ] =ABORTED_COMMAND ; +u8 sn[4 ]; + +memset (sn,0 ,sizeof (sn)); + sn[0 ] =0x70 ; + sn[2 ] =ABORTED_COMMAND ; +scsi_eh_cpy_sense (scp, sn,sizeof (sn)); scp->result =CHECK_CONDITION <<1 ; } }
drivers/scsi/ps3rom.c +18 -13
@@ -323,10 +323,13 @@ static int ps3rom_queuecommand(struct scsi_cmnd *cmd, }if (res) { -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); +u8 sense[4 ]; + +memset (sense,0 ,sizeof (sense)); cmd->result = res; - cmd->sense_buffer[0 ] =0x70 ; - cmd->sense_buffer[2 ] =ILLEGAL_REQUEST ; + sense[0 ] =0x70 ; + sense[2 ] =ILLEGAL_REQUEST ; + scsi_eh_cpy_sense(cmd, sense, sizeof(sense); priv->curr_cmd =NULL ; cmd->scsi_done (cmd); }
@@ -399,17 +402,19 @@ static irqreturn_t ps3rom_interrupt(int irq, void *data) goto done; } -if (decode_lv1_status (status, &sense_key, &asc, &ascq)) { +if (!decode_lv1_status (status, &sense_key, &asc, &ascq)) { +u8 sense[14 ]; + +memset (sense,0 ,sizeof (sense)); + sense[0 ] =0x70 ; + sense[2 ] = sense_key; + sense[7 ] =16 -6 ; + sense[12 ] = asc; + sense[13 ] = ascq; +scsi_eh_cpy_sense (cmd, sense,sizeof (sense)); + cmd->result =SAM_STAT_CHECK_CONDITION ; + }else cmd->result =DID_ERROR <<16 ; - goto done; - } - - cmd->sense_buffer[0 ] =0x70 ; - cmd->sense_buffer[2 ] = sense_key; - cmd->sense_buffer[7 ] =16 -6 ; - cmd->sense_buffer[12 ] = asc; - cmd->sense_buffer[13 ] = ascq; - cmd->result =SAM_STAT_CHECK_CONDITION ; done: priv->curr_cmd =NULL ;
drivers/scsi/qla1280.c +7 -19
@@ -362,6 +362,7 @@#include <scsi/scsi_device.h> #include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> +#include <scsi/scsi_eh.h> #if defined(CONFIG_IA64_GENERIC ) || defined(CONFIG_IA64_SGI_SN2 )#include <asm/sn/io.h>
@@ -527,8 +528,6 @@ __setup("qla1280=", qla1280_setup);#define CMD_SP (Cmnd) &Cmnd->SCp#define CMD_CDBLEN (Cmnd) Cmnd->cmd_len#define CMD_CDBP (Cmnd) Cmnd->cmnd -#define CMD_SNSP (Cmnd) Cmnd->sense_buffer -#define CMD_SNSLEN (Cmnd) SCSI_SENSE_BUFFERSIZE#define CMD_RESULT (Cmnd) Cmnd->result#define CMD_HANDLE (Cmnd) Cmnd->host_scribble#define CMD_REQUEST (Cmnd) Cmnd->request->cmd
@@ -3698,32 +3697,21 @@ qla1280_status_entry(struct scsi_qla_host *ha, struct response *pkt,if (scsi_status &SAM_STAT_CHECK_CONDITION ) {if (comp_status !=CS_ARS_FAILED ) { -uint16_t req_sense_length = -le16_to_cpu (pkt->req_sense_length); -if (req_sense_length <CMD_SNSLEN (cmd)) - sense_sz = req_sense_length; -else -/* -* scsi_cmnd->sense_buffer is -* 64 bytes, why only copy 63? -* This looks wrong! /Jes -*/ - sense_sz =CMD_SNSLEN (cmd) -1 ; + sense_sz =min_t (size_t, +sizeof (pkt->req_sense_data), +le16_to_cpu (pkt->req_sense_length)); -memcpy (cmd->sense_buffer, - &pkt->req_sense_data, sense_sz); +scsi_eh_cpy_sense (cmd, pkt->req_sense_data, + sense_sz); }else sense_sz =0 ; -memset (cmd->sense_buffer + sense_sz,0 , -SCSI_SENSE_BUFFERSIZE - sense_sz);dprintk (2 ,"qla1280_status_entry: Check " "condition Sense data, b %i, t %i, " "l %i\n" , bus, target, lun);if (sense_sz)qla1280_dump_buffer (2 , - (char *)cmd->sense_buffer, - sense_sz); + (char *)pkt->req_sense_data, sense_sz); } }
drivers/scsi/qla4xxx/ql4_isr.c +4 -8
@@ -10,6 +10,7 @@#include "ql4_dbg.h" #include "ql4_inline.h" +#include <scsi/scsi_eh.h> /** * qla2x00_process_completed_request() - Process a Fast Post response. * @ha: SCSI driver HA context
@@ -123,14 +124,11 @@ static void qla4xxx_status_entry(struct scsi_qla_host *ha,break ;/* Copy Sense Data into sense buffer. */ -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); - sensebytecnt =le16_to_cpu (sts_entry->senseDataByteCnt);if (sensebytecnt ==0 )break ; -memcpy (cmd->sense_buffer, sts_entry->senseData, -min_t (uint16_t, sensebytecnt,SCSI_SENSE_BUFFERSIZE )); +scsi_eh_cpy_sense (cmd, sts_entry->senseData, sensebytecnt); DEBUG2(printk("scsi%ld:%d:%d:%d: %s: sense key = %x, " "ASC/ASCQ = %02x/%02x\n", ha->host_no,
@@ -207,15 +205,13 @@ static void qla4xxx_status_entry(struct scsi_qla_host *ha,break ;/* Copy Sense Data into sense buffer. */ -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); - sensebytecnt =le16_to_cpu (sts_entry->senseDataByteCnt);if (sensebytecnt ==0 )break ; -memcpy (cmd->sense_buffer, sts_entry->senseData, -min_t (uint16_t, sensebytecnt,SCSI_SENSE_BUFFERSIZE )); +scsi_eh_cpy_sense (cmd, sts_entry->senseData, + sensebytecnt); DEBUG2(printk("scsi%ld:%d:%d:%d: %s: sense key = %x, " "ASC/ASCQ = %02x/%02x\n", ha->host_no,
drivers/scsi/stex.c +15 -16
@@ -33,6 +33,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/scsi_dbg.h> +#include <scsi/scsi_eh.h> #define DRV_NAME "stex"#define ST_DRIVER_VERSION "3.6.0000.1"
@@ -290,8 +291,6 @@ struct st_ccb {struct req_msg *req;struct scsi_cmnd *cmd; -void *sense_buffer; -unsigned int sense_bufflen;int sg_count;u32 req_type;
@@ -364,13 +363,18 @@ static struct status_msg *stex_get_status(struct st_hba *hba)static void stex_set_sense (struct scsi_cmnd *cmd,u8 sk,u8 asc,u8 ascq) { +u8 sense_buffer[14 ]; + +memset (sense_buffer,0 ,sizeof (sense_buffer)); cmd->result = (DRIVER_SENSE <<24 ) |SAM_STAT_CHECK_CONDITION ; - cmd->sense_buffer[0 ] =0x70 ;/* fixed format, current */ - cmd->sense_buffer[2 ] = sk; - cmd->sense_buffer[7 ] =18 -8 ;/* additional sense length */ - cmd->sense_buffer[12 ] = asc; - cmd->sense_buffer[13 ] = ascq; + sense_buffer[0 ] =0x70 ;/* fixed format, current */ + sense_buffer[2 ] = sk; + sense_buffer[7 ] =18 -8 ;/* additional sense length */ + sense_buffer[12 ] = asc; + sense_buffer[13 ] = ascq; + +scsi_eh_cpy_sense (cmd, sense_buffer,sizeof (sense_buffer)); } static void stex_invalid_field(struct scsi_cmnd *cmd,
@@ -642,8 +646,6 @@ stex_queuecommand(struct scsi_cmnd *cmd, void (* done)(struct scsi_cmnd *))memcpy (req->cdb, cmd->cmnd,STEX_CDB_LENGTH ); hba->ccb[tag].cmd = cmd; - hba->ccb[tag].sense_bufflen =SCSI_SENSE_BUFFERSIZE ; - hba->ccb[tag].sense_buffer = cmd->sense_buffer; hba->ccb[tag].req_type =0 ;if (cmd->sc_data_direction !=DMA_NONE )
@@ -699,15 +701,14 @@ static void stex_copy_data(struct st_ccb *ccb, struct status_msg *resp, unsigned int variable) { size_t count = variable; + +if (ccb->cmd ==NULL ) +return ;if (resp->scsi_status !=SAM_STAT_GOOD ) { -if (ccb->sense_buffer !=NULL ) -memcpy (ccb->sense_buffer, resp->variable, -min (variable, ccb->sense_bufflen)); +scsi_eh_cpy_sense (ccb->cmd, resp->variable, variable);return ; } -if (ccb->cmd ==NULL ) -return ;stex_internal_copy (ccb->cmd, resp->variable, &count, ccb->sg_count,ST_TO_CMD ); }
@@ -1295,8 +1296,6 @@ static void stex_hba_stop(struct st_hba *hba) hba->ccb[tag].cmd =NULL ; hba->ccb[tag].sg_count =0 ; - hba->ccb[tag].sense_bufflen =0 ; - hba->ccb[tag].sense_buffer =NULL ; hba->ccb[tag].req_type |=PASSTHRU_REQ_TYPE ;stex_send_cmd (hba, req, tag);
Signature · 1 line
1.5.3.3
RE: [PATCH 3/24][RFC] scsi-drivers: more drivers use new scsi_eh_cpy_sense()
ACK for aacraid and ips with condition that community accepts the RFC's premise.
The code changes appear trivial and make sense.
I had some upstream changes in the set_sense code in aacraid related to a recent discussion regarding error propagation that is overlapped directly by the changes in this RFC. However, it looks like the changes in the SCSI layer to deal with the error propagation will mask the problem in set_sense reducing the pressure on me submitting the set_sense code changes; so I can hold on to them for a while until this RFC is settled and submit them as a janitor-style changes later.
Sincerely -- Mark Salyzyn
Quoted 1,129 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Boaz Harrosh
Sent: Monday, February 04, 2008 10:48 AM
To: James Bottomley; FUJITA Tomonori; Christoph Hellwig; Jens Axboe; Jeff Garzik; linux-scsi
Cc: Andrew Morton
Subject: [PATCH 3/24][RFC] scsi-drivers: more drivers use newscsi_eh_cpy_sense()All below drivers had the sense information stored in some driver internal structure, which was copied or manipulated and set into
sense_buffer. So usescsi_eh_cpy_sense()in it's place. In case of manipulation of sense data. a temporary buffer is used, then copied.Some places that inspect the sense buffer are converted to use
scsi_sense()accessor.driver files changed:
drivers/s390/scsi/zfcp_fsf.c drivers/scsi/3w-xxxx.c drivers/scsi/aacraid/aachba.c drivers/scsi/aic7xxx/aic79xx_osm.c drivers/scsi/aic7xxx/aic7xxx_osm.c drivers/scsi/arcmsr/arcmsr_hba.c drivers/scsi/ipr.c drivers/scsi/ips.c drivers/scsi/megaraid/megaraid_mbox.c drivers/scsi/ps3rom.c drivers/scsi/qla4xxx/ql4_isr.c drivers/scsi/stex.cSigned-off-by Boaz Harrosh <[email protected]>--- drivers/s390/scsi/zfcp_fsf.c | 11 ++---- drivers/scsi/3w-xxxx.c | 20 +++++++++---- drivers/scsi/aacraid/aachba.c | 52 ++++++++++---------------------- drivers/scsi/aic7xxx/aic79xx_osm.c | 34 +++++++-------------- drivers/scsi/aic7xxx/aic7xxx_osm.c | 22 ++++---------- drivers/scsi/arcmsr/arcmsr_hba.c | 32 ++++++++++++-------- drivers/scsi/arm/fas216.c | 2 +- drivers/scsi/ipr.c | 17 ++++++----- drivers/scsi/ips.c | 13 ++++---- drivers/scsi/megaraid/megaraid_mbox.c | 24 ++++++++++----- drivers/scsi/ps3rom.c | 31 +++++++++++-------- drivers/scsi/qla1280.c | 26 ++++------------ drivers/scsi/qla4xxx/ql4_isr.c | 12 ++----- drivers/scsi/stex.c | 31 +++++++++---------- 14 files changed, 147 insertions(+), 180 deletions(-)drivers/s390/scsi/zfcp_fsf.c +1
-2@@ -4209,13 +4209,11 @@ zfcp_fsf_send_fcp_command_task_handler(struct zfcp_fsf_req *fsf_req)/* check for sense data */ if (unlikely(fcp_rsp_iu->validity.bits.fcp_sns_len_valid)) { +u8 *sense; sns_len =FSF_FCP_RSP_SIZE -sizeof (struct fcp_rsp_iu ) + fcp_rsp_iu->fcp_rsp_len; ZFCP_LOG_TRACE("room for %i bytes sense data in QTCB\n", sns_len); - sns_len =min (sns_len, (u32 )SCSI_SENSE_BUFFERSIZE ); - ZFCP_LOG_TRACE("room for %i bytes sense datain SCSI command\n",
drivers/s390/scsi/zfcp_fsf.c +3
-5- SCSI_SENSE_BUFFERSIZE); sns_len =min (sns_len, fcp_rsp_iu->fcp_sns_len);ZFCP_LOG_TRACE ("scpnt->result =0x%x, command was:\n" , scpnt->result);@@ -4224,10 +4222,9 @@ zfcp_fsf_send_fcp_command_task_handler(struct zfcp_fsf_req *fsf_req) ZFCP_LOG_TRACE("%i bytes sense data provided by FCP\n", fcp_rsp_iu->fcp_sns_len); -memcpy (scpnt->sense_buffer, -zfcp_get_fcp_sns_info_ptr (fcp_rsp_iu), sns_len); -ZFCP_HEX_DUMP (ZFCP_LOG_LEVEL_TRACE , - (void *)scpnt->sense_buffer, sns_len); + sense =zfcp_get_fcp_sns_info_ptr (fcp_rsp_iu); +scsi_eh_cpy_sense (scpnt, sense, sns_len); +ZFCP_HEX_DUMP (ZFCP_LOG_LEVEL_TRACE , sense, sns_len);} /* check for overrun */drivers/scsi/3w-xxxx.c +1
-0@@ -214,6 +214,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/scsi_cmnd.h> +#include <scsi/scsi_eh.h> #include "3w-xxxx.h" /* Globals */ @@ -410,23 +411,30 @@ static inttw_decode_sense(TW_Device_Extension
*tw_dev, intrequest_id, int fillif ((command->status == 0xc7) || (command->status == 0xcb)) { for (i = 0; i < ARRAY_SIZE(tw_sense_table); i++) { if (command->flags ==tw_sense_table[i][0]) {
drivers/scsi/3w-xxxx.c +3
-0+u8 sense[14 ]; + struct scsi_cmnd *srb = +tw_dev->srb[request_id];
+ memset(sense, 0,
sizeof(sense)); /* Valid bit and'current errors' */
-
tw_dev->srb[request_id]->sense_buffer[0] = (0x1<< 7 |0x70);drivers/scsi/3w-xxxx.c +1
-1+ sense[0 ] = (0x1 <<7 |0x70 );/* Sense key */ -tw_dev->srb[request_id]->sense_buffer[2] = tw_sense_table[i][1];
+ sense[2] =
tw_sense_table[i][1];/* Additional sense length */-
tw_dev->srb[request_id]->sense_buffer[7] =0xa; /* 10 bytes */drivers/scsi/3w-xxxx.c +1
-1+ sense[7 ] =0xa ;/* 10 bytes */ /* Additional sense code */ -tw_dev->srb[request_id]->sense_buffer[12] = tw_sense_table[i][2];
+ sense[12] =
tw_sense_table[i][2];/* Additional sensecode qualifier */
-
tw_dev->srb[request_id]->sense_buffer[13] = tw_sense_table[i][3];
+ sense[13] =
tw_sense_table[i][3];-
tw_dev->srb[request_id]->result = (DID_OK<< 16) | (CHECK_CONDITION<< 1);drivers/scsi/3w-xxxx.c +4
-0+scsi_eh_cpy_sense (srb, sense, +sizeof (sense)); + srb->result = (DID_OK << 16) | +(
CHECK_CONDITION<< 1);
returnTW_ISR_DONT_RESULT; /* Special case for isr to not over-write result */} }drivers/scsi/aacraid/aachba.c +2
-1@@ -39,6 +39,7 @@#include <scsi/scsi_cmnd.h> #include <scsi/scsi_device.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_eh.h> #include "aacraid.h" @@ -862,7 +863,7 @@ static int setinqserial(struct aac_dev *dev, void *data, int cid)static void set_sense (u8 *sense_buf,u8 sense_key,u8 sense_code,u8 a_sense_code,u8 incorrect_length,u8 bit_pointer,u16 field_pointer, -u32 residue) +u32 residue,struct scsi_cmnd *cmd) { sense_buf[0 ] =0xF0 ;/* Sense data valid, err code 70h (current error) */
sense_buf[1] = 0; /* Segment number, always zero */drivers/scsi/aacraid/aachba.c +2
-0@@ -897,6 +898,8 @@ static void set_sense(u8 *sense_buf, u8 sense_key, u8 sense_code, sense_buf[16 ] = field_pointer >>8 ;/* MSB */ sense_buf[17 ] = field_pointer;/* LSB */ } + +scsi_eh_cpy_sense (cmd, sense_buf,sizeof (struct sense_data )); }static int aac_bounds_32(struct
aac_dev* dev, structscsi_cmnd* cmd, u64 lba)drivers/scsi/aacraid/aachba.c +1
-4@@ -910,10 +913,7 @@ static int aac_bounds_32(struct aac_dev * dev, struct scsi_cmnd * cmd, u64 lba)HARDWARE_ERROR ,SENCODE_INTERNAL_TARGET_FAILURE ,ASENCODE_INTERNAL_TARGET_FAILURE ,0 ,0 , - 0, 0); -memcpy (cmd->sense_buffer, &dev->fsa_dev[cid].sense_data, -min_t (size_t,sizeof (dev->fsa_dev[cid].sense_data), -SCSI_SENSE_BUFFERSIZE )); + 0, 0, cmd);cmd->scsi_done(cmd); return 1; }drivers/scsi/aacraid/aachba.c +3
-12@@ -1524,10 +1524,7 @@ static void io_callback(void *context, struct fib * fibptr)HARDWARE_ERROR ,SENCODE_INTERNAL_TARGET_FAILURE , ASENCODE_INTERNAL_TARGET_FAILURE, 0, 0, - 0, 0); -memcpy (scsicmd->sense_buffer, &dev->fsa_dev[cid].sense_data, -min_t (size_t, sizeof(dev->fsa_dev[cid].sense_data), -SCSI_SENSE_BUFFERSIZE )); + 0, 0, scsicmd); }aac_fib_complete (fibptr);aac_fib_free (fibptr);@@ -1737,10 +1734,7 @@ static void synchronize_callback(void *context, struct fib *fibptr)HARDWARE_ERROR ,SENCODE_INTERNAL_TARGET_FAILURE ,ASENCODE_INTERNAL_TARGET_FAILURE ,0 ,0 , - 0, 0); -memcpy (cmd->sense_buffer, &dev->fsa_dev[cid].sense_data, -min_t (size_t,sizeof (dev->fsa_dev[cid].sense_data), -SCSI_SENSE_BUFFERSIZE )); + 0, 0, cmd); }aac_fib_complete (fibptr);@@ -1948,10 +1942,7 @@ int aac_scsi_cmd(struct scsi_cmnd * scsicmd)set_sense ((u8 *) &dev->fsa_dev[cid].sense_data,ILLEGAL_REQUEST ,SENCODE_INVALID_COMMAND , -ASENCODE_INVALID_COMMAND ,0 ,0 ,0 ,0 ); -memcpy (scsicmd->sense_buffer, &dev->fsa_dev[cid].sense_data, -min_t (size_t,sizeof (dev->fsa_dev[cid].sense_data), -SCSI_SENSE_BUFFERSIZE )); +ASENCODE_INVALID_COMMAND ,0 ,0 ,0 ,0 , scsicmd);scsicmd->scsi_done(scsicmd); return 0; }drivers/scsi/aacraid/aachba.c +1
-6@@ -1998,12 +1989,7 @@ int aac_scsi_cmd(struct scsi_cmnd * scsicmd)set_sense ((u8 *) &dev->fsa_dev[cid].sense_data,ILLEGAL_REQUEST ,SENCODE_INVALID_CDB_FIELD , -ASENCODE_NO_SENSE ,0 ,7 ,2 ,0 ); -memcpy (scsicmd->sense_buffer, - &dev->fsa_dev[cid].sense_data, -min_t (size_t, -sizeof (dev->fsa_dev[cid].sense_data), -SCSI_SENSE_BUFFERSIZE )); +ASENCODE_NO_SENSE ,0 ,7 ,2 ,0 , scsicmd);} scsicmd->scsi_done(scsicmd); return 0;drivers/scsi/aacraid/aachba.c +2
-1@@ -2176,7 +2162,8 @@ int aac_scsi_cmd(struct scsi_cmnd * scsicmd) }case REQUEST_SENSE :dprintk ((KERN_DEBUG "REQUEST SENSE command.\n" )); -memcpy (scsicmd->sense_buffer, &dev->fsa_dev[cid].sense_data, sizeof (struct sense_data)); +scsi_eh_cpy_sense (scsicmd, &dev->fsa_dev[cid].sense_data, +sizeof (struct sense_data )); memset(&dev->fsa_dev[cid].sense_data, 0,sizeof (struct sense_data)); scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD; scsicmd->scsi_done(scsicmd);drivers/scsi/aacraid/aachba.c +0
-4@@ -2256,11 +2243,7 @@ int aac_scsi_cmd(struct scsi_cmnd * scsicmd) scsicmd->result =DID_OK <<16 |COMMAND_COMPLETE <<8 |SAM_STAT_CHECK_CONDITION ; set_sense((u8 *) &dev->fsa_dev[cid].sense_data, ILLEGAL_REQUEST, SENCODE_INVALID_COMMAND, -ASENCODE_INVALID_COMMAND ,0 ,0 ,0 ,0 ); - memcpy(scsicmd->sense_buffer, &dev->fsa_dev[cid].sense_data, - min_t(size_t, -
sizeof(dev->fsa_dev[cid].sense_data),drivers/scsi/aacraid/aachba.c +1
-1- SCSI_SENSE_BUFFERSIZE)); + ASENCODE_INVALID_COMMAND, 0,0, 0, 0, scsicmd); scsicmd->scsi_done(scsicmd); return 0; }drivers/scsi/aacraid/aachba.c +0
-1@@ -2403,7 +2386,6 @@ static void aac_srb_callback(void *context, struct fib * fibptr) srbreply = (struct aac_srb_reply *)fib_data (fibptr); - scsicmd->sense_buffer[0 ] ='\0' ;/* Initialize sense valid flag to false */
/* * Calculate resid for sg */drivers/scsi/aacraid/aachba.c +1
-3@@ -2420,10 +2402,9 @@ static void aac_srb_callback(void *context, struct fib * fibptr)if (le32_to_cpu (srbreply->status) !=ST_OK ){int len; printk(KERN_WARNING "aac_srb_callback: srb failed, status = %d\n", le32_to_cpu(srbreply->status)); - len = min_t(u32, le32_to_cpu(srbreply->sense_data_size), - SCSI_SENSE_BUFFERSIZE); + len = le32_to_cpu(srbreply->sense_data_size); scsicmd->result =DID_ERROR <<16 |COMMAND_COMPLETE <<8 |SAM_STAT_CHECK_CONDITION ; - memcpy(scsicmd->sense_buffer,
srbreply->sense_data, len);drivers/scsi/aacraid/aachba.c +2
-2+scsi_eh_cpy_sense (scsicmd, srbreply->sense_data, len); }/* @@ -2524,13 +2505,12 @@ static void aac_srb_callback(void *context, struct fib * fibptr)if (le32_to_cpu (srbreply->scsi_status) ==SAM_STAT_CHECK_CONDITION ) {int len; scsicmd->result |=SAM_STAT_CHECK_CONDITION ; - len =min_t (u32,le32_to_cpu (srbreply->sense_data_size), -SCSI_SENSE_BUFFERSIZE ); + len =le32_to_cpu (srbreply->sense_data_size);#ifdef AAC_DETAILED_STATUS_INFO printk(KERN_WARNING "aac_srb_callback: check condition, status = %d len=%d\n", le32_to_cpu(srbreply->status), len);#endif
- memcpy(scsicmd->sense_buffer,srbreply->sense_data, len);drivers/scsi/aacraid/aachba.c +1
-0+scsi_eh_cpy_sense (scsicmd, srbreply->sense_data, len); }/* * OR in the scsi status (already shifted up a bit) drivers/scsi/aic7xxx/aic79xx_osm.c +1
-10@@ -1654,13 +1654,6 @@ ahd_done(struct ahd_softc *ahd, struct scb *scb) }ahd_linux_unmap_scb (ahd, scb); -/* -* Guard against stale sense data. -* The Linux mid-layer assumes that sense -* was retrieved anytime the first byte of -* the sense buffer looks "sane". -*/ - cmd->sense_buffer[0 ] =0 ;if (ahd_get_transaction_status (scb) ==CAM_REQ_INPROG ) {uint32_t amount_xferred;@@ -1779,11 +1772,11 @@ ahd_linux_handle_scsi_status(structahd_softc *ahd, struct scsi_status_iu_header *siu;u_int sense_size;u_int sense_offset; +u8 *sense_buffer;if (scb->flags &SCB_SENSE ) { - sense_size = min(sizeof(struct scsi_sense_data) - - ahd_get_sense_residual(scb), -(u_long)SCSI_SENSE_BUFFERSIZE);
+sense_size= sizeof(structscsi_sense_data) -
+ahd_get_sense_residual(scb); sense_offset = 0; } else { /*drivers/scsi/aic7xxx/aic79xx_osm.c +4
-8@@ -1792,16 +1785,12 @@ ahd_linux_handle_scsi_status(structahd_softc *ahd, */ siu = (struct scsi_status_iu_header *) scb->sense_data; - sense_size =min_t (size_t, -scsi_4btoul (siu->sense_length), - SCSI_SENSE_BUFFERSIZE); + sense_size =scsi_4btoul (siu->sense_length); sense_offset =SIU_SENSE_OFFSET (siu); } - -memset (cmd->sense_buffer,0 , SCSI_SENSE_BUFFERSIZE); -memcpy (cmd->sense_buffer, -ahd_get_sense_buf (ahd, scb) - + sense_offset, sense_size); + sense_buffer =ahd_get_sense_buf (ahd, scb) + + sense_offset; + scsi_eh_cpy_sense(cmd, sense_buffer,sense_size); cmd->result |= (DRIVER_SENSE << 24);#ifdef
AHD_DEBUGdrivers/scsi/aic7xxx/aic79xx_osm.c +1
-1@@ -1813,7 +1802,7 @@ ahd_linux_handle_scsi_status(struct ahd_softc *ahd, for (i = 0; i < sense_size; i++) {if ((i &0xF ) ==0 )printf ("\n" ); -printf ("0x%x " , cmd->sense_buffer[i]); +printf ("0x%x " , sense_buffer[i]);} printf("\n"); }drivers/scsi/aic7xxx/aic79xx_osm.c +3
-4@@ -1918,10 +1907,9 @@ ahd_linux_queue_cmd_complete(struct ahd_softc *ahd, struct scsi_cmnd *cmd)if ((cmd->result >>24 ) !=DRIVER_SENSE ) { do_fallback =1 ; }else { -struct scsi_sense_data *sense; - - sense = (struct scsi_sense_data *) - cmd->sense_buffer; +struct scsi_sense_data *sense = + (struct scsi_sense_data *)scsi_sense (cmd); + if (sense->extra_len >= 5 &&(sense->add_sense_code == 0x47 || sense->add_sense_code==
0x48))drivers/scsi/aic7xxx/aic7xxx_osm.c +1
-14@@ -1677,13 +1677,6 @@ ahc_done(struct ahc_softc *ahc, struct scb *scb) }ahc_linux_unmap_scb (ahc, scb); -/* -* Guard against stale sense data. -* The Linux mid-layer assumes that sense -* was retrieved anytime the first byte of -* the sense buffer looks "sane". -*/ - cmd->sense_buffer[0 ] =0 ;if (ahc_get_transaction_status (scb) ==CAM_REQ_INPROG ) {uint32_t amount_xferred;@@ -1800,15 +1793,12 @@ ahc_linux_handle_scsi_status(structahc_softc *ahc, cmd = scb->io_ctx;if (scb->flags &SCB_SENSE ) {u_int sense_size; +u8 *sense_buffer; - sense_size =min (sizeof (struct scsi_sense_data) - -ahc_get_sense_residual (scb), - (u_long)SCSI_SENSE_BUFFERSIZE); -memcpy (cmd->sense_buffer, -ahc_get_sense_buf (ahc, scb), sense_size); -if (sense_size <SCSI_SENSE_BUFFERSIZE ) -memset(&cmd->sense_buffer[sense_size], 0,
drivers/scsi/aic7xxx/aic7xxx_osm.c +3
-2-SCSI_SENSE_BUFFERSIZE - sense_size); + sense_size =sizeof (struct scsi_sense_data ) + -ahc_get_sense_residual (scb); + sense_buffer = (u8*)ahc_get_sense_buf(ahc, scb);
+ scsi_eh_cpy_sense(cmd,sense_buffer,sense_size); cmd->result |= (DRIVER_SENSE << 24); #ifdef AHC_DEBUG if (ahc_debug & AHC_SHOW_SENSE) {drivers/scsi/aic7xxx/aic7xxx_osm.c +1
-1@@ -1819,7 +1809,7 @@ ahc_linux_handle_scsi_status(struct ahc_softc *ahc, for (i = 0; i < sense_size; i++) {if ((i &0xF ) ==0 )printf ("\n" ); -printf ("0x%x " , cmd->sense_buffer[i]); +printf ("0x%x " , sense_buffer[i]);} printf("\n"); }drivers/scsi/arcmsr/arcmsr_hba.c +3
-4@@ -69,6 +69,7 @@#include <scsi/scsi_device.h> #include <scsi/scsi_transport.h> #include <scsi/scsicam.h> +#include <scsi/scsi_eh.h> #include "arcmsr.h" MODULE_AUTHOR ("Erich Chen <[email protected]>" );@@ -628,18 +629,15 @@ static void arcmsr_report_sense_info(struct CommandControlBlock *ccb) {struct scsi_cmnd *pcmd = ccb->pcmd; -struct SENSE_DATA *sensebuffer = (struct SENSE_DATA *)pcmd->sense_buffer; +struct SENSE_DATA *sensebuffer = + (struct SENSE_DATA *)ccb->arcmsr_cdb.SenseData; pcmd->result =DID_OK <<16 ; -if (sensebuffer) { -int sense_data_length = - sizeof(struct SENSE_DATA) <
SCSI_SENSE_BUFFERSIZE
- ?sizeof(struct SENSE_DATA):SCSI_SENSE_BUFFERSIZE;drivers/scsi/arcmsr/arcmsr_hba.c +0
-2-memset (sensebuffer,0 ,SCSI_SENSE_BUFFERSIZE ); - memcpy(sensebuffer,
ccb->arcmsr_cdb.SenseData,sense_data_length);drivers/scsi/arcmsr/arcmsr_hba.c +5
-3- sensebuffer->ErrorCode =SCSI_SENSE_CURRENT_ERRORS ; - sensebuffer->Valid =1 ; - } + + sensebuffer->ErrorCode =SCSI_SENSE_CURRENT_ERRORS ; + sensebuffer->Valid =1 ; + +scsi_eh_cpy_sense (pcmd, sensebuffer,sizeof (sensebuffer)); } static u32 arcmsr_disable_outbound_ints(structAdapterControlBlock *acb)
drivers/scsi/arcmsr/arcmsr_hba.c +3
-1@@ -1442,8 +1440,10 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb, \ wqbuf_lastindex = acb->wqbuf_lastindex; wqbuf_firstindex = acb->wqbuf_firstindex;if (wqbuf_lastindex != wqbuf_firstindex) { +u8 sb[16 ];struct SENSE_DATA *sensebuffer = - (struct SENSE_DATA *)cmd->sense_buffer; + (struct SENSE_DATA *)sb; +memset (sensebuffer,0 ,sizeof (sensebuffer));arcmsr_post_ioctldata2iop(acb); /* has error report sensedata */ sensebuffer->ErrorCode = 0x70;drivers/scsi/arcmsr/arcmsr_hba.c +2
-0@@ -1451,6 +1451,8 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb, \ sensebuffer->AdditionalSenseLength = 0x0A; sensebuffer->AdditionalSenseCode =0x20 ; sensebuffer->Valid =1 ; +scsi_eh_cpy_sense (cmd, sensebuffer, +sizeof (*sensebuffer)); retvalue =ARCMSR_MESSAGE_FAIL ;} else { my_empty_len =(wqbuf_firstindex-wqbuf_lastindex - 1)
drivers/scsi/arcmsr/arcmsr_hba.c +4
-2@@ -1471,14 +1473,18 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb, \arcmsr_post_ioctldata2iop (acb); } }else { -/* has error report sensedata */ +u8 sb[16 ];struct SENSE_DATA *sensebuffer = - (struct SENSE_DATA *)cmd->sense_buffer; + (struct SENSE_DATA *)sb; +memset (sensebuffer,0 ,sizeof (sensebuffer)); +/* has error report sensedata */ sensebuffer->ErrorCode =0x70 ; sensebuffer->SenseKey =ILLEGAL_REQUEST ;
sensebuffer->AdditionalSenseLength=0x0A;sensebuffer->AdditionalSenseCode = 0x20; sensebuffer->Valid = 1;drivers/scsi/arcmsr/arcmsr_hba.c +2
-0+ scsi_eh_cpy_sense(cmd, sensebuffer, +sizeof(sensebuffer)); retvalue = ARCMSR_MESSAGE_FAIL; } }drivers/scsi/arm/fas216.c +1
-1@@ -2009,7 +2009,7 @@ static voidfas216_rq_sns_done(FAS216_Info *info, struct scsi_cmnd *SCpnt, * have valid data in the sense buffer that could * confuse the higher levels. */ -memset (SCpnt->sense_buffer,0 , sizeof(SCpnt->sense_buffer)); +scsi_eh_reset_sense (SCpnt);//printk("scsi%d.%c: sense buffer: ", info->host->host_no, '0' +
SCpnt->device->id);
//{ int i; for (i = 0; i < 32; i++) printk("%02x ",
SCpnt->sense_buffer[i]);printk("\n"); }/*drivers/scsi/ipr.c +2
-2@@ -4379,8 +4379,8 @@ static void ipr_erp_done(struct ipr_cmnd *ipr_cmd) scmd_printk(KERN_ERR, scsi_cmd, "Request Sense failed with IOASC: 0x%08X\n", ioasc); }else { -memcpy (scsi_cmd->sense_buffer, ipr_cmd->sense_buffer, -SCSI_SENSE_BUFFERSIZE ); +scsi_eh_cpy_sense (scsi_cmd, ipr_cmd->sense_buffer, +sizeof (ipr_cmd->sense_buffer)); }if (res) {drivers/scsi/ipr.c +2
-2@@ -4574,12 +4574,12 @@ static void ipr_dump_ioasa(structipr_ioa_cfg *ioa_cfg, static void ipr_gen_sense(struct ipr_cmnd *ipr_cmd) { u32 failing_lba; -u8 *sense_buf = ipr_cmd->scsi_cmd->sense_buffer; +u8 sense_buf[24 ];struct ipr_resource_entry *res = ipr_cmd->scsi_cmd->device->hostdata;struct ipr_ioasa *ioasa = &ipr_cmd->ioasa;u32 ioasc =be32_to_cpu (ioasa->ioasc); -memset (sense_buf,0 ,SCSI_SENSE_BUFFERSIZE ); +memset (sense_buf,0 ,sizeof (sense_buf));if (ioasc >=
IPR_FIRST_DRIVER_IOASC)
return;drivers/scsi/ipr.c +0
-0@@ -4652,6 +4652,8 @@ static void ipr_gen_sense(struct ipr_cmnd *ipr_cmd) sense_buf[7 ] =6 ;/* additional length */
} }+ scsi_eh_cpy_sense(ipr_cmd->scsi_cmd,
sense_buf,sizeof(sense_buf));drivers/scsi/ipr.c +3
-3+ }/** @@ -4671,9 +4673,8 @@ static int ipr_get_autosense(struct ipr_cmnd *ipr_cmd)if ((be32_to_cpu (ioasa->ioasc_specific) &IPR_AUTOSENSE_VALID ) ==0 )return 0 ; -memcpy (ipr_cmd->scsi_cmd->sense_buffer, ioasa->auto_sense.data, -min_t (u16,be16_to_cpu (ioasa->auto_sense.auto_sense_len), -SCSI_SENSE_BUFFERSIZE )); + scsi_eh_cpy_sense(ipr_cmd->scsi_cmd, ioasa->auto_sense.data, +be16_to_cpu(ioasa->auto_sense.auto_sense_len)); return 1;}
drivers/scsi/ipr.c +1
-1@@ -4836,7 +4837,7 @@ static int ipr_queuecommand(structscsi_cmnd *scsi_cmd, * and the ioa_is_dead check can be removed */ if (unlikely (ioa_cfg->ioa_is_dead || !res)) { -memset (scsi_cmd->sense_buffer,0 , SCSI_SENSE_BUFFERSIZE); +scsi_eh_reset_sense (scsi_cmd); scsi_cmd->result = (DID_NO_CONNECT <<16 );scsi_cmd->scsi_done(scsi_cmd); return 0;drivers/scsi/ips.c +2
-3@@ -3431,13 +3431,13 @@ ips_map_status(ips_ha_t * ha, ips_scb_t * scb, ips_stat_t * sp) IPS_CMD_EXTENDED_DCDB_SG)) { tapeDCDB = (IPS_DCDB_TABLE_TAPE *) & scb->dcdb; -memcpy (scb->scsi_cmd->sense_buffer, +scsi_eh_cpy_sense (scb->scsi_cmd, tapeDCDB->sense_info, - SCSI_SENSE_BUFFERSIZE); + sizeof(tapeDCDB->sense_info)); } else { -memcpy(scb->scsi_cmd->sense_buffer,
+
scsi_eh_cpy_sense(scb->scsi_cmd,scb->dcdb.sense_info,drivers/scsi/ips.c +1
-1- SCSI_SENSE_BUFFERSIZE); +
sizeof(scb->dcdb.sense_info));
}
device_error= 2; /*
check condition */}drivers/scsi/ips.c +1
-1@@ -3571,7 +3571,7 @@ static int ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb) {int ret; -char *sp; +u8 sp[16 ];int device_error;IPS_DCDB_TABLE_TAPE *tapeDCDB;int TimeOut;@@ -3816,13 +3816,14 @@ ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb)/* Set the Return Info to appear like the Command was */
/* attempted, a Check Condition occurred, and Sense */ /* Data indicating an Invalid CDBOpCode is returned. */
drivers/scsi/ips.c +1
-1- sp = (char *) scb->scsi_cmd->sense_buffer; +memset (sp,0 ,sizeof (sp)); sp[0 ] =0x70 ;/* Error Code */ sp[2 ] =ILLEGAL_REQUEST ;/* Sense Key 5 Illegal Req. */
sp[7] = 0x0A; /* Additional Sense Length */ sp[12] = 0x20; /* ASC = Invalid OpCode */ sp[13] = 0x00; /* ASCQ */+ scsi_eh_cpy_sense(scb->scsi_cmd, sp,
sizeof(sp));
device_error= 2; /* Indicate
Check Condition */scb->scsi_cmd->result = device_error| (
DID_OK<< 16);drivers/scsi/megaraid/megaraid_mbox.c +8
-3@@ -70,6 +70,7 @@* For history of changes, see Documentation/ChangeLog.megaraid */ +#include <scsi/scsi_eh.h> #include "megaraid_mbox.h" static int megaraid_init (void );@@ -1616,9 +1617,13 @@ megaraid_mbox_build_cmd(adapter_t *adapter, struct scsi_cmnd *scp, int *busy) }if (scp->cmnd[1 ] &MEGA_SCSI_INQ_EVPD ) { - scp->sense_buffer[0 ] =0x70 ; - scp->sense_buffer[2 ] =ILLEGAL_REQUEST ; - scp->sense_buffer[12 ] =MEGA_INVALID_FIELD_IN_CDB ; +u8 sense[14 ]; + +memset (sense,0 ,sizeof (sense)); + sense[0 ] =0x70 ; + sense[2 ] =ILLEGAL_REQUEST ; + sense[12 ] =MEGA_INVALID_FIELD_IN_CDB ; + scsi_eh_cpy_sense(scp, sense,sizeof(sense)); scp->result = CHECK_CONDITION << 1; return NULL; }drivers/scsi/megaraid/megaraid_mbox.c +1
-2@@ -2365,8 +2370,7 @@ megaraid_mbox_dpc(unsigned long devp)if (mbox->cmd ==MBOXCMD_PASSTHRU || mbox->cmd ==MBOXCMD_PASSTHRU64 ) { -memcpy (scp->sense_buffer, pthru->reqsensearea, -14 ); +scsi_eh_cpy_sense (scp, pthru->reqsensearea,14 );scp->result = DRIVER_SENSE << 24 | DID_OK << 16 |
CHECK_CONDITION<< 1;drivers/scsi/megaraid/megaraid_mbox.c +1
-1@@ -2374,15 +2378,19 @@ megaraid_mbox_dpc(unsigned long devp)else {if (mbox->cmd ==MBOXCMD_EXTPTHRU ) { -memcpy (scp->sense_buffer, +scsi_eh_cpy_sense (scp, epthru->reqsensearea,14 ); scp->result =
DRIVER_SENSE<< 24 |DID_OK << 16 | CHECK_CONDITION << 1; } else {drivers/scsi/megaraid/megaraid_mbox.c +0
-2- scp->sense_buffer[0 ] =0x70 ; - scp->sense_buffer[2 ]=
ABORTED_COMMAND;drivers/scsi/megaraid/megaraid_mbox.c +6
-0+u8 sn[4 ]; + +memset (sn,0 ,sizeof (sn)); + sn[0 ] =0x70 ; + sn[2 ] =ABORTED_COMMAND ; +scsi_eh_cpy_sense(scp, sn, sizeof(sn)); scp->result = CHECK_CONDITION << 1; } }drivers/scsi/ps3rom.c +6
-3@@ -323,10 +323,13 @@ static int ps3rom_queuecommand(struct scsi_cmnd *cmd, }if (res) { -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); +u8 sense[4 ]; + +memset (sense,0 ,sizeof (sense)); cmd->result = res; - cmd->sense_buffer[0 ] =0x70 ; - cmd->sense_buffer[2 ] =ILLEGAL_REQUEST ; + sense[0 ] =0x70 ; + sense[2 ] =ILLEGAL_REQUEST ; + scsi_eh_cpy_sense(cmd, sense, sizeof(sense); priv->curr_cmd =NULL ; cmd->scsi_done (cmd);}drivers/scsi/ps3rom.c +12
-10@@ -399,17 +402,19 @@ static irqreturn_t ps3rom_interrupt(int irq, void *data) goto done; } -if (decode_lv1_status (status, &sense_key, &asc, &ascq)) { +if (!decode_lv1_status (status, &sense_key, &asc, &ascq)) { +u8 sense[14 ]; + +memset (sense,0 ,sizeof (sense)); + sense[0 ] =0x70 ; + sense[2 ] = sense_key; + sense[7 ] =16 -6 ; + sense[12 ] = asc; + sense[13 ] = ascq; +scsi_eh_cpy_sense (cmd, sense,sizeof (sense)); + cmd->result =SAM_STAT_CHECK_CONDITION ; + }else cmd->result =DID_ERROR <<16 ; - goto done; - } - - cmd->sense_buffer[0 ] =0x70 ; - cmd->sense_buffer[2 ] = sense_key; - cmd->sense_buffer[7 ] =16 -6 ; - cmd->sense_buffer[12 ] = asc; - cmd->sense_buffer[13 ] = ascq; - cmd->result =SAM_STAT_CHECK_CONDITION ; done:priv->curr_cmd = NULL;drivers/scsi/qla1280.c +6
-18@@ -362,6 +362,7 @@#include <scsi/scsi_device.h> #include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> +#include <scsi/scsi_eh.h> #if defined(CONFIG_IA64_GENERIC ) || defined(CONFIG_IA64_SGI_SN2 )#include <asm/sn/io.h> @@ -527,8 +528,6 @@ __setup("qla1280=", qla1280_setup);#define CMD_SP (Cmnd) &Cmnd->SCp#define CMD_CDBLEN (Cmnd) Cmnd->cmd_len#define CMD_CDBP (Cmnd) Cmnd->cmnd -#define CMD_SNSP (Cmnd) Cmnd->sense_buffer -#define CMD_SNSLEN (Cmnd) SCSI_SENSE_BUFFERSIZE#define CMD_RESULT (Cmnd) Cmnd->result#define CMD_HANDLE (Cmnd) Cmnd->host_scribble#define CMD_REQUEST (Cmnd) Cmnd->request->cmd@@ -3698,32 +3697,21 @@ qla1280_status_entry(struct scsi_qla_host *ha, struct response *pkt, if (scsi_status & SAM_STAT_CHECK_CONDITION) { if (comp_status != CS_ARS_FAILED) { - uint16_t req_sense_length = - le16_to_cpu(pkt->req_sense_length); -if (req_sense_length < CMD_SNSLEN(cmd)) - sense_sz = req_sense_length; -else -/* -* scsi_cmnd->sense_buffer is -* 64 bytes, why only copy 63? -* This looks wrong! /Jes -*/ - sense_sz =CMD_SNSLEN (cmd) -1 ; + sense_sz =min_t (size_t, +sizeof (pkt->req_sense_data), +le16_to_cpu (pkt->req_sense_length)); -memcpy (cmd->sense_buffer, - &pkt->req_sense_data, sense_sz); +scsi_eh_cpy_sense (cmd, pkt->req_sense_data, + sense_sz); } else sense_sz =0 ; -memset (cmd->sense_buffer + sense_sz,0 , -SCSI_SENSE_BUFFERSIZE - sense_sz);dprintk (2 ,"qla1280_status_entry: Check " "condition Sense data, b %i, t %i, " "l %i\n" , bus, target, lun); if (sense_sz) qla1280_dump_buffer(2, - (char*)cmd->sense_buffer,
drivers/scsi/qla1280.c +1
-1- sense_sz); + (char*)pkt->req_sense_data, sense_sz); } }drivers/scsi/qla4xxx/ql4_isr.c +1
-0@@ -10,6 +10,7 @@#include "ql4_dbg.h" #include "ql4_inline.h" +#include <scsi/scsi_eh.h> /** * qla2x00_process_completed_request() - Process a Fast Post response.
* @ha: SCSI driver HA contextdrivers/scsi/qla4xxx/ql4_isr.c +1
-4@@ -123,14 +124,11 @@ static void qla4xxx_status_entry(structscsi_qla_host *ha, break;/* Copy Sense Data into sense buffer. */ -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); - sensebytecnt =le16_to_cpu (sts_entry->senseDataByteCnt);if (sensebytecnt ==0 )break ; -memcpy (cmd->sense_buffer, sts_entry->senseData, -min_t (uint16_t, sensebytecnt, SCSI_SENSE_BUFFERSIZE)); + scsi_eh_cpy_sense(cmd, sts_entry->senseData,sensebytecnt);
DEBUG2(printk("scsi%ld:%d:%d:%d: %s: sense key = %x, " "ASC/ASCQ = %02x/%02x\n", ha->host_no,drivers/scsi/qla4xxx/ql4_isr.c +0
-4@@ -207,15 +205,13 @@ static void qla4xxx_status_entry(structscsi_qla_host *ha, break;/* Copy Sense Data into sense buffer. */ -memset (cmd->sense_buffer,0 , SCSI_SENSE_BUFFERSIZE); - sensebytecnt =le16_to_cpu (sts_entry->senseDataByteCnt);if (sensebytecnt ==0 )break ; - memcpy(cmd->sense_buffer, sts_entry->senseData, - min_t(uint16_t, sensebytecnt,
SCSI_SENSE_BUFFERSIZE));drivers/scsi/qla4xxx/ql4_isr.c +2
-0+scsi_eh_cpy_sense (cmd, sts_entry->senseData, + sensebytecnt); DEBUG2(printk("scsi%ld:%d:%d:%d: %s:sense key = %x, "
"ASC/ASCQ =%02x/%02x\n",
ha->host_no,drivers/scsi/stex.c +11
-7@@ -33,6 +33,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/scsi_dbg.h> +#include <scsi/scsi_eh.h> #define DRV_NAME "stex"#define ST_DRIVER_VERSION "3.6.0000.1"@@ -290,8 +291,6 @@ struct st_ccb {struct req_msg *req;struct scsi_cmnd *cmd; -void *sense_buffer; -unsigned int sense_bufflen;int sg_count;u32 req_type;@@ -364,13 +363,18 @@ static struct status_msg *stex_get_status(struct st_hba *hba)static void stex_set_sense (struct scsi_cmnd *cmd,u8 sk,u8 asc,u8 ascq) { +u8 sense_buffer[14 ]; + +memset (sense_buffer,0 ,sizeof (sense_buffer)); cmd->result = (DRIVER_SENSE <<24 ) |SAM_STAT_CHECK_CONDITION ; - cmd->sense_buffer[0 ] =0x70 ;/* fixed format, current */ - cmd->sense_buffer[2 ] = sk; - cmd->sense_buffer[7 ] =18 -8 ;/* additional sense length */ - cmd->sense_buffer[12 ] = asc; - cmd->sense_buffer[13 ] = ascq; + sense_buffer[0 ] =0x70 ;/* fixed format, current */ + sense_buffer[2 ] = sk; + sense_buffer[7 ] =18 -8 ;/* additional sense length */ + sense_buffer[12 ] = asc; + sense_buffer[13 ] = ascq; + +scsi_eh_cpy_sense (cmd, sense_buffer,sizeof (sense_buffer)); }static void stex_invalid_field(struct
scsi_cmnd*cmd,drivers/scsi/stex.c +0
-2@@ -642,8 +646,6 @@ stex_queuecommand(struct scsi_cmnd *cmd, void (* done)(struct scsi_cmnd *)) memcpy(req->cdb, cmd->cmnd, STEX_CDB_LENGTH); hba->ccb[tag].cmd = cmd; - hba->ccb[tag].sense_bufflen =SCSI_SENSE_BUFFERSIZE ; - hba->ccb[tag].sense_buffer = cmd->sense_buffer; hba->ccb[tag].req_type =0 ;if (cmd->sc_data_direction != DMA_NONE)drivers/scsi/stex.c +4
-8@@ -699,15 +701,14 @@ static void stex_copy_data(struct st_ccb *ccb, struct status_msg *resp, unsigned int variable) { size_t count = variable; + +if (ccb->cmd ==NULL ) +return ;if (resp->scsi_status !=SAM_STAT_GOOD ) { -if (ccb->sense_buffer !=NULL ) -memcpy (ccb->sense_buffer, resp->variable, -min (variable, ccb->sense_bufflen)); +scsi_eh_cpy_sense (ccb->cmd, resp->variable, variable);return ; } -if (ccb->cmd ==NULL ) -return ;stex_internal_copy (ccb->cmd, resp->variable, &count, ccb->sg_count,ST_TO_CMD ); }@@ -1295,8 +1296,6 @@ static void stex_hba_stop(struct st_hba *hba) hba->ccb[tag].cmd =NULL ; hba->ccb[tag].sg_count =0 ; - hba->ccb[tag].sense_bufflen =0 ; - hba->ccb[tag].sense_buffer =NULL ; hba->ccb[tag].req_type |=PASSTHRU_REQ_TYPE ;stex_send_cmd (hba, req, tag); --1.5.3.3
-
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
RFC PATCH 4/24 firewire & ieee1394: Simple convert to new scsi_eh_cpy_sense.
In reply to Boaz Harrosh Abstract away scsi_cmnd->sense_buffer for later removal.
--- drivers/firewire/fw-sbp2.c | 7 +++++-- drivers/ieee1394/sbp2.c | 9 +++++++-- 2 files changed, 12 insertions(+), 4 deletions(-)
drivers/firewire/fw-sbp2.c +5 -2
@@ -46,6 +46,7 @@#include <scsi/scsi_cmnd.h> #include <scsi/scsi_device.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_eh.h> #include "fw-transaction.h" #include "fw-topology.h"
@@ -1016,8 +1017,9 @@ static struct fw_driver sbp2_driver = { };static unsigned int -sbp2_status_to_sense_data (u8 *sbp2_status,u8 *sense_data) +sbp2_status_to_sense_data (u8 *sbp2_status,struct scsi_cmnd *srb) { +u8 sense_data[16 ];int sam_status; sense_data[0 ] =0x70 ;
@@ -1036,6 +1038,7 @@ sbp2_status_to_sense_data(u8 *sbp2_status, u8 *sense_data) sense_data[13 ] = sbp2_status[3 ]; sense_data[14 ] = sbp2_status[12 ]; sense_data[15 ] = sbp2_status[13 ]; +scsi_eh_cpy_sense (srb, sense_data,sizeof (sense_data)); sam_status = sbp2_status[0 ] &0x3f ;
@@ -1081,7 +1084,7 @@ complete_command_orb(struct sbp2_orb *base_orb, struct sbp2_status *status)if (result ==DID_OK <<16 &&STATUS_GET_LEN (*status) >1 ) result =sbp2_status_to_sense_data (STATUS_GET_DATA (*status), - orb->cmd->sense_buffer); + orb->cmd); }else {/* * If the orb completes with status == NULL, something
drivers/ieee1394/sbp2.c +7 -2
@@ -89,6 +89,7 @@#include <scsi/scsi_dbg.h> #include <scsi/scsi_device.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_eh.h> #include "csr1212.h" #include "highlevel.h"
@@ -1672,8 +1673,11 @@ static int sbp2_send_command(struct sbp2_lu *lu, struct scsi_cmnd *SCpnt,* Translates SBP-2 status into SCSI sense data for check conditions */ static unsigned int sbp2_status_to_sense_data (unchar *sbp2_status, -unchar *sense_data) +struct scsi_cmnd *SCpnt) { +u8 sense_data[16 ]; + +memset (sense_data,0 ,sizeof (sense_data));/* OK, it's pretty ugly... ;-) */ sense_data[0 ] =0x70 ; sense_data[1 ] =0x0 ;
@@ -1691,6 +1695,7 @@ static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status, sense_data[13 ] = sbp2_status[11 ]; sense_data[14 ] = sbp2_status[20 ]; sense_data[15 ] = sbp2_status[21 ]; +scsi_eh_cpy_sense (SCpnt, sense_data,sizeof (sense_data));return sbp2_status[8 ] &0x3f ; }
@@ -1784,7 +1789,7 @@ static int sbp2_handle_status_write(struct hpsb_host *host, int nodeid,if (STATUS_GET_LEN (h) >1 ) scsi_status =sbp2_status_to_sense_data ( - (unchar *)sb, SCpnt->sense_buffer); + (unchar *)sb, SCpnt);if (STATUS_TEST_DEAD (h))sbp2_agent_reset (lu,0 );
Signature · 1 line
1.5.3.3
Re: [PATCH 4/24][RFC] firewire & ieee1394: Simple convert to new scsi_eh_cpy_sense.
Quoted 16 lines · Boaz Harrosh
drivers/ieee1394/sbp2.c +4
-1@@ -1672,8 +1673,11 @@ static int sbp2_send_command(struct sbp2_lu *lu, struct scsi_cmnd *SCpnt,* Translates SBP-2 status into SCSI sense data for check conditions */ static unsigned int sbp2_status_to_sense_data (unchar *sbp2_status, -unchar *sense_data) +struct scsi_cmnd *SCpnt) { +u8 sense_data[16 ]; + +memset (sense_data,0 ,sizeof (sense_data));/* OK, it's pretty ugly... ;-) */ sense_data[0 ] =0x70 ; sense_data[1 ] =0x0 ;
drivers/ieee1394/sbp2.c static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status, · line 1701
Quoted 7 lines
drivers/ieee1394/sbp2.c +1
-0@@ -1691,6 +1695,7 @@ static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status, sense_data[13 ] = sbp2_status[11 ]; sense_data[14 ] = sbp2_status[20 ]; sense_data[15 ] = sbp2_status[21 ]; +scsi_eh_cpy_sense (SCpnt, sense_data,sizeof (sense_data));return sbp2_status[8 ] &0x3f ; }
You don't need the memset.
Also, here and in drivers/firewire/fw-sbp2.c, the SCSI sense data could AFAICS be rewritten in-place in sbp2_status. But I don't know if this is a worthwhile optimization; it would reduce readability.
Signature · 3 lines
Stefan Richter -=====-==--- --=- --=-- http://arcgraph.de/sr/
Re: [PATCH 4/24][RFC] firewire & ieee1394: Simple convert to new scsi_eh_cpy_sense.
Quoted 22 lines · Stefan Richter
Quoted 21 lines · Boaz Harrosh
drivers/ieee1394/sbp2.c +5
-1@@ -1672,8 +1673,11 @@ static int sbp2_send_command(struct sbp2_lu *lu, struct scsi_cmnd *SCpnt,* Translates SBP-2 status into SCSI sense data for check conditions */ static unsigned int sbp2_status_to_sense_data (unchar *sbp2_status, -unchar *sense_data) +struct scsi_cmnd *SCpnt) { +u8 sense_data[16 ]; + +memset (sense_data,0 ,sizeof (sense_data));/* OK, it's pretty ugly... ;-) */ sense_data[0 ] =0x70 ; sense_data[1 ] =0x0 ;@@ -1691,6 +1695,7 @@ static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status, sense_data[13 ] = sbp2_status[11 ]; sense_data[14 ] = sbp2_status[20 ]; sense_data[15 ] = sbp2_status[21 ]; +scsi_eh_cpy_sense (SCpnt, sense_data,sizeof (sense_data));
Quoted 5 lines
drivers/ieee1394/sbp2.c static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status, · line 1701
Quoted 2 lines
drivers/ieee1394/sbp2.c +0
-0return sbp2_status[8 ] &0x3f ; }You don't need the memset.
OK I see what you mean now, they are all used. Thanks.
Quoted 3 lines
Also, here and in
drivers/firewire/fw-sbp2.c, the SCSI sense data could AFAICS be rewritten in-place insbp2_status. But I don't know if this is a worthwhile optimization; it would reduce readability.
Right, it's a very unlikely code path. readability is more important here.
I will fix both places. Maybe I can use also here what Jeff suggested in the other mail. We'll see how it goes.
Boaz
RFC PATCH 5/24 dpt_i2o: Use new scsi_eh_cpy_sense()
In reply to Boaz Harrosh - Abstract away scsi_cmnd->sense_buffer for later removal.
- Removed a filtering out of a REQUEST_SENSE at .queuecommand In the case of sense beeing clean. This is no longer relevant since scsi-ml will always send a zero out sense buffer even on a resend, so this means outside REQUEST_SENSE would never go through. If this is intended then comment and check should change.
--- drivers/scsi/dpt_i2o.c | 25 +++++++------------------ 1 files changed, 7 insertions(+), 18 deletions(-)
drivers/scsi/dpt_i2o.c +7 -18
@@ -71,6 +71,7 @@ MODULE_DESCRIPTION("Adaptec I2O RAID Driver");#include <scsi/scsi_device.h> #include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> +#include <scsi/scsi_eh.h> #include "dpt/dptsig.h" #include "dpti.h"
@@ -385,18 +386,6 @@ static int adpt_queue(struct scsi_cmnd * cmd, void (*done) (struct scsi_cmnd *))struct adpt_device * pDev =NULL ;/* dpt per device information */ cmd->scsi_done = done; -/* -* SCSI REQUEST_SENSE commands will be executed automatically by the -* Host Adapter for any errors, so they should not be executed -* explicitly unless the Sense Data is zero indicating that no error -* occurred. -*/ - -if ((cmd->cmnd[0 ] ==REQUEST_SENSE ) && (cmd->sense_buffer[0 ] !=0 )) { - cmd->result = (DID_OK <<16 ); - cmd->scsi_done (cmd); -return 0 ; - } pHba = (adpt_hba *)cmd->device->host->hostdata[0 ];if (!pHba) {
@@ -2226,8 +2215,6 @@ static s32 adpt_i2o_to_scsi(void __iomem *reply, struct scsi_cmnd* cmd) pHba = (adpt_hba *) cmd->device->host->hostdata[0 ]; - cmd->sense_buffer[0 ] ='\0' ;// initialize sense valid flag to false -if (!(reply_flags &MSG_FAIL )) {switch (detailed_status &I2O_SCSI_DSC_MASK ) {case I2O_SCSI_DSC_SUCCESS :
@@ -2297,11 +2284,13 @@ static s32 adpt_i2o_to_scsi(void __iomem *reply, struct scsi_cmnd* cmd)// copy over the request sense data if it was a check // condition status if (dev_status ==SAM_STAT_CHECK_CONDITION ) { -u32 len =min (SCSI_SENSE_BUFFERSIZE ,40 ); +u8 sense_buffer[40 ]; +u32 len =sizeof (sense_buffer);// Copy over the sense data -memcpy_fromio (cmd->sense_buffer, (reply+28 ) , len); -if (cmd->sense_buffer[0 ] ==0x70 /* class 7 */ && - cmd->sense_buffer[2 ] ==DATA_PROTECT ){ +memcpy_fromio (sense_buffer, (reply+28 ) , len); +scsi_eh_cpy_sense (cmd, sense_buffer, len); +if (sense_buffer[0 ] ==0x70 /* class 7 */ && + sense_buffer[2 ] ==DATA_PROTECT ){/* This is to handle an array failed */ cmd->result = (DID_TIME_OUT <<16 ); printk(KERN_WARNING"%s: SCSI Data Protect-Device (%d,%d,%d) hba_status=0x%x, dev_status=0x%x, cmd=0x%x\n",
Signature · 1 line
1.5.3.3
RE: [PATCH 5/24][RFC] dpt_i2o: Use new scsi_eh_cpy_sense()
ACK with condition that community accepts the RFC's entire premise.
The removed code that shunted the REQUEST_SENSE was based on the assumption that the sense data in the current scsi command packet was left over from the previous command's execution with a check condition as the scsi command packet is reused to issue the REQUEST_SENSE. For a new, or second from the target's point of view, request sense to the target issued by these older kernels would always return an erased sense. The dpt_i2o driver does not itself maintain the sense history, nor does the Firmware. This behavior, I believe, is not the case for current kernels so the code fragment made little sense (pun not intended). If my historical knowledge is correct, this (now removed) workaround makes no more sense because the scsi layer correctly manages adapters that produce auto-request sense an
d does not ever turn around the command and send a second request for sense information.
Given this understanding, I have no problem with the removed fragment of REQUEST_SENSE shunting. However, I do urge some target error recovery testing, tape drives being the likely type of target affected by this change. I have no such hardware to confirm...
Sincerely -- Mark Salyzyn
Quoted 103 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Boaz Harrosh
Sent: Monday, February 04, 2008 10:59 AM
To: James Bottomley; FUJITA Tomonori; Christoph Hellwig; Jens Axboe; Jeff Garzik; linux-scsi
Cc: Andrew Morton
Subject: [PATCH 5/24][RFC]dpt_i2o: Use newscsi_eh_cpy_sense()- Abstract away
scsi_cmnd->sense_bufferfor later removal.- Removed a filtering out of a
REQUEST_SENSEat .queuecommand In the case of sense beeing clean. This is no longer relevant since scsi-ml will always send a zero out sense buffer even on a resend, so this means outsideREQUEST_SENSEwould never go through. If this is intended then comment and check should change.Signed-off-by Boaz Harrosh <[email protected]>--- drivers/scsi/dpt_i2o.c | 25 +++++++------------------ 1 files changed, 7 insertions(+), 18 deletions(-)drivers/scsi/dpt_i2o.c +1
-0@@ -71,6 +71,7 @@ MODULE_DESCRIPTION("Adaptec I2O RAID Driver");#include <scsi/scsi_device.h> #include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> +#include <scsi/scsi_eh.h> #include "dpt/dptsig.h" #include "dpti.h" @@ -385,18 +386,6 @@ static int adpt_queue(struct scsi_cmnd * cmd, void (*done) (struct scsi_cmnd *))struct adpt_device * pDev =NULL ;/* dpt per device information */
cmd->scsi_done = done;drivers/scsi/dpt_i2o.c +0
-2-/* -* SCSI REQUEST_SENSE commands will be executed automatically by the
drivers/scsi/dpt_i2o.c +0
-2-* Host Adapter for any errors, so they should not be executed -* explicitly unless the Sense Data is zero indicating that no error
drivers/scsi/dpt_i2o.c +0
-4-* occurred. -*/ - - if ((cmd->cmnd[0] == REQUEST_SENSE) &&(cmd->sense_buffer[0] != 0)) {
drivers/scsi/dpt_i2o.c +0
-5- cmd->result = (DID_OK <<16 ); - cmd->scsi_done (cmd); -return 0 ; - } pHba = (adpt_hba *)cmd->device->host->hostdata[0 ];if (!pHba) {@@ -2226,8 +2215,6 @@ static s32 adpt_i2o_to_scsi(void __iomem *reply, struct scsi_cmnd* cmd) pHba = (adpt_hba *) cmd->device->host->hostdata[0 ]; - cmd->sense_buffer[0 ] ='\0' ;// initialize sense valid flag to false
drivers/scsi/dpt_i2o.c +2
-4-if (!(reply_flags &MSG_FAIL )) {switch (detailed_status &I2O_SCSI_DSC_MASK ) {case I2O_SCSI_DSC_SUCCESS :@@ -2297,11 +2284,13 @@ static s32 adpt_i2o_to_scsi(void __iomem *reply, struct scsi_cmnd* cmd)// copy over the request sense data if it was a check // condition status if (dev_status ==SAM_STAT_CHECK_CONDITION ) { -u32 len =min (SCSI_SENSE_BUFFERSIZE ,40 ); +u8 sense_buffer[40 ]; +u32 len =sizeof (sense_buffer);// Copy over the sense data -memcpy_fromio (cmd->sense_buffer, (reply+28) , len); -if (cmd->sense_buffer[0 ] ==0x70 /* class 7 */ &&
drivers/scsi/dpt_i2o.c +4
-1- cmd->sense_buffer[2] == DATA_PROTECT ){ +memcpy_fromio (sense_buffer, (reply+28 ) , len); +scsi_eh_cpy_sense (cmd, sense_buffer, len); +if (sense_buffer[0 ] ==0x70 /* class 7 */ && + sense_buffer[2 ] ==DATA_PROTECT ){/* This is to handle an array failed */
cmd->result = (DID_TIME_OUT << 16); printk(KERN_WARNING"%s: SCSIData Protect-Device (%d,%d,%d) hba_status=0x%x,
dev_status=0x%x, cmd=0x%x\n",
--
1.5.3.3-
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
Re: [PATCH 5/24][RFC] dpt_i2o: Use new scsi_eh_cpy_sense()
Quoted 14 lines · Mark
ACK with condition that community accepts the RFC's entire premise.
The removed code that shunted the
REQUEST_SENSEwas based on the assumption that the sense data in the current scsi command packet was left over from the previous command's execution with a check condition as the scsi command packet is reused to issue theREQUEST_SENSE. For a new, or second from the target's point of view, request sense to the target issued by these older kernels would always return an erased sense. Thedpt_i2odriver does not itself maintain the sense history, nor does the Firmware. This behavior, I believe, is not the case for current kernels so the code fragment made little sense (pun not intended). If my historical knowledge is correct, this (now removed) workaround makes no more sense because the scsi layer correctly manages adapters that produce auto-request sense and does not ever turn around the command and send a second request for sense information.
Quoted 4 lines
Given this understanding, I have no problem with the removed fragment of
REQUEST_SENSEshunting. However, I do urge some target error recovery testing, tape drives being the likely type of target affected by this change. I have no such hardware to confirm...
Sincerely -- Mark Salyzyn
I have removed this test because the midlayer does a scsi_eh_reset_sense() just before the new invocation of a command. So even if the second bad REQUEST_SENSE comes this will not filter it out anymore. If such a thing still happens? A driver state machine must be used to filter it out, or of course midlayer should be fixed.
Boaz
RE: [PATCH 5/24][RFC] dpt_i2o: Use new scsi_eh_cpy_sense()
I do not think the midlayer needs to be fixed. I think this was a bug/feature that presented itself in the 2.2 tree when we were developing this driver in 1996...
Sincerely -- Mark Salyzyn
Quoted 51 lines
-----Original Message-----
From: Boaz Harrosh [mailto:[email protected]]
Sent: Tuesday, February 05, 2008 3:52 AM
To: Salyzyn, Mark
Cc: James Bottomley; FUJITA Tomonori; Christoph Hellwig; Jens Axboe; Jeff Garzik; linux-scsi; Andrew Morton
Subject: Re: [PATCH 5/24][RFC]dpt_i2o: Use newscsi_eh_cpy_sense()Quoted 3 lines · Mark
ACK with condition that community accepts the RFC's entire premise.
The removed code that shunted the
REQUEST_SENSEwas basedon the assumption
Quoted 1 line
that the sense data in the current scsi command packet was
left over from the
Quoted 1 line
previous command's execution with a check condition as the
scsi command packet
Quoted 1 line
is reused to issue the
REQUEST_SENSE. For a new, or secondfrom the target's point
Quoted 1 line
of view, request sense to the target issued by these older
kernels would always
Quoted 1 line
return an erased sense. The
dpt_i2odriver does not itselfmaintain the sense history,
Quoted 1 line
nor does the Firmware. This behavior, I believe, is not the
case for current kernels so
Quoted 1 line
the code fragment made little sense (pun not intended). If
my historical knowledge is
Quoted 1 line
correct, this (now removed) workaround makes no more sense
because the scsi layer correctly
Quoted 1 line
manages adapters that produce auto-request sense and does
not ever turn around the command
Quoted 3 lines
and send a second request for sense information.
Given this understanding, I have no problem with the
removed fragment of
REQUEST_SENSEshunting.Quoted 1 line
However, I do urge some target error recovery testing, tape
drives being the likely type of target
Quoted 2 lines
affected by this change. I have no such hardware to confirm... Sincerely -- Mark Salyzyn
I have removed this test because the midlayer does a
scsi_eh_reset_sense()just before
the new invocation of a command. So even if the second badREQUEST_SENSEcomes this
will not filter it out anymore. If such a thing still happens? A driver state machine
must be used to filter it out, or of course midlayer should be fixed.Boaz
RFC PATCH 6/24 gdth: Use of scsi_eh API and sense accessors
In reply to Boaz Harrosh Use of new scsi_eh API for setting sense information into the scsi command.
--- drivers/scsi/gdth.c | 47 ++++++++++++++++++++++++++--------------------- drivers/scsi/gdth.h | 1 + 2 files changed, 27 insertions(+), 21 deletions(-)
drivers/scsi/gdth.c +26 -21
@@ -2098,6 +2098,16 @@ static void gdth_putq(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar priority)#endif } +static void gdth_set_4byte_sense (struct scsi_cmnd *scp,u8 sense_code) +{ +u8 sense[4 ]; + +memset (sense,0 ,sizeof (sense)); + sense[0 ] =0x70 ; + sense[2 ] = sense_code; +scsi_eh_cpy_sense (scp, sense,sizeof (sense)); +} +static void gdth_next (gdth_ha_str *ha) { registerScsi_Cmnd *pscp;
@@ -2199,9 +2209,7 @@ static void gdth_next(gdth_ha_str *ha) this_cmd = FALSE; next_cmd = FALSE; }else { -memset ((char *)nscp->sense_buffer,0 ,16 ); - nscp->sense_buffer[0 ] =0x70 ; - nscp->sense_buffer[2 ] =NOT_READY ; +gdth_set_4byte_sense (nscp,NOT_READY ); nscp->result = (DID_OK <<16 ) | (CHECK_CONDITION <<1 );if (!nscp_cmndinfo->wait_for_completion) nscp_cmndinfo->wait_for_completion++;
@@ -2244,9 +2252,7 @@ static void gdth_next(gdth_ha_str *ha)TRACE2 (("cmd 0x%x target %d: UNIT_ATTENTION\n" , nscp->cmnd[0 ], t)); ha->hdr[t].media_changed = FALSE; -memset ((char *)nscp->sense_buffer,0 ,16 ); - nscp->sense_buffer[0 ] =0x70 ; - nscp->sense_buffer[2 ] =UNIT_ATTENTION ; +gdth_set_4byte_sense (nscp,UNIT_ATTENTION ); nscp->result = (DID_OK <<16 ) | (CHECK_CONDITION <<1 );if (!nscp_cmndinfo->wait_for_completion) nscp_cmndinfo->wait_for_completion++;
@@ -2263,7 +2269,7 @@ static void gdth_next(gdth_ha_str *ha)if ( (nscp->cmnd[4 ]&1 ) && !(ha->hdr[t].devtype&1 ) ) {TRACE (("Prevent r. nonremov. drive->do nothing\n" )); nscp->result =DID_OK <<16 ; - nscp->sense_buffer[0 ] =0 ; +scsi_eh_reset_sense (nscp);if (!nscp_cmndinfo->wait_for_completion) nscp_cmndinfo->wait_for_completion++;else
@@ -2296,9 +2302,7 @@ static void gdth_next(gdth_ha_str *ha)TRACE2 (("cmd 0x%x target %d: UNIT_ATTENTION\n" , nscp->cmnd[0 ], t)); ha->hdr[t].media_changed = FALSE; -memset ((char *)nscp->sense_buffer,0 ,16 ); - nscp->sense_buffer[0 ] =0x70 ; - nscp->sense_buffer[2 ] =UNIT_ATTENTION ; +gdth_set_4byte_sense (nscp,UNIT_ATTENTION ); nscp->result = (DID_OK <<16 ) | (CHECK_CONDITION <<1 );if (!nscp_cmndinfo->wait_for_completion) nscp_cmndinfo->wait_for_completion++;
@@ -2410,7 +2414,6 @@ static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp) scp->cmnd[0],t)); scp->result =DID_OK <<16 ; - scp->sense_buffer[0 ] =0 ;switch (scp->cmnd[0 ]) {case TEST_UNIT_READY :
@@ -2726,8 +2729,8 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar b) } }else { - page =virt_to_page (scp->sense_buffer); - offset = (ulong )scp->sense_buffer & ~PAGE_MASK ; + page =virt_to_page (cmndinfo->sense); + offset = (ulong )cmndinfo->sense & ~PAGE_MASK ; sense_paddr =pci_map_page (ha->pdev,page,offset,16 ,PCI_DMA_FROMDEVICE );
@@ -3395,9 +3398,14 @@ static int gdth_sync_event(gdth_ha_str *ha, int service, unchar index,pci_unmap_sg (ha->pdev,gdth_sglist (scp),gdth_sg_count (scp), cmndinfo->dma_dir); -if (cmndinfo->sense_paddr) +if (cmndinfo->sense_paddr) {pci_unmap_page (ha->pdev, cmndinfo->sense_paddr,16 ,PCI_DMA_FROMDEVICE ); +/* this here is called before gdth_next so it will not +* overwrite fake sense returned there. +*/ +scsi_eh_cpy_sense (scp, cmndinfo->sense,16 ); + }if (ha->status ==S_OK ) { cmndinfo->status =S_OK ;
@@ -3441,7 +3449,7 @@ static int gdth_sync_event(gdth_ha_str *ha, int service, unchar index, ha->hdr[t].cluster_type &= ~CLUSTER_RESERVED ; } scp->result =DID_OK <<16 ; - scp->sense_buffer[0 ] =0 ; +scsi_eh_reset_sense (scp); } }else { cmndinfo->status = ha->status;
@@ -3457,9 +3465,7 @@ static int gdth_sync_event(gdth_ha_str *ha, int service, unchar index, cmndinfo->priority =HIGH_PRI ;return 2 ; } -memset ((char *)scp->sense_buffer,0 ,16 ); - scp->sense_buffer[0 ] =0x70 ; - scp->sense_buffer[2 ] =NOT_READY ; +gdth_set_4byte_sense (scp,NOT_READY ); scp->result = (DID_OK <<16 ) | (CHECK_CONDITION <<1 ); }else if (service ==CACHESERVICE ) { if (ha->status == S_CACHE_UNKNOWN &&
@@ -3468,12 +3474,11 @@ static int gdth_sync_event(gdth_ha_str *ha, int service, unchar index,/* bus reset -> force GDT_CLUST_INFO */ ha->hdr[t].cluster_type &= ~CLUSTER_RESERVED ; } -memset ((char *)scp->sense_buffer,0 ,16 ); +scsi_eh_reset_sense (scp);if (ha->status == (ushort )S_CACHE_RESERV ) { scp->result = (DID_OK <<16 ) | (RESERVATION_CONFLICT <<1 ); }else { - scp->sense_buffer[0 ] =0x70 ; - scp->sense_buffer[2 ] =NOT_READY ; +gdth_set_4byte_sense (scp,NOT_READY ); scp->result = (DID_OK <<16 ) | (CHECK_CONDITION <<1 ); }if (!cmndinfo->internal_command) {
drivers/scsi/gdth.h +1 -0
@@ -915,6 +915,7 @@ typedef struct {struct gdth_cmndinfo {/* per-command private info */ int index;int internal_command;/* don't call scsi_done */ +u8 sense[16 ];/* 16 is used allover */ dma_addr_t sense_paddr;/* sense dma-addr */ unchar priority;int timeout;
Signature · 1 line
1.5.3.3
Re: [PATCH 6/24][RFC] gdth: Use of scsi_eh API and sense accessors
Quoted 16 lines · Boaz Harrosh
Use of new
scsi_ehAPI for setting sense information into the scsi command.Signed-off-by Boaz Harrosh <[email protected]>--- drivers/scsi/gdth.c | 47 ++++++++++++++++++++++++++--------------------- drivers/scsi/gdth.h | 1 + 2 files changed, 27 insertions(+), 21 deletions(-)drivers/scsi/gdth.c +0
-0@@ -2098,6 +2098,16 @@ static void gdth_putq(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar priority)#endif }
drivers/scsi/gdth.c static void gdth_putq(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar priority) · line 2109
Quoted 10 lines
drivers/scsi/gdth.c +9
-0+static void gdth_set_4byte_sense (struct scsi_cmnd *scp,u8 sense_code) +{ +u8 sense[4 ]; + +memset (sense,0 ,sizeof (sense)); + sense[0 ] =0x70 ; + sense[2 ] = sense_code; +scsi_eh_cpy_sense (scp, sense,sizeof (sense)); +}
IMO, setting 0x70 and 0x72 is highly common, and worthy of some simple helper functions. See ata_scsi_set_sense() in libata-scsi.c or stex_set_sense() in stex.c, which is a copy of the former.
Jeff
Re: [PATCH 6/24][RFC] gdth: Use of scsi_eh API and sense accessors
Quoted 32 lines · Jeff Garzik
Quoted 16 lines · Boaz Harrosh
Use of new
scsi_ehAPI for setting sense information into the scsi command.Signed-off-by Boaz Harrosh <[email protected]>--- drivers/scsi/gdth.c | 47 ++++++++++++++++++++++++++--------------------- drivers/scsi/gdth.h | 1 + 2 files changed, 27 insertions(+), 21 deletions(-)drivers/scsi/gdth.c +0
-0@@ -2098,6 +2098,16 @@ static void gdth_putq(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar priority)#endif }drivers/scsi/gdth.c static void gdth_putq(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar priority) · line 2109
Quoted 10 lines
drivers/scsi/gdth.c +9
-0+static void gdth_set_4byte_sense (struct scsi_cmnd *scp,u8 sense_code) +{ +u8 sense[4 ]; + +memset (sense,0 ,sizeof (sense)); + sense[0 ] =0x70 ; + sense[2 ] = sense_code; +scsi_eh_cpy_sense (scp, sense,sizeof (sense)); +}IMO, setting
0x70and0x72is highly common, and worthy of some simple helper functions. Seeata_scsi_set_sense()inlibata-scsi.corstex_set_sense()instex.c, which is a copy of the former.
Quoted 2 lines
Jeff
Thanks, Yes I was thinking of a more general sense-formating helper but I'm not yet sure of it's API. If you also thinks so, it motivates me to define one and use it in a lot of places that do such formating.
Boaz
RFC PATCH 7/24 qla2xxx: convert to new scsi_eh_cpy_sense API
In reply to Boaz Harrosh this driver is special in that it would read sense in parts until done. Same mechanics left here but read into a driver internal buffer, which is then scsi_eh_cpy_sense() into command when done.
--- drivers/scsi/qla2xxx/qla_def.h | 4 ++-- drivers/scsi/qla2xxx/qla_isr.c | 17 +++++++---------- 2 files changed, 9 insertions(+), 12 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +2 -2
@@ -195,8 +195,9 @@ typedef struct srb {/* Single transfer DMA context */ dma_addr_t dma_handle; -uint32_t request_sense_length; +uint16_t request_sense_length;uint8_t *request_sense_ptr; +uint8_t sense_buffer[SCSI_SENSE_BUFFERSIZE ]; } srb_t;/*
@@ -2601,7 +2602,6 @@ typedef struct scsi_qla_host {#define CMD_COMPL_STATUS (Cmnd) ((Cmnd)->SCp.this_residual)#define CMD_RESID_LEN (Cmnd) ((Cmnd)->SCp.buffers_residual)#define CMD_SCSI_STATUS (Cmnd) ((Cmnd)->SCp.Status) -#define CMD_ACTUAL_SNSLEN (Cmnd) ((Cmnd)->SCp.Message)#define CMD_ENTRY_STATUS (Cmnd) ((Cmnd)->SCp.have_data_in)#endif
drivers/scsi/qla2xxx/qla_isr.c +7 -10
@@ -8,6 +8,7 @@#include <linux/delay.h> #include <scsi/scsi_tcq.h> +#include <scsi/scsi_eh.h> static void qla2x00_mbx_completion (scsi_qla_host_t *,uint16_t );static void qla2x00_process_completed_request (struct scsi_qla_host *,uint32_t );
@@ -825,18 +826,15 @@ qla2x00_process_response_queue(struct scsi_qla_host *ha)static inline void qla2x00_handle_sense (srb_t *sp,uint8_t *sense_data,uint32_t sense_len) { -struct scsi_cmnd *cp = sp->cmd; -if (sense_len >=SCSI_SENSE_BUFFERSIZE ) sense_len =SCSI_SENSE_BUFFERSIZE ; -CMD_ACTUAL_SNSLEN (cp) = sense_len; sp->request_sense_length = sense_len; - sp->request_sense_ptr = cp->sense_buffer; -if (sp->request_sense_length >32 ) + sp->request_sense_ptr = sp->sense_buffer; +if (sense_len >32 ) sense_len =32 ; -memcpy (cp->sense_buffer, sense_data, sense_len); +memcpy (sp->sense_buffer, sense_data, sense_len); sp->request_sense_ptr += sense_len; sp->request_sense_length -= sense_len;
@@ -847,8 +845,7 @@ qla2x00_handle_sense(srb_t *sp, uint8_t *sense_data, uint32_t sense_len)"cmd=%p pid=%ld\n" , __func__, sp->ha->host_no, cp->device->channel, cp->device->id, cp->device->lun, cp, cp->serial_number));if (sense_len) -DEBUG5 (qla2x00_dump_buffer (cp->sense_buffer, -CMD_ACTUAL_SNSLEN (cp))); +DEBUG5 (qla2x00_dump_buffer (cp->sense_buffer, sense_len)); }/**
@@ -1005,7 +1002,6 @@ qla2x00_status_entry(scsi_qla_host_t *ha, void *pkt)if (lscsi_status !=SS_CHECK_CONDITION )break ; -memset (cp->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE );if (!(scsi_status &SS_SENSE_LEN_VALID ))break ;
@@ -1064,7 +1060,6 @@ qla2x00_status_entry(scsi_qla_host_t *ha, void *pkt)if (lscsi_status !=SS_CHECK_CONDITION )break ; -memset (cp->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE );if (!(scsi_status &SS_SENSE_LEN_VALID ))break ;
@@ -1268,6 +1263,8 @@ qla2x00_status_cont_entry(scsi_qla_host_t *ha, sts_cont_entry_t *pkt)/* Place command on done queue. */ if (sp->request_sense_length ==0 ) { +scsi_eh_cpy_sense (cp, sp->sense_buffer, + sp->request_sense_ptr - sp->sense_buffer); ha->status_srb =NULL ;qla2x00_sp_compl (ha, sp); }
Signature · 1 line
1.5.3.3
RFC PATCH 8/24 isd200, transport: Use scsi_eh_cpy_sense API
In reply to Boaz Harrosh Use of new scsi_eh_cpy_sense() to set sense information into the command.
--- drivers/usb/storage/isd200.c | 14 +++++++------- drivers/usb/storage/transport.c | 26 +++++++++++++------------- 2 files changed, 20 insertions(+), 20 deletions(-)
drivers/usb/storage/isd200.c +7 -7
@@ -53,6 +53,7 @@#include <scsi/scsi.h> #include <scsi/scsi_cmnd.h> #include <scsi/scsi_device.h> +#include <scsi/scsi_eh.h> #include "usb.h" #include "transport.h"
@@ -365,8 +366,9 @@ struct sense_data {static void isd200_build_sense (struct us_data *us,struct scsi_cmnd *srb) {struct isd200_info *info = (struct isd200_info *)us->extra; -struct sense_data *buf = (struct sense_data *) &srb->sense_buffer[0 ]; -unsigned char error = info->ATARegs[ATA_REG_ERROR_OFFSET ]; +struct sense_data sense; +struct sense_data *buf = &sense; +unsigned char error = info->ATARegs[IDE_ERROR_OFFSET ];if (error &ATA_ERROR_MEDIA_CHANGE ) { buf->ErrorCode =0x70 |SENSE_ERRCODE_VALID ;
@@ -399,6 +401,9 @@ static void isd200_build_sense(struct us_data *us, struct scsi_cmnd *srb) buf->AdditionalSenseCode =0 ; buf->AdditionalSenseCodeQualifier =0 ; } +scsi_eh_cpy_sense (srb, &sense,sizeof (sense)); + + srb->result = buf->Flags ?SAM_STAT_CHECK_CONDITION :SAM_STAT_GOOD ; }
@@ -639,11 +644,6 @@ static void isd200_invoke_transport( struct us_data *us, }if (result ==ISD200_GOOD ) {isd200_build_sense (us, srb); - srb->result =SAM_STAT_CHECK_CONDITION ; - -/* If things are really okay, then let's show that */ -if ((srb->sense_buffer[2 ] &0xf ) ==0x0 ) - srb->result =SAM_STAT_GOOD ; }else { srb->result =DID_ERROR <<16 ;/* Need reset here */
drivers/usb/storage/transport.c +13 -13
@@ -635,15 +635,15 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)US_DEBUGP ("-- Result from auto-sense is %d\n" , temp_result);US_DEBUGP ("-- code: 0x%x, key: 0x%x, ASC: 0x%x, ASCQ: 0x%x\n" , - srb->sense_buffer[0 ], - srb->sense_buffer[2 ] &0xf , - srb->sense_buffer[12 ], - srb->sense_buffer[13 ]); +scsi_sense (srb)[0 ], +scsi_sense (srb)[2 ] &0xf , +scsi_sense (srb)[12 ], +scsi_sense (srb)[13 ]);#ifdef CONFIG_USB_STORAGE_DEBUG usb_stor_show_sense ( - srb->sense_buffer[2 ] &0xf , - srb->sense_buffer[12 ], - srb->sense_buffer[13 ]); +scsi_sense (srb)[2 ] &0xf , +scsi_sense (srb)[12 ], +scsi_sense (srb)[13 ]);#endif /* set the result so the higher layers expect this data */
@@ -654,12 +654,12 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)* we did an unsolicited auto-sense. */ if (result ==USB_STOR_TRANSPORT_GOOD &&/* Filemark 0, ignore EOM, ILI 0, no sense */ - (srb->sense_buffer[2 ] &0xaf ) ==0 && + (scsi_sense (srb)[2 ] &0xaf ) ==0 &&/* No ASC or ASCQ */ - srb->sense_buffer[12 ] ==0 && - srb->sense_buffer[13 ] ==0 ) { +scsi_sense (srb)[12 ] ==0 && +scsi_sense (srb)[13 ] ==0 ) { srb->result =SAM_STAT_GOOD ; - srb->sense_buffer[0 ] =0x0 ; +scsi_eh_reset_sense (srb); } }
@@ -1056,8 +1056,8 @@ int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct us_data *us)case US_BULK_STAT_OK :/* device babbled -- return fake sense data */ if (fake_sense) { -memcpy (srb->sense_buffer, - usb_stor_sense_invalidCDB, +scsi_eh_cpy_sense (srb, + usb_stor_sense_invalidCDB,sizeof (usb_stor_sense_invalidCDB));return USB_STOR_TRANSPORT_NO_SENSE ; }
Signature · 1 line
1.5.3.3
RFC PATCH 9/24 ultrastor: Use scsi_eh_cpy_sense() API
In reply to Boaz Harrosh - allocate a driver private area sense buffer, and set IO to transfer sense into that buffer.
- at end of command execution copy the sense information using scsi_eh_cpy_sense().
--- drivers/scsi/ultrastor.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
drivers/scsi/ultrastor.c +9 -5
@@ -199,6 +199,7 @@ struct mscp {void (*done) (struct scsi_cmnd *);struct scsi_cmnd *SCint;ultrastor_sg_list sglist[ULTRASTOR_24F_MAX_SG ];/* use larger size for 24F */ +u8 sense_buffer[SCSI_SENSE_BUFFERSIZE ]; };
@@ -743,19 +744,20 @@ static int ultrastor_queuecommand(struct scsi_cmnd *SCpnt, }else {/* Unset scatter/gather flag in SCSI command packet */ my_mscp->sg = FALSE; - my_mscp->transfer_data =isa_virt_to_bus (scsi_sglist (SCpnt)); - my_mscp->transfer_data_length =scsi_bufflen (SCpnt); + my_mscp->transfer_data =0 ; + my_mscp->transfer_data_length =0 ; } my_mscp->command_link =0 ;/*???*/ my_mscp->scsi_command_link_id =0 ;/*???*/ - my_mscp->length_of_sense_byte =SCSI_SENSE_BUFFERSIZE ; my_mscp->length_of_scsi_cdbs = SCpnt->cmd_len;memcpy (my_mscp->scsi_cdbs, SCpnt->cmnd, my_mscp->length_of_scsi_cdbs); my_mscp->adapter_status =0 ; my_mscp->target_status =0 ; - my_mscp->sense_data =isa_virt_to_bus (&SCpnt->sense_buffer); my_mscp->done = done; my_mscp->SCint = SCpnt; +memset (my_mscp->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); + my_mscp->length_of_sense_byte =SCSI_SENSE_BUFFERSIZE ; + my_mscp->sense_data =isa_virt_to_bus (my_mscp->sense_buffer); SCpnt->host_scribble = (unsigned char *)my_mscp;/* Find free OGM slot. On 24F, look for OGM status byte == 0.
@@ -1140,7 +1142,9 @@ static void ultrastor_interrupt(void *dev_id) status =DID_TIME_OUT <<16 ;break ; } - +if (mscp->sense_buffer[0 ]) +scsi_eh_cpy_sense (SCtmp, mscp->sense_buffer, +sizeof (mscp->sense_buffer)); SCtmp->result = status | mscp->target_status; SCtmp->host_scribble =NULL ;
Signature · 1 line
1.5.3.3
RFC PATCH 10/24 usb/microtek: No special handling for REQUEST_SENSE command please
In reply to Boaz Harrosh - Request sense command now comes like any other command with sg-list and regular dma mapping. So just remove that special handling. No thanks!
- Some left-over cleanup from the accessors and not use_sg patch
--- drivers/usb/image/microtek.c | 20 +++++--------------- 1 files changed, 5 insertions(+), 15 deletions(-)
drivers/usb/image/microtek.c +5 -15
@@ -480,23 +480,13 @@ static void mts_command_done( struct urb *transfer )return ; } -if (context->srb->cmnd[0 ] ==REQUEST_SENSE ) { -mts_int_submit_urb (transfer, - context->data_pipe, - context->srb->sense_buffer, +if (context->data) +mts_int_submit_urb (transfer, context->data_pipe, context->data, context->data_length, - mts_data_done); - }else {if ( context->data ) { -mts_int_submit_urb (transfer, - context->data_pipe, - context->data, - context->data_length, -scsi_sg_count (context->srb) >1 ? - mts_do_sg : mts_data_done); - }else { +scsi_sg_count (context->srb) >1 ? + mts_do_sg : mts_data_done); +else mts_get_status (transfer); - } - }return ; }
Signature · 1 line
1.5.3.3
RFC PATCH 11/24 libata: Use scsi_eh API for REQUEST_SENSE invocation
In reply to Boaz Harrosh both libata-scsi and libata-eh used a cooked up REQUEST_SENSE command to retrieve sense data. Use of the scsi_eh_{prep,restore}_cmnd() can facilitate and simplify the code. And insulates code from scsi future changes.
Am I right in assuming that ata_exec_internal_sg() executes synchronously (called from atapi_eh_request_sense()) and once returned contain valid sense data?
Also other places in libata where converted to new scsi_eh API and accessors.
Set shost->sense_buffsize in ata_scsi_add_hosts() for all drivers.
--- drivers/ata/libata-eh.c | 29 +++++++++++------------------ drivers/ata/libata-scsi.c | 44 ++++++++++++++++++++++---------------------- include/linux/libata.h | 3 +++ 3 files changed, 36 insertions(+), 40 deletions(-)
drivers/ata/libata-eh.c +11 -18
@@ -1270,29 +1270,19 @@ static int ata_eh_read_log_10h(struct ata_device *dev,static unsigned int atapi_eh_request_sense (struct ata_queued_cmd *qc) {struct ata_device *dev = qc->dev; -unsigned char *sense_buf = qc->scsicmd->sense_buffer;struct ata_port *ap = dev->link->ap;struct ata_taskfile tf; -u8 cdb[ATAPI_CDB_LEN ]; +struct scsi_eh_save ses; +struct scsi_cmnd *cmd = qc->scsicmd; +int ret;DPRINTK ("ATAPI request sense\n" ); -/* FIXME: is this needed? */ -memset (sense_buf,0 ,SCSI_SENSE_BUFFERSIZE ); - -/* initialize sense_buf with the error register, -* for the case where they are -not- overwritten -*/ - sense_buf[0 ] =0x70 ; - sense_buf[2 ] = qc->result_tf.feature >>4 ; - +/*?? Is there a maximum size here that ATAPI will confuse if more ??*/ +scsi_eh_prep_cmnd (cmd, &ses,NULL ,0 , ~0 );/* some devices time out if garbage left in tf */ ata_tf_init (dev, &tf); -memset (cdb,0 ,ATAPI_CDB_LEN ); - cdb[0 ] =REQUEST_SENSE ; - cdb[4 ] =SCSI_SENSE_BUFFERSIZE ; - tf.flags |=ATA_TFLAG_ISADDR |ATA_TFLAG_DEVICE ; tf.command =ATA_CMD_PACKET ;
@@ -1302,12 +1292,15 @@ static unsigned int atapi_eh_request_sense(struct ata_queued_cmd *qc) tf.feature |=ATAPI_PKT_DMA ; }else { tf.protocol =ATAPI_PROT_PIO ; - tf.lbam =SCSI_SENSE_BUFFERSIZE ; + tf.lbam =scsi_bufflen (cmd); tf.lbah =0 ; } -return ata_exec_internal (dev, &tf, cdb,DMA_FROM_DEVICE , - sense_buf,SCSI_SENSE_BUFFERSIZE ,0 ); + ret =ata_exec_internal_sg (dev, &tf, cmd->cmnd,DMA_FROM_DEVICE , +scsi_sglist (cmd),scsi_sg_count (cmd),0 ); + +scsi_eh_restore_cmnd (cmd, &ses); +return ret; }/**
drivers/ata/libata-scsi.c +22 -22
107 lines not on this page · raw message
include/linux/libata.h +3 -0
@@ -34,6 +34,7 @@#include <linux/ata.h> #include <linux/workqueue.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_eh.h> #include <linux/acpi.h> #include <linux/cdrom.h>
@@ -454,6 +455,8 @@ struct ata_queued_cmd {struct ata_taskfile tf;u8 cdb[ATAPI_CDB_LEN ]; +struct scsi_eh_save ses; +unsigned long flags;/* ATA_QCFLAG_xxx */ unsigned int tag;unsigned int n_elem;
Signature · 1 line
1.5.3.3
RFC PATCH 12/24 53c700: Use scsi_eh API for REQUEST_SENSE invocation
In reply to Boaz Harrosh - Use scsi_eh_prep/restor_cmnd() for synchronous REQUEST_SENSE invocation.
- Refactor some code that is now commonly used in 2 places.
--- drivers/scsi/53c700.c | 134 +++++++++++++++++-------------------------------- drivers/scsi/53c700.h | 20 +++----- 2 files changed, 54 insertions(+), 100 deletions(-)
drivers/scsi/53c700.c +47 -87
@@ -336,6 +336,8 @@ NCR_700_detect(struct scsi_host_template *tpnt,if (tpnt->proc_name ==NULL ) tpnt->proc_name ="53c700" ; + tpnt->sense_buffsize =SCSI_SENSE_BUFFERSIZE ; + host =scsi_host_alloc (tpnt,4 );if (!host)return NULL ;
@@ -578,6 +580,34 @@ save_for_reselection(struct NCR_700_Host_Parameters *hostdata, hostdata->cmd =NULL ; } +STATIC void +NCR_700_map (struct NCR_700_Host_Parameters *hostdata,struct scsi_cmnd *SCp, +struct NCR_700_command_slot *slot,int move_ins) +{ +int i; +int sg_count; +struct scatterlist *sg; + + sg_count =scsi_dma_map (SCp); +BUG_ON (sg_count <0 ); + +scsi_for_each_sg (SCp, sg, sg_count, i) { +dma_addr_t vPtr =sg_dma_address (sg); +__u32 count =sg_dma_len (sg); + + slot->SG[i].ins =bS_to_host (move_ins | count); +DEBUG ((" scatter block %d: move %d[%08x] from 0x%lx\n" , + i, count, slot->SG[i].ins, (unsigned long )vPtr)); + slot->SG[i].pAddr =bS_to_host (vPtr); + } + slot->SG[i].ins =bS_to_host (SCRIPT_RETURN ); + slot->SG[i].pAddr =0 ; +dma_cache_sync (hostdata->dev, slot->SG,sizeof (slot->SG),DMA_TO_DEVICE ); +DEBUG ((" SETTING %08lx to %x\n" , + (&slot->pSG[i].ins), + slot->SG[i].ins)); +} + STATIC inline voidNCR_700_unmap (struct NCR_700_Host_Parameters *hostdata,struct scsi_cmnd *SCp,struct NCR_700_command_slot *slot)
@@ -598,26 +628,18 @@ NCR_700_scsi_done(struct NCR_700_Host_Parameters *hostdata,struct NCR_700_command_slot *slot = (struct NCR_700_command_slot *)SCp->host_scribble; -dma_unmap_single (hostdata->dev, slot->pCmd, -MAX_COMMAND_SIZE ,DMA_TO_DEVICE ); +NCR_700_unmap (hostdata, SCp, slot);if (slot->flags ==NCR_700_FLAG_AUTOSENSE ) { -char *cmnd =NCR_700_get_sense_cmnd (SCp->device); +struct NCR_700_Device_Parameters *ndp = +NCR_700_Device_Parameters (SCp->device);#ifdef NCR_700_DEBUG printk (" ORIGINAL CMD %p RETURNED %d, new return is %d sense is\n" , SCp, SCp->cmnd[7 ], result);scsi_print_sense ("53c700" , SCp);#endif -dma_unmap_single (hostdata->dev, slot->dma_handle, -SCSI_SENSE_BUFFERSIZE ,DMA_FROM_DEVICE ); -/* restore the old result if the request sense was -* successful */ -if (result ==0 ) - result = cmnd[7 ]; -/* restore the original length */ - SCp->cmd_len = cmnd[8 ]; - }else -NCR_700_unmap (hostdata, SCp, slot); +scsi_eh_restore_cmnd (SCp, &ndp->ses); + }free_slot (slot, hostdata);#ifdef NCR_700_DEBUG
@@ -988,8 +1010,8 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp, "broken device is looping in contingent allegiance: ignoring\n");NCR_700_scsi_done (hostdata, SCp, hostdata->status[0 ]); }else { -char *cmnd = -NCR_700_get_sense_cmnd (SCp->device); +struct NCR_700_Device_Parameters *ndp = +NCR_700_Device_Parameters (SCp->device);#ifdef NCR_DEBUG scsi_print_command (SCp); printk(" cmd %p has status %d, requesting sense\n",
@@ -1007,32 +1029,14 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp,MAX_COMMAND_SIZE , DMA_TO_DEVICE); - cmnd[0 ] =REQUEST_SENSE ; - cmnd[1 ] = (SCp->device->lun &0x7 ) <<5 ; - cmnd[2 ] =0 ; - cmnd[3 ] =0 ; - cmnd[4 ] =SCSI_SENSE_BUFFERSIZE ; - cmnd[5 ] =0 ; -/* Here's a quiet hack: the -* REQUEST_SENSE command is six bytes, -* so store a flag indicating that -* this was an internal sense request -* and the original status at the end -* of the command */ - cmnd[6 ] =NCR_700_INTERNAL_SENSE_MAGIC ; - cmnd[7 ] = hostdata->status[0 ]; - cmnd[8 ] = SCp->cmd_len; - SCp->cmd_len =6 ;/* command length for -* REQUEST_SENSE */ - slot->pCmd =dma_map_single (hostdata->dev, cmnd,MAX_COMMAND_SIZE ,DMA_TO_DEVICE ); - slot->dma_handle =dma_map_single (hostdata->dev, SCp->sense_buffer,SCSI_SENSE_BUFFERSIZE ,DMA_FROM_DEVICE ); - slot->SG[0 ].ins =bS_to_host (SCRIPT_MOVE_DATA_IN |SCSI_SENSE_BUFFERSIZE ); - slot->SG[0 ].pAddr =bS_to_host (slot->dma_handle); - slot->SG[1 ].ins =bS_to_host (SCRIPT_RETURN ); - slot->SG[1 ].pAddr =0 ; +scsi_eh_prep_cmnd (SCp, &ndp->ses,NULL ,0 , ~0 ); +NCR_700_map (hostdata, SCp, slot, +SCRIPT_MOVE_DATA_IN ); + slot->pCmd =dma_map_single ( + hostdata->dev, SCp->cmnd, +MAX_COMMAND_SIZE ,DMA_TO_DEVICE ); + slot->resume_offset = hostdata->pScript; -dma_cache_sync (hostdata->dev, slot->SG,sizeof (slot->SG[0 ])*2 ,DMA_TO_DEVICE ); -dma_cache_sync (hostdata->dev, SCp->sense_buffer,SCSI_SENSE_BUFFERSIZE ,DMA_FROM_DEVICE );/* queue the command for reissue */ slot->state =NCR_700_SLOT_QUEUED ;
@@ -1041,27 +1045,6 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp, hostdata->cmd =NULL ; } }else { -// Currently rely on the mid layer evaluation -// of the tag queuing capability -// -//if(status_byte(hostdata->status[0]) == GOOD && -// SCp->cmnd[0] == INQUIRY && SCp->use_sg == 0) { -// /* Piggy back the tag queueing support -// * on this command */ -// dma_sync_single_for_cpu(hostdata->dev, -// slot->dma_handle, -// SCp->request_bufflen, -// DMA_FROM_DEVICE); -// if(((char *)SCp->request_buffer)[7] & 0x02) { -// scmd_printk(KERN_INFO, SCp, -// "Enabling Tag Command Queuing\n"); -// hostdata->tag_negotiated |= (1<<scmd_id(SCp)); -// NCR_700_set_flag(SCp->device, NCR_700_DEV_BEGIN_TAG_QUEUEING); -// } else { -// NCR_700_clear_flag(SCp->device, NCR_700_DEV_BEGIN_TAG_QUEUEING); -// hostdata->tag_negotiated &= ~(1<<scmd_id(SCp)); -// } -//} NCR_700_scsi_done (hostdata, SCp, hostdata->status[0 ]); } }else if ((dsps &0xfffff0f0 ) ==A_UNEXPECTED_PHASE ) {
@@ -1873,32 +1856,9 @@ NCR_700_queuecommand(struct scsi_cmnd *SCp, void (*done)(struct scsi_cmnd *))/* now build the scatter gather list */ direction = SCp->sc_data_direction; -if (move_ins !=0 ) { -int i; -int sg_count; -dma_addr_t vPtr =0 ; -struct scatterlist *sg; -__u32 count =0 ; - - sg_count =scsi_dma_map (SCp); -BUG_ON (sg_count <0 ); - -scsi_for_each_sg (SCp, sg, sg_count, i) { - vPtr =sg_dma_address (sg); - count =sg_dma_len (sg); - - slot->SG[i].ins =bS_to_host (move_ins | count); -DEBUG ((" scatter block %d: move %d[%08x] from 0x%lx\n" , - i, count, slot->SG[i].ins, (unsigned long )vPtr)); - slot->SG[i].pAddr =bS_to_host (vPtr); - } - slot->SG[i].ins =bS_to_host (SCRIPT_RETURN ); - slot->SG[i].pAddr =0 ; -dma_cache_sync (hostdata->dev, slot->SG,sizeof (slot->SG),DMA_TO_DEVICE ); -DEBUG ((" SETTING %08lx to %x\n" , - (&slot->pSG[i].ins), - slot->SG[i].ins)); - } +if (move_ins !=0 ) +NCR_700_map (hostdata, SCp, slot, move_ins); + slot->resume_offset =0 ; slot->pCmd =dma_map_single (hostdata->dev, SCp->cmnd,MAX_COMMAND_SIZE ,DMA_TO_DEVICE );
drivers/scsi/53c700.h +7 -13
@@ -13,6 +13,7 @@#include <scsi/scsi_device.h> #include <scsi/scsi_cmnd.h> +#include <scsi/scsi_eh.h> /* Turn on for general debugging---too verbose for normal use */ #undef NCR_700_DEBUG
@@ -77,14 +78,10 @@ struct NCR_700_SG_List { };struct NCR_700_Device_Parameters { -/* space for creating a request sense command. Really, except -* for the annoying SCSI-2 requirement for LUN information in -* cmnd[1], this could be in static storage */ -unsigned char cmnd[MAX_COMMAND_SIZE ]; +struct scsi_eh_save ses;__u8 depth; }; -/* The SYNC negotiation sequence looks like: * * If DEV_NEGOTIATED_SYNC not set, tack and SDTR message on to the
@@ -103,26 +100,23 @@ struct NCR_700_Device_Parameters {#define NCR_700_DEV_BEGIN_SYNC_NEGOTIATION (1<<17)#define NCR_700_DEV_PRINT_SYNC_NEGOTIATION (1<<19) -static inline char *NCR_700_get_sense_cmnd (struct scsi_device *SDp) +static inline struct NCR_700_Device_Parameters * +NCR_700_Device_Parameters (struct scsi_device *SDp) {struct NCR_700_Device_Parameters *hostdata = SDp->hostdata; -return hostdata->cmnd; +return hostdata; }static inline void NCR_700_set_depth (struct scsi_device *SDp,__u8 depth) { -struct NCR_700_Device_Parameters *hostdata = SDp->hostdata; - - hostdata->depth = depth; +NCR_700_Device_Parameters (SDp)->depth = depth; }static inline __u8 NCR_700_get_depth (struct scsi_device *SDp) { -struct NCR_700_Device_Parameters *hostdata = SDp->hostdata; - -return hostdata->depth; +return NCR_700_Device_Parameters (SDp)->depth; }static inline int NCR_700_is_flag_set (struct scsi_device *SDp,__u32 flag)
Signature · 1 line
1.5.3.3
RFC PATCH 13/24 aic7xxx_old: Use scsi_eh API for REQUEST_SENSE invocation
In reply to Boaz Harrosh - Use scsi_eh_prep/restore_cmnd() for synchronous REQUEST_SENSE invocation.
- Use new sense accessors where needed.
--- drivers/scsi/aic7xxx_old.c | 74 ++++++++++++------------------------------- 1 files changed, 21 insertions(+), 53 deletions(-)
drivers/scsi/aic7xxx_old.c +21 -53
@@ -786,10 +786,7 @@ struct aic7xxx_scb {struct hw_scatterlist *sg_list;/* SG list in adapter format */ unsigned char tag_action;unsigned char sg_count; -unsigned char *sense_cmd;/* -* Allocate 6 characters for -* sense command. -*/ +struct scsi_eh_save ses;unsigned char *cmnd;unsigned int sg_length;/* * We init this during
@@ -823,9 +820,6 @@ static struct { { CIOPARERR, "CIOBUS Parity Error" } }; -static unsigned char -generic_sense[] = {REQUEST_SENSE ,0 ,0 ,0 ,255 ,0 }; - typedef struct { scb_queue_type free_scbs; /* * SCBs assigned to free slot on
@@ -1277,6 +1271,8 @@ static void aic7xxx_print_sequencer(struct aic7xxx_host *p, int downloaded);#ifdef AIC7XXX_VERBOSE_DEBUGGING static void aic7xxx_check_scbs (struct aic7xxx_host *p,char *buffer);#endif +static void aic7xxx_buildscb (struct aic7xxx_host *p,struct scsi_cmnd *cmd, +struct aic7xxx_scb *scb);/**************************************************************************** *
@@ -2528,7 +2524,7 @@ static int aic7xxx_allocate_scb(struct aic7xxx_host *p) {struct aic7xxx_scb *scbp =NULL ; -int scb_size = (sizeof (struct hw_scatterlist ) *AIC7XXX_MAX_SG ) +12 +6 ; +int scb_size = (sizeof (struct hw_scatterlist ) *AIC7XXX_MAX_SG ) +12 ;int i;int step =PAGE_SIZE /1024 ;unsigned long scb_count =0 ;
@@ -2598,9 +2594,8 @@ aic7xxx_allocate_scb(struct aic7xxx_host *p) scbp = &scb_ap[i]; scbp->hscb = &p->scb_data->hscbs[p->scb_data->numscbs]; scbp->sg_list = &hsgp[i *AIC7XXX_MAX_SG ]; - scbp->sense_cmd = bufs; - scbp->cmnd = bufs +6 ; - bufs +=12 +6 ; + scbp->cmnd = bufs; + bufs +=12 ; scbp->scb_dma = scb_dma;memset (scbp->hscb,0 ,sizeof (struct aic7xxx_hwscb )); scbp->hscb->tag = p->scb_data->numscbs;
@@ -2694,10 +2689,7 @@ aic7xxx_done(struct aic7xxx_host *p, struct aic7xxx_scb *scb)if (scb->flags &SCB_SENSE ) { -pci_unmap_single (p->pdev, -le32_to_cpu (scb->sg_list[0 ].address), -SCSI_SENSE_BUFFERSIZE , -PCI_DMA_FROMDEVICE ); +scsi_eh_restore_cmnd (cmd, &scb->ses); }if (scb->flags &SCB_RECOVERY_SCB ) {
@@ -2720,8 +2712,8 @@ aic7xxx_done(struct aic7xxx_host *p, struct aic7xxx_scb *scb)* after failing to negotiate a wide or sync transfer message. */ if ((scb->flags &SCB_SENSE ) && - ((scb->cmd->sense_buffer[12 ] ==0x43 ) ||/* INVALID_MESSAGE */ - (scb->cmd->sense_buffer[12 ] ==0x49 )))/* MESSAGE_ERROR */ + ((scsi_sense (scb->cmd)[12 ] ==0x43 ) ||/* INVALID_MESSAGE */ + (scsi_sense (scb->cmd)[12 ] ==0x49 )))/* MESSAGE_ERROR */ { message_error = TRUE; }
@@ -4263,18 +4255,8 @@ aic7xxx_handle_seqint(struct aic7xxx_host *p, unsigned char intstat)* Send a sense command to the requesting target. * XXX - revisit this and get rid of the memcopys. */ -memcpy (scb->sense_cmd, &generic_sense[0 ], -sizeof (generic_sense)); - - scb->sense_cmd[1 ] = (cmd->device->lun <<5 ); - scb->sense_cmd[4 ] =SCSI_SENSE_BUFFERSIZE ; - - scb->sg_list[0 ].length = -cpu_to_le32 (SCSI_SENSE_BUFFERSIZE ); - scb->sg_list[0 ].address = -cpu_to_le32 (pci_map_single (p->pdev, cmd->sense_buffer, -SCSI_SENSE_BUFFERSIZE , -PCI_DMA_FROMDEVICE )); +scsi_eh_prep_cmnd (cmd, &scb->ses,NULL ,0 , ~0 ); +aic7xxx_buildscb (p, cmd, scb);/* * XXX - We should allow disconnection, but can't as it
@@ -4283,21 +4265,6 @@ aic7xxx_handle_seqint(struct aic7xxx_host *p, unsigned char intstat)/* hscb->control &= DISCENB; */ hscb->control =0 ; hscb->target_status =0 ; - hscb->SG_list_pointer = -cpu_to_le32 (SCB_DMA_ADDR (scb, scb->sg_list)); - hscb->SCSI_cmd_pointer = -cpu_to_le32 (SCB_DMA_ADDR (scb, scb->sense_cmd)); - hscb->data_count = scb->sg_list[0 ].length; - hscb->data_pointer = scb->sg_list[0 ].address; - hscb->SCSI_cmd_length =COMMAND_SIZE (scb->sense_cmd[0 ]); - hscb->residual_SG_segment_count =0 ; - hscb->residual_data_count[0 ] =0 ; - hscb->residual_data_count[1 ] =0 ; - hscb->residual_data_count[2 ] =0 ; - - scb->sg_count = hscb->SG_segment_count =1 ; - scb->sg_length =SCSI_SENSE_BUFFERSIZE ; - scb->tag_action =0 ; scb->flags |=SCB_SENSE ;/* * Ensure the target is busy since this will be an
@@ -6303,9 +6270,16 @@ aic7xxx_handle_command_completion_intr(struct aic7xxx_host *p)*/ scb->flags &= ~(SCB_ABORT |SCB_RESET ); } -else if (scb->flags &SCB_SENSE ) + cmd = scb->cmd; +if (scb->hscb->residual_SG_segment_count !=0 ) + { +aic7xxx_calculate_residual (p, scb); + } + cmd->result |= (aic7xxx_error (cmd) <<16 ); +aic7xxx_done (p, scb); +if (scb->flags &SCB_SENSE ) { -char *buffer = &scb->cmd->sense_buffer[0 ]; +const u8 *buffer =scsi_sense (cmd);if (buffer[12 ] ==0x47 || buffer[12 ] ==0x54 ) {
@@ -6317,13 +6291,6 @@ aic7xxx_handle_command_completion_intr(struct aic7xxx_host *p) aic_dev->needwdtr = aic_dev->needwdtr_copy; } } - cmd = scb->cmd; -if (scb->hscb->residual_SG_segment_count !=0 ) - { -aic7xxx_calculate_residual (p, scb); - } - cmd->result |= (aic7xxx_error (cmd) <<16 ); -aic7xxx_done (p, scb); } }
@@ -11141,6 +11108,7 @@ static struct scsi_host_template driver_template = { .max_sectors = 2048, .cmd_per_lun = 3, .use_clustering = ENABLE_CLUSTERING, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };#include "scsi_module.c"
Signature · 1 line
1.5.3.3
RFC PATCH 14/24 ] dc395x: Use scsi_eh API for REQUEST_SENSE invocation
In reply to Boaz Harrosh - Using scsi_eh_{prep,restore}_cmnd() for synchronous REQUEST_SENSE invocation. simplifies code alot, because it can now use the regular command invocation code path.
- Use new sense accessors where needed.
- use scsi_print_sense() (that is there for ages) in place of a driver's made one. (Is that needed still)
--- drivers/scsi/dc395x.c | 140 ++++++++---------------------------------------- 1 files changed, 24 insertions(+), 116 deletions(-)
drivers/scsi/dc395x.c +24 -116
@@ -64,6 +64,8 @@#include <scsi/scsi_cmnd.h> #include <scsi/scsi_device.h> #include <scsi/scsi_host.h> +#include <scsi/scsi_eh.h> +#include <scsi/scsi_dbg.h> #include "dc395x.h"
@@ -236,16 +238,8 @@ struct ScsiReqBlk {u8 sg_index;/* Index of HW sg entry for this request */ size_t total_xfer_length;/* Total number of bytes remaining to be transfered */ size_t request_length;/* Total number of bytes in this request */ -/* -* The sense buffer handling function, request_sense, uses -* the first hw sg entry (segment_x[0]) and the transfer -* length (total_xfer_length). While doing this it stores the -* original values into the last sg hw list -* (srb->segment_x[DC395x_MAX_SG_LISTENTRY - 1] and the -* total_xfer_length in xferred. These values are restored in -* pci_unmap_srb_sense. This is the only place xferred is used. -*/ -size_t xferred;/* Saved copy of total_xfer_length */ + +struct scsi_eh_save ses;u16 state;
@@ -1624,18 +1618,11 @@ static u8 start_scsi(struct AdapterCtlBlk* acb, struct DeviceCtlBlk* dcb,dprintkdbg (DBG_KG ,"start_scsi: (pid#%li) <%02i-%i> cmnd=0x%02x tag=%i\n" , srb->cmd->serial_number, srb->cmd->device->id, srb->cmd->device->lun, srb->cmd->cmnd[0 ], srb->tag_number); -if (srb->flag &AUTO_REQSENSE ) { -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,REQUEST_SENSE ); -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO , (dcb->target_lun <<5 )); -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,0 ); -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,0 ); -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,SCSI_SENSE_BUFFERSIZE ); -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,0 ); - }else { - ptr = (u8 *)srb->cmd->cmnd; -for (i =0 ; i < srb->cmd->cmd_len; i++) -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO , *ptr++); - } + ptr = (u8 *)srb->cmd->cmnd; + +for (i =0 ; i < srb->cmd->cmd_len; i++) +DC395x_write8 (acb,TRM_S1040_SCSI_FIFO , *ptr++); + no_cmd:DC395x_write16 (acb,TRM_S1040_SCSI_CONTROL ,DO_HWRESELECT |DO_DATALATCH );
@@ -1894,29 +1881,19 @@ static void command_phase0(struct AdapterCtlBlk *acb, struct ScsiReqBlk *srb,static void command_phase1 (struct AdapterCtlBlk *acb,struct ScsiReqBlk *srb,u16 *pscsi_status) { -struct DeviceCtlBlk *dcb;u8 *ptr;u16 i;dprintkdbg (DBG_0 ,"command_phase1: (pid#%li)\n" , srb->cmd->serial_number);clear_fifo (acb,"command_phase1" );DC395x_write16 (acb,TRM_S1040_SCSI_CONTROL ,DO_CLRATN ); -if (!(srb->flag &AUTO_REQSENSE )) { - ptr = (u8 *)srb->cmd->cmnd; -for (i =0 ; i < srb->cmd->cmd_len; i++) { -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO , *ptr); - ptr++; - } - }else { -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,REQUEST_SENSE ); - dcb = acb->active_dcb; -/* target id */ -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO , (dcb->target_lun <<5 )); -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,0 ); -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,0 ); -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,SCSI_SENSE_BUFFERSIZE ); -DC395x_write8 (acb,TRM_S1040_SCSI_FIFO ,0 ); + + ptr = (u8 *)srb->cmd->cmnd; +for (i =0 ; i < srb->cmd->cmd_len; i++) { +DC395x_write8 (acb,TRM_S1040_SCSI_FIFO , *ptr); + ptr++; } + srb->state |=SRB_COMMAND ;/* it's important for atn stop */ DC395x_write16 (acb,TRM_S1040_SCSI_CONTROL ,DO_DATALATCH );
@@ -3290,17 +3267,9 @@ static void pci_unmap_srb_sense(struct AdapterCtlBlk *acb, {if (!(srb->flag &AUTO_REQSENSE ))return ; -/* Unmap sense buffer */ -dprintkdbg (DBG_SG ,"pci_unmap_srb_sense: buffer=%08x\n" , - srb->segment_x[0 ].address); -pci_unmap_single (acb->dev, srb->segment_x[0 ].address, - srb->segment_x[0 ].length,PCI_DMA_FROMDEVICE ); -/* Restore SG stuff */ - srb->total_xfer_length = srb->xferred; - srb->segment_x[0 ].address = - srb->segment_x[DC395x_MAX_SG_LISTENTRY -1 ].address; - srb->segment_x[0 ].length = - srb->segment_x[DC395x_MAX_SG_LISTENTRY -1 ].length; + +pci_unmap_srb (acb, srb); +scsi_eh_restore_cmnd (srb->cmd, &srb->ses); }
@@ -3331,51 +3300,8 @@ static void srb_done(struct AdapterCtlBlk *acb, struct DeviceCtlBlk *dcb, srb->flag &= ~AUTO_REQSENSE ; srb->adapter_status =0 ; srb->target_status =CHECK_CONDITION <<1 ; -if (debug_enabled (DBG_1 )) { -switch (cmd->sense_buffer[2 ] &0x0f ) { -case NOT_READY : -dprintkl (KERN_DEBUG , -"ReqSense: NOT_READY cmnd=0x%02x <%02i-%i> stat=%i scan=%i " , - cmd->cmnd[0 ], dcb->target_id, - dcb->target_lun, status, acb->scan_devices); -break ; -case UNIT_ATTENTION : -dprintkl (KERN_DEBUG , -"ReqSense: UNIT_ATTENTION cmnd=0x%02x <%02i-%i> stat=%i scan=%i " , - cmd->cmnd[0 ], dcb->target_id, - dcb->target_lun, status, acb->scan_devices); -break ; -case ILLEGAL_REQUEST : -dprintkl (KERN_DEBUG , -"ReqSense: ILLEGAL_REQUEST cmnd=0x%02x <%02i-%i> stat=%i scan=%i " , - cmd->cmnd[0 ], dcb->target_id, - dcb->target_lun, status, acb->scan_devices); -break ; -case MEDIUM_ERROR : -dprintkl (KERN_DEBUG , -"ReqSense: MEDIUM_ERROR cmnd=0x%02x <%02i-%i> stat=%i scan=%i " , - cmd->cmnd[0 ], dcb->target_id, - dcb->target_lun, status, acb->scan_devices); -break ; -case HARDWARE_ERROR : -dprintkl (KERN_DEBUG , -"ReqSense: HARDWARE_ERROR cmnd=0x%02x <%02i-%i> stat=%i scan=%i " , - cmd->cmnd[0 ], dcb->target_id, - dcb->target_lun, status, acb->scan_devices); -break ; - } -if (cmd->sense_buffer[7 ] >=6 ) -printk ("sense=0x%02x ASC=0x%02x ASCQ=0x%02x " -"(0x%08x 0x%08x)\n" , - cmd->sense_buffer[2 ], cmd->sense_buffer[12 ], - cmd->sense_buffer[13 ], - *((unsigned int *)(cmd->sense_buffer +3 )), - *((unsigned int *)(cmd->sense_buffer +8 ))); -else -printk ("sense=0x%02x No ASC/ASCQ (0x%08x)\n" , - cmd->sense_buffer[2 ], - *((unsigned int *)(cmd->sense_buffer +3 ))); - } +if (debug_enabled (DBG_1 )) +scsi_print_sense ("dc395x" , cmd);if (status == (CHECK_CONDITION <<1 )) { cmd->result =DID_BAD_TARGET <<16 ;
@@ -3678,32 +3604,13 @@ static void request_sense(struct AdapterCtlBlk *acb, struct DeviceCtlBlk *dcb,dprintkdbg (DBG_1 ,"request_sense: (pid#%li) <%02i-%i>\n" , cmd->serial_number, cmd->device->id, cmd->device->lun); +scsi_eh_prep_cmnd (cmd, &srb->ses,NULL ,0 , ~0 ); +build_srb (cmd, dcb, srb); + srb->flag |=AUTO_REQSENSE ; srb->adapter_status =0 ; srb->target_status =0 ; -/* KG: Can this prevent crap sense data ? */ -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); - -/* Save some data */ - srb->segment_x[DC395x_MAX_SG_LISTENTRY -1 ].address = - srb->segment_x[0 ].address; - srb->segment_x[DC395x_MAX_SG_LISTENTRY -1 ].length = - srb->segment_x[0 ].length; - srb->xferred = srb->total_xfer_length; -/* srb->segment_x : a one entry of S/G list table */ - srb->total_xfer_length =SCSI_SENSE_BUFFERSIZE ; - srb->segment_x[0 ].length =SCSI_SENSE_BUFFERSIZE ; -/* Map sense buffer */ - srb->segment_x[0 ].address = -pci_map_single (acb->dev, cmd->sense_buffer, -SCSI_SENSE_BUFFERSIZE ,PCI_DMA_FROMDEVICE ); -dprintkdbg (DBG_SG ,"request_sense: map buffer %p->%08x(%05x)\n" , - cmd->sense_buffer, srb->segment_x[0 ].address, -SCSI_SENSE_BUFFERSIZE ); - srb->sg_count =1 ; - srb->sg_index =0 ; -if (start_scsi (acb, dcb, srb)) {/* Should only happen, if sb. else grabs the bus */ dprintkl(KERN_DEBUG, "request_sense: (pid#%li) failed <%02i-%i>\n",
@@ -4763,6 +4670,7 @@ static struct scsi_host_template dc395x_driver_template = { .eh_bus_reset_handler = dc395x_eh_bus_reset, .unchecked_isa_dma = 0, .use_clustering = DISABLE_CLUSTERING, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };
Signature · 1 line
1.5.3.3
RFC PATCH 15/24 tmscsim: Use scsi_eh API for REQUEST_SENSE invocation
In reply to Boaz Harrosh - Use scsi_eh_prep/restore_cmnd() for synchronous REQUEST_SENSE invocation.
- Use new sense accessors where needed.
- Cleanup of no longer used bits
--- drivers/scsi/tmscsim.c | 72 ++++++----------------------------------------- drivers/scsi/tmscsim.h | 11 +------ 2 files changed, 12 insertions(+), 71 deletions(-)
drivers/scsi/tmscsim.c +10 -62
@@ -242,7 +242,6 @@#include <scsi/scsicam.h> #include <scsi/scsi_tcq.h> -#define DC390_BANNER "Tekram DC390/AM53C974"#define DC390_VERSION "2.1d 2004-05-27"
@@ -428,33 +427,13 @@ static __inline__ void dc390_Going_remove (struct dc390_dcb* pDCB, struct dc390_ pDCB->GoingSRBCnt--; } -static struct scatterlist *dc390_sg_build_single (struct scatterlist *sg,void *addr,unsigned int length) -{ -sg_init_one (sg, addr, length); -return sg; -} -/* Create pci mapping */ static int dc390_pci_map (struct dc390_srb * pSRB) {int error =0 ;struct scsi_cmnd *pcmd = pSRB->pcmd; -struct pci_dev *pdev = pSRB->pSRBDCB->pDCBACB->pdev; -dc390_cmd_scp_t * cmdp = ((dc390_cmd_scp_t *)(&pcmd->SCp)); - -/* Map sense buffer */ -if (pSRB->SRBFlag &AUTO_REQSENSE ) { - pSRB->pSegmentList =dc390_sg_build_single (&pSRB->Segmentx, pcmd->sense_buffer,SCSI_SENSE_BUFFERSIZE ); - pSRB->SGcount =pci_map_sg (pdev, pSRB->pSegmentList,1 , -DMA_FROM_DEVICE ); - cmdp->saved_dma_handle =sg_dma_address (pSRB->pSegmentList); -/* TODO: error handling */ -if (pSRB->SGcount !=1 ) - error =1 ; -DEBUG1 (printk ("%s(): Mapped sense buffer %p at %x\n" , __FUNCTION__, pcmd->sense_buffer, cmdp->saved_dma_handle)); -/* Map SG list */ - }else if (scsi_sg_count (pcmd)) { +if (scsi_sg_count (pcmd)) {int nseg; nseg =scsi_dma_map (pcmd);
@@ -478,17 +457,10 @@ static int dc390_pci_map (struct dc390_srb* pSRB)static void dc390_pci_unmap (struct dc390_srb * pSRB) {struct scsi_cmnd *pcmd = pSRB->pcmd; -struct pci_dev *pdev = pSRB->pSRBDCB->pDCBACB->pdev; -DEBUG1 (dc390_cmd_scp_t* cmdp = ((dc390_cmd_scp_t *)(&pcmd->SCp))); -if (pSRB->SRBFlag) { -pci_unmap_sg (pdev, &pSRB->Segmentx,1 ,DMA_FROM_DEVICE ); -DEBUG1 (printk ("%s(): Unmapped sense buffer at %x\n" , __FUNCTION__, cmdp->saved_dma_handle)); - }else { -scsi_dma_unmap (pcmd); -DEBUG1 (printk ("%s(): Unmapped SG at %p with %d elements\n" , - __FUNCTION__,scsi_sglist (pcmd),scsi_sg_count (pcmd))); - } +scsi_dma_unmap (pcmd); +DEBUG1 (printk ("%s(): Unmapped SG at %p with %d elements\n" , + __FUNCTION__,scsi_sglist (pcmd),scsi_sg_count (pcmd))); } static void __inline__
@@ -593,23 +565,10 @@ dc390_StartSCSI( struct dc390_acb* pACB, struct dc390_dcb* pDCB, struct dc390_sr/* Command is written in CommandPhase, if SEL_W_ATN_STOP ... */ if (cmd !=SEL_W_ATN_STOP ) { -if ( pSRB->SRBFlag &AUTO_REQSENSE ) - { -DC390_write8 (ScsiFifo,REQUEST_SENSE ); -DC390_write8 (ScsiFifo, pDCB->TargetLUN <<5 ); -DC390_write8 (ScsiFifo,0 ); -DC390_write8 (ScsiFifo,0 ); -DC390_write8 (ScsiFifo,SCSI_SENSE_BUFFERSIZE ); -DC390_write8 (ScsiFifo,0 ); -DEBUG1 (printk (KERN_DEBUG "DC390: AutoReqSense !\n" )); - } -else /* write cmnd to bus */ - {u8 *ptr;u8 i; ptr = (u8 *)scmd->cmnd;for (i =0 ; i < scmd->cmd_len; i++)DC390_write8 (ScsiFifo, *(ptr++)); - } } DEBUG0(if (pACB->pActiveDCB) \ printk (KERN_WARNING "DC390: ActiveDCB != 0\n"));
@@ -1369,30 +1328,17 @@ dc390_DataInPhase( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus)static void dc390_CommandPhase (struct dc390_acb * pACB,struct dc390_srb * pSRB,u8 *psstatus) { -struct dc390_dcb * pDCB;u8 i, cnt;u8 *ptr;DC390_write8 (ScsiCmd,RESET_ATN_CMD );DC390_write8 (ScsiCmd,CLEAR_FIFO_CMD ); -if ( !(pSRB->SRBFlag &AUTO_REQSENSE ) ) - { + cnt = (u8 ) pSRB->pcmd->cmd_len; ptr = (u8 *) pSRB->pcmd->cmnd;for (i=0 ; i < cnt; i++)DC390_write8 (ScsiFifo, *(ptr++)); - } -else - { -DC390_write8 (ScsiFifo,REQUEST_SENSE ); - pDCB = pACB->pActiveDCB; -DC390_write8 (ScsiFifo, pDCB->TargetLUN <<5 ); -DC390_write8 (ScsiFifo,0 ); -DC390_write8 (ScsiFifo,0 ); -DC390_write8 (ScsiFifo,SCSI_SENSE_BUFFERSIZE ); -DC390_write8 (ScsiFifo,0 ); -DEBUG0 (printk (KERN_DEBUG "DC390: AutoReqSense (CmndPhase)!\n" )); - } + pSRB->SRBState =SRB_COMMAND ;DC390_write8 (ScsiCmd,INFO_XFER_CMD ); }
@@ -1678,16 +1624,16 @@ dc390_RequestSense(struct dc390_acb* pACB, struct dc390_dcb* pDCB, struct dc390_ pcmd->cmnd[0], pDCB->TargetID, pDCB->TargetLUN)); pSRB->SRBFlag |=AUTO_REQSENSE ; - pSRB->SavedTotXLen = pSRB->TotalXferredLen; pSRB->AdaptStatus =0 ; pSRB->TargetStatus =0 ;/* CHECK_CONDITION<<1; */ /* We are called from SRBdone, original PCI mapping has been removed * already, new one is set up from StartSCSI */ pSRB->SGIndex =0 ; - pSRB->TotalXferredLen =0 ; pSRB->SGToBeXferLen =0 ; + +scsi_eh_prep_cmnd (pcmd, &pSRB->ses,NULL ,0 , ~0 );return dc390_StartSCSI (pACB, pDCB, pSRB); }
@@ -1708,6 +1654,7 @@ dc390_SRBdone( struct dc390_acb* pACB, struct dc390_dcb* pDCB, struct dc390_srb* pSRB, pcmd->serial_number));if (pSRB->SRBFlag &AUTO_REQSENSE ) {/* Last command was a Request Sense */ +scsi_eh_restore_cmnd (pSRB->pcmd, &pSRB->ses); pSRB->SRBFlag &= ~AUTO_REQSENSE ; pSRB->AdaptStatus =0 ; pSRB->TargetStatus =SAM_STAT_CHECK_CONDITION ;
@@ -2210,6 +2157,7 @@ static struct scsi_host_template driver_template = { .cmd_per_lun = 1, .use_clustering = ENABLE_CLUSTERING, .max_sectors = 0x4000, /* 8MiB = 16 * 1024 * 512 */ + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };/***********************************************************************
drivers/scsi/tmscsim.h +2 -9
@@ -9,6 +9,7 @@#define _TMSCSIM_H#include <linux/types.h> +#include <scsi/scsi_eh.h> #define SCSI_IRQ_NONE 255
@@ -26,7 +27,7 @@*/ struct dc390_srb { -//u8 CmdBlock[12]; +struct scsi_eh_save ses;struct dc390_srb *pNextSRB;struct dc390_dcb *pSRBDCB;
@@ -40,7 +41,6 @@ unsigned long SGBusAddr; /*;a segment starting address as seen by AM53C974Aaddresses by default */ unsigned long SGToBeXferLen;/*; to be xfer length */ unsigned long TotalXferredLen; -unsigned long SavedTotXLen;unsigned long Saved_Ptr;u32 SRBState;
@@ -298,13 +298,6 @@ struct dc390_srb SRB_array[MAX_SRB_CNT]; /* 50 SRBs */#define ABORT_TAG 0x0d/* -* SISC query queue -*/ -typedef struct { -dma_addr_t saved_dma_handle; -}dc390_cmd_scp_t ; - -/* ;========================================================== ; EEPROM byte offset ;==========================================================
Signature · 1 line
1.5.3.3
RFC PATCH 16/24 Add .sense_buffsize to drivers that use scsi_eh_prep_cmnd
In reply to Boaz Harrosh With new sense handling drivers that use scsi_eh_prep_cmnd() should set .sense_buffsize to none-zero at host template.
--- drivers/scsi/aha152x.c | 1 + drivers/scsi/arm/cumana_1.c | 1 + drivers/scsi/arm/oak.c | 1 + drivers/scsi/atari_scsi.c | 1 + drivers/scsi/dmx3191d.c | 1 + drivers/scsi/dtc.c | 1 + drivers/scsi/g_NCR5380.c | 1 + drivers/scsi/mac_scsi.c | 1 + drivers/scsi/pas16.c | 1 + drivers/scsi/sun3_scsi.c | 3 ++- drivers/scsi/sun3_scsi_vme.c | 1 + drivers/scsi/t128.c | 1 + drivers/usb/storage/scsiglue.c | 3 +++ 13 files changed, 16 insertions(+), 1 deletions(-)
drivers/scsi/aha152x.c +1 -0
@@ -3477,6 +3477,7 @@ static struct scsi_host_template aha152x_driver_template = { .cmd_per_lun = 1, .use_clustering = DISABLE_CLUSTERING, .slave_alloc = aha152x_adjust_queue, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };#if !defined(PCMCIA )
drivers/scsi/arm/cumana_1.c +1 -0
@@ -225,6 +225,7 @@ static struct scsi_host_template cumanascsi_template = { .unchecked_isa_dma = 0, .use_clustering = DISABLE_CLUSTERING, .proc_name = "CumanaSCSI-1", + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, }; static int __devinit
drivers/scsi/arm/oak.c +1 -0
@@ -127,6 +127,7 @@ static struct scsi_host_template oakscsi_template = { .cmd_per_lun = 2, .use_clustering = DISABLE_CLUSTERING, .proc_name = "oakscsi", + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, }; static int __devinit
drivers/scsi/atari_scsi.c +1 -0
@@ -1140,6 +1140,7 @@ static struct scsi_host_template driver_template = { .sg_tablesize = 0, /* initialized at run-time */ .cmd_per_lun = 0, /* initialized at run-time */ .use_clustering = DISABLE_CLUSTERING + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };
drivers/scsi/dmx3191d.c +1 -0
@@ -66,6 +66,7 @@ static struct scsi_host_template dmx3191d_driver_template = { .sg_tablesize = SG_ALL, .cmd_per_lun = 2, .use_clustering = DISABLE_CLUSTERING, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, }; static int __devinit dmx3191d_probe_one(struct pci_dev *pdev,
drivers/scsi/dtc.c +1 -0
@@ -482,5 +482,6 @@ static struct scsi_host_template driver_template = { .sg_tablesize = SG_ALL, .cmd_per_lun = CMD_PER_LUN, .use_clustering = DISABLE_CLUSTERING, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };#include "scsi_module.c"
drivers/scsi/g_NCR5380.c +1 -0
@@ -925,6 +925,7 @@ static struct scsi_host_template driver_template = { .sg_tablesize = SG_ALL, .cmd_per_lun = CMD_PER_LUN, .use_clustering = DISABLE_CLUSTERING, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };#include <linux/module.h> #include "scsi_module.c"
drivers/scsi/mac_scsi.c +1 -0
@@ -594,6 +594,7 @@ static struct scsi_host_template driver_template = { .cmd_per_lun = CMD_PER_LUN, .unchecked_isa_dma = 0, .use_clustering = DISABLE_CLUSTERING + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };
drivers/scsi/pas16.c +1 -0
@@ -628,6 +628,7 @@ static struct scsi_host_template driver_template = { .sg_tablesize = SG_ALL, .cmd_per_lun = CMD_PER_LUN, .use_clustering = DISABLE_CLUSTERING, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };#include "scsi_module.c"
drivers/scsi/sun3_scsi.c +2 -1
@@ -633,7 +633,8 @@ static struct scsi_host_template driver_template = { .this_id = 7, .sg_tablesize = SG_TABLESIZE, .cmd_per_lun = CMD_PER_LUN, - .use_clustering = DISABLE_CLUSTERING + .use_clustering = DISABLE_CLUSTERING, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };
drivers/scsi/sun3_scsi_vme.c +1 -0
@@ -577,6 +577,7 @@ static struct scsi_host_template driver_template = { .sg_tablesize = SG_TABLESIZE, .cmd_per_lun = CMD_PER_LUN, .use_clustering = DISABLE_CLUSTERING + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };
drivers/scsi/t128.c +1 -0
@@ -443,5 +443,6 @@ static struct scsi_host_template driver_template = { .sg_tablesize = SG_ALL, .cmd_per_lun = CMD_PER_LUN, .use_clustering = DISABLE_CLUSTERING, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };#include "scsi_module.c"
drivers/usb/storage/scsiglue.c +3 -0
@@ -499,6 +499,9 @@ struct scsi_host_template usb_stor_host_template = {/* we do our own delay after a device or bus reset */ .skip_settle_delay = 1, +/* calling scsi_eh_prep_cmnd needs this set */ + .sense_buffsize = US_SENSE_SIZE, +/* sysfs device attributes */ .sdev_attrs = sysfs_device_attr_list,
Signature · 1 line
1.5.3.3
RFC PATCH 17/24 BusLogic: Use of scsi_make_sense() API for DMA-able sense buffer
In reply to Boaz Harrosh - Use a pre allocated, DMA mapped, sense buffer at each command, Using the scsi_make_sense() API. And scsi_return_sense() when done.
- set .pre_allocate_sense at host template.
--- drivers/scsi/BusLogic.c | 24 +++++++++++------------- drivers/scsi/BusLogic.h | 1 + 2 files changed, 12 insertions(+), 13 deletions(-)
drivers/scsi/BusLogic.c +11 -13
@@ -42,17 +42,19 @@#include <linux/spinlock.h> #include <linux/jiffies.h> #include <linux/dma-mapping.h> -#include <scsi/scsicam.h> #include <asm/dma.h> #include <asm/io.h> #include <asm/system.h> +#include <scsi/scsicam.h> #include <scsi/scsi.h> #include <scsi/scsi_cmnd.h> #include <scsi/scsi_device.h> #include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> +#include <scsi/scsi_eh.h> +#include "BusLogic.h" #include "FlashPoint.c"
@@ -309,6 +311,7 @@ static void BusLogic_DeallocateCCB(struct BusLogic_CCB *CCB)pci_unmap_single (HostAdapter->PCI_Device,CCB ->SenseDataPointer,CCB ->SenseDataLength,PCI_DMA_FROMDEVICE ); +scsi_return_sense (CCB ->Command,CCB ->sense_buffer);CCB ->Command =NULL ;CCB ->Status = BusLogic_CCB_Free;CCB ->Next = HostAdapter->Free_CCBs;
@@ -2627,7 +2630,7 @@ static void BusLogic_ProcessCompletedCCBs(struct BusLogic_HostAdapter *HostAdaptBusLogic_Notice ("\n" , HostAdapter);BusLogic_Notice ("Sense " , HostAdapter);for (i =0 ; i <CCB ->SenseDataLength; i++) -BusLogic_Notice (" %02X" , HostAdapter, Command->sense_buffer[i]); +BusLogic_Notice (" %02X" , HostAdapter,CCB ->sense_buffer[i]);BusLogic_Notice ("\n" , HostAdapter); } }
@@ -2816,16 +2819,6 @@ static int BusLogic_QueueCommand(struct scsi_cmnd *Command, void (*CompletionRouint Count;struct BusLogic_CCB *CCB ;/* -SCSI REQUEST_SENSE commands will be executed automatically by the Host -Adapter for any errors, so they should not be executed explicitly unless -the Sense Data is zero indicating that no error occurred. -*/ -if (CDB [0 ] ==REQUEST_SENSE && Command->sense_buffer[0 ] !=0 ) { - Command->result =DID_OK <<16 ; -CompletionRoutine (Command); -return 0 ; - } -/* Allocate a CCB from the Host Adapter's free list. In the unlikely event that there are none available and memory allocation fails, wait 1 second and try again. If that fails, the Host Adapter is probably hung so signal
@@ -2948,7 +2941,10 @@ static int BusLogic_QueueCommand(struct scsi_cmnd *Command, void (*CompletionRou }memcpy (CCB ->CDB,CDB , CDB_Length);CCB ->SenseDataLength =SCSI_SENSE_BUFFERSIZE ; -CCB ->SenseDataPointer =pci_map_single (HostAdapter->PCI_Device, Command->sense_buffer,CCB ->SenseDataLength,PCI_DMA_FROMDEVICE ); +CCB ->sense_buffer =scsi_make_sense (Command); +CCB ->SenseDataPointer =pci_map_single (HostAdapter->PCI_Device, +CCB ->sense_buffer,CCB ->SenseDataLength, +PCI_DMA_FROMDEVICE );CCB ->Command = Command; Command->scsi_done = CompletionRoutine;if (BusLogic_MultiMasterHostAdapterP (HostAdapter)) {
@@ -3575,6 +3571,8 @@ static struct scsi_host_template Bus_Logic_template = { .unchecked_isa_dma = 1, .max_sectors = 128, .use_clustering = ENABLE_CLUSTERING, + .pre_allocate_sense = 1, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };/*
drivers/scsi/BusLogic.h +1 -0
@@ -890,6 +890,7 @@ struct BusLogic_CCB {struct BusLogic_CCB *NextAll;struct BusLogic_ScatterGatherSegment ScatterGatherList[BusLogic_ScatterGatherLimit]; +u8 *sense_buffer; };/*
Signature · 1 line
1.5.3.3
RFC PATCH 18/24 eata: Use of scsi_make_sense() API for DMA-able sense buffer
In reply to Boaz Harrosh - Use a pre allocated, DMA mapped, sense buffer at each command, Using the scsi_make_sense() API. And scsi_return_sense() when done.
- mark this driver as: "need for a pre allocated sense buffer" at host template.
--- drivers/scsi/eata.c | 20 +++++++++++++------- 1 files changed, 13 insertions(+), 7 deletions(-)
drivers/scsi/eata.c +13 -7
@@ -501,6 +501,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/scsicam.h> +#include <scsi/scsi_eh.h> static int eata2x_detect (struct scsi_host_template *);static int eata2x_release (struct Scsi_Host *);
@@ -524,6 +525,8 @@ static struct scsi_host_template driver_template = { .this_id = 7, .unchecked_isa_dma = 1, .use_clustering = ENABLE_CLUSTERING, + .pre_allocate_sense = 1, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };#if !defined(__BIG_ENDIAN_BITFIELD) && !defined(__LITTLE_ENDIAN_BITFIELD)
@@ -801,6 +804,7 @@ struct mscp {u_int32_t data_address;/* If sg=0 Data Address, if sg=1 sglist address */ u_int32_t sp_dma_addr;/* Address where sp is DMA'ed when cp completes */ u_int32_t sense_addr;/* Address where Sense Data is DMA'ed on error */ +u8 *sense_buffer;/* scsi_{make,return}_sense pointer */ /* Additional fields begin here. */ struct scsi_cmnd *SCpnt;
@@ -1619,9 +1623,9 @@ static void map_dma(unsigned int i, struct hostdata *ha) SCpnt = cpp->SCpnt; pci_dir = SCpnt->sc_data_direction; -if (SCpnt->sense_buffer) - cpp->sense_addr = -H2DEV (pci_map_single (ha->pdev, SCpnt->sense_buffer, + cpp->sense_buffer =scsi_make_sense (SCpnt); + cpp->sense_addr = +H2DEV (pci_map_single (ha->pdev, cpp->sense_buffer,SCSI_SENSE_BUFFERSIZE ,PCI_DMA_FROMDEVICE )); cpp->sense_len =SCSI_SENSE_BUFFERSIZE ;
@@ -1651,9 +1655,11 @@ static void unmap_dma(unsigned int i, struct hostdata *ha) SCpnt = cpp->SCpnt; pci_dir = SCpnt->sc_data_direction; -if (DEV2H (cpp->sense_addr)) +if (DEV2H (cpp->sense_addr)) {pci_unmap_single (ha->pdev,DEV2H (cpp->sense_addr),DEV2H (cpp->sense_len),PCI_DMA_FROMDEVICE ); +scsi_return_sense (SCpnt, cpp->sense_buffer); + }scsi_dma_unmap (SCpnt);
@@ -2428,7 +2434,7 @@ static irqreturn_t ihdlr(int irq, struct Scsi_Host *shost)/* Works around a flaw in scsi.c */ else if (tstatus == CHECK_CONDITION && SCpnt->device->type == TYPE_DISK - && (SCpnt->sense_buffer[2] & 0xf) == RECOVERED_ERROR) + && (cpp->sense_buffer[2] & 0xf) == RECOVERED_ERROR) status =DID_BUS_BUSY <<16 ; else
@@ -2440,13 +2446,13 @@ static irqreturn_t ihdlr(int irq, struct Scsi_Host *shost)if (spp->target_status && SCpnt->device->type ==TYPE_DISK && (!(tstatus ==CHECK_CONDITION && ha->iocount <=1000 && - (SCpnt->sense_buffer[2 ] &0xf ) ==NOT_READY ))) + (cpp->sense_buffer[2 ] &0xf ) ==NOT_READY )))printk ("%s: ihdlr, target %d.%d:%d, pid %ld, " "target_status 0x%x, sense key 0x%x.\n" , ha->board_name, SCpnt->device->channel, SCpnt->device->id, SCpnt->device->lun, SCpnt->serial_number, - spp->target_status, SCpnt->sense_buffer[2 ]); + spp->target_status, cpp->sense_buffer[2 ]); ha->target_to[SCpnt->device->id][SCpnt->device->channel] =0 ;
Signature · 1 line
1.5.3.3
RFC PATCH 19/24 initio: Use of scsi_make_sense() API for DMA-able sense buffer
In reply to Boaz Harrosh - Use a pre allocated, DMA mapped, sense buffer at each command, Using the scsi_make_sense() API. And scsi_return_sense() when done.
- Set .pre_allocate_sense && .sense_buffsize at host template.
--- drivers/scsi/initio.c | 12 +++++++++--- drivers/scsi/initio.h | 1 + 2 files changed, 10 insertions(+), 3 deletions(-)
drivers/scsi/initio.c +9 -3
@@ -102,6 +102,7 @@#include <scsi/scsi_device.h> #include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> +#include <scsi/scsi_eh.h> #include "initio.h"
@@ -2578,8 +2579,9 @@ static void initio_build_scb(struct initio_host * host, struct scsi_ctrl_blk * c cblk->flags |=SCF_SENSE ;/* Turn on auto request sense */ + cblk->sense_buffer =scsi_make_sense (cmnd);/* Map the sense buffer into bus memory */ - dma_addr =dma_map_single (&host->pci_dev->dev, cmnd->sense_buffer, + dma_addr =dma_map_single (&host->pci_dev->dev, cblk->sense_buffer,SENSE_SIZE ,DMA_FROM_DEVICE ); cblk->senseptr =cpu_to_le32 ((u32 )dma_addr); cblk->senselen =cpu_to_le32 (SENSE_SIZE );
@@ -2733,7 +2735,8 @@ static int i91u_biosparam(struct scsi_device *sdev, struct block_device *dev,* was mapped originally as part of initio_build_scb */ -static void i91u_unmap_scb (struct pci_dev *pci_dev,struct scsi_cmnd *cmnd) +static void i91u_unmap_scb (struct pci_dev *pci_dev,struct scsi_ctrl_blk *cblk, +struct scsi_cmnd *cmnd) {/* auto sense buffer */ if (cmnd->SCp.ptr) {
@@ -2741,6 +2744,7 @@ static void i91u_unmap_scb(struct pci_dev *pci_dev, struct scsi_cmnd *cmnd) (dma_addr_t)((unsigned long )cmnd->SCp.ptr), SENSE_SIZE, DMA_FROM_DEVICE); cmnd->SCp.ptr =NULL ; +scsi_return_sense (cmnd, cblk->sense_buffer); }/* request buffer */
@@ -2817,7 +2821,7 @@ static void i91uSCBPost(u8 * host_mem, u8 * cblk_mem) cmnd->result = cblk->tastat | (cblk->hastat <<16 );WARN_ON (cmnd ==NULL ); -i91u_unmap_scb (host->pci_dev, cmnd); +i91u_unmap_scb (host->pci_dev, cblk, cmnd); cmnd->scsi_done (cmnd);/* Notify system DONE */ initio_release_scb (host, cblk);/* Release SCB for current channel */ }
@@ -2833,6 +2837,8 @@ static struct scsi_host_template initio_template = { .sg_tablesize = SG_ALL, .cmd_per_lun = 1, .use_clustering = ENABLE_CLUSTERING, + .pre_allocate_sense = 1, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, }; static int initio_probe_one(struct pci_dev *pdev,
drivers/scsi/initio.h +1 -0
@@ -384,6 +384,7 @@ struct scsi_ctrl_blk {void (*post) (u8 *,u8 *);/*4C POST routine */ struct scsi_cmnd *srb;/*50 SRB Pointer */ struct sg_entry sglist[TOTAL_SG_ENTRY ];/*54 Start of SG list */ +u8 *sense_buffer; };/* Bit Definition for status */
Signature · 1 line
1.5.3.3
RFC PATCH 20/24 u14-34f: Use of scsi_make_sense() API for DMA-able sense buffer
In reply to Boaz Harrosh - Use a pre allocated, DMA mapped, sense buffer at each command, Using the scsi_make_sense() API. And scsi_return_sense() when done.
- Set .pre_allocate_sense && .sense_buffsize at host template.
--- drivers/scsi/u14-34f.c | 22 ++++++++++++++-------- 1 files changed, 14 insertions(+), 8 deletions(-)
drivers/scsi/u14-34f.c +14 -8
@@ -429,6 +429,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_tcq.h> #include <scsi/scsicam.h> +#include <scsi/scsi_eh.h> static int u14_34f_detect (struct scsi_host_template *);static int u14_34f_release (struct Scsi_Host *);
@@ -451,6 +452,8 @@ static struct scsi_host_template driver_template = { .this_id = 7, .unchecked_isa_dma = 1, .use_clustering = ENABLE_CLUSTERING, + .pre_allocate_sense = 1, + .sense_buffsize = SCSI_SENSE_BUFFERSIZE, };#if !defined(__BIG_ENDIAN_BITFIELD) && !defined(__LITTLE_ENDIAN_BITFIELD)
@@ -577,6 +580,7 @@ struct mscp {/* Additional fields begin here. */ struct scsi_cmnd *SCpnt; +u8 *sense_buffer;unsigned int cpp_index;/* cp index */ /* All the cp structure is zero filled by queuecommand except the
@@ -1118,8 +1122,9 @@ static void map_dma(unsigned int i, unsigned int j) { cpp = &HD (j)->cp[i]; SCpnt = cpp->SCpnt; pci_dir = SCpnt->sc_data_direction; -if (SCpnt->sense_buffer) - cpp->sense_addr =H2DEV (pci_map_single (HD (j)->pdev, SCpnt->sense_buffer, + cpp->sense_buffer =scsi_make_sense (SCpnt); +BUG_ON (!cpp->sense_buffer); + cpp->sense_addr =H2DEV (pci_map_single (HD (j)->pdev, cpp->sense_buffer,SCSI_SENSE_BUFFERSIZE ,PCI_DMA_FROMDEVICE )); cpp->sense_len =SCSI_SENSE_BUFFERSIZE ;
@@ -1144,7 +1149,7 @@ static void map_dma(unsigned int i, unsigned int j) { }else { pci_dir =PCI_DMA_BIDIRECTIONAL ; - cpp->data_len =H2DEV (scsi_bufflen (SCpnt)); + cpp->data_len =0 ; } }
@@ -1156,9 +1161,9 @@ static void unmap_dma(unsigned int i, unsigned int j) { cpp = &HD (j)->cp[i]; SCpnt = cpp->SCpnt; pci_dir = SCpnt->sc_data_direction; -if (DEV2H (cpp->sense_addr)) -pci_unmap_single (HD (j)->pdev,DEV2H (cpp->sense_addr), +pci_unmap_single (HD (j)->pdev,DEV2H (cpp->sense_addr),DEV2H (cpp->sense_len),PCI_DMA_FROMDEVICE ); +scsi_return_sense (SCpnt, cpp->sense_buffer);scsi_dma_unmap (SCpnt);
@@ -1298,6 +1303,7 @@ static int u14_34f_queuecommand(struct scsi_cmnd *SCpnt, void (*done)(struct scs/* Use data transfer direction SCpnt->sc_data_direction */ scsi_to_dev_dir (i, j); +/* FIXME:map_dma can fail */ /* Map DMA buffers and SG list */ map_dma (i, j);
@@ -1821,7 +1827,7 @@ static irqreturn_t ihdlr(int irq, unsigned int j) {/* Works around a flaw in scsi.c */ else if (tstatus == CHECK_CONDITION && SCpnt->device->type == TYPE_DISK - && (SCpnt->sense_buffer[2] & 0xf) == RECOVERED_ERROR) + && (spp->sense_buffer[2] & 0xf) == RECOVERED_ERROR) status =DID_BUS_BUSY <<16 ; else
@@ -1832,11 +1838,11 @@ static irqreturn_t ihdlr(int irq, unsigned int j) {if (spp->target_status && SCpnt->device->type ==TYPE_DISK && (!(tstatus ==CHECK_CONDITION &&HD (j)->iocount <=1000 && - (SCpnt->sense_buffer[2 ] &0xf ) ==NOT_READY ))) + (spp->sense_buffer[2 ] &0xf ) ==NOT_READY )))scmd_printk (KERN_INFO , SCpnt,"ihdlr, pid %ld, target_status 0x%x, sense key 0x%x.\n" , SCpnt->serial_number, spp->target_status, - SCpnt->sense_buffer[2 ]); + spp->sense_buffer[2 ]);HD (j)->target_to[scmd_id (SCpnt)][scmd_channel (SCpnt)] =0 ;
Signature · 1 line
1.5.3.3
RFC PATCH 21/24 scsi_tgt: use of sense accessors
In reply to Boaz Harrosh FIXME: I need help with this driver (Pete?)
I used scsi_sense() in a none const way. But since scsi_tgt is the ULD here, it can just access it's own sense buffer directly. I did not use scsi_eh_cpy_sense() because I did not want the extra copy. Pete will want to use a 260 bytes buffer here.
Need-help-from: Pete Wyckoff <[email protected]>
--- drivers/scsi/scsi_tgt_lib.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/scsi/scsi_tgt_lib.c +2 -1
@@ -29,6 +29,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_transport.h> #include <scsi/scsi_tgt.h> +#include <scsi/scsi_eh.h> #include "scsi_tgt_priv.h"
@@ -397,7 +398,7 @@ static int scsi_tgt_copy_sense(struct scsi_cmnd *cmd, unsigned long uaddr, {char __user *p = (char __user *) uaddr; -if (copy_from_user (cmd->sense_buffer, p, +if (copy_from_user (scsi_sense (cmd), p,min_t (unsigned,SCSI_SENSE_BUFFERSIZE , len))) {printk (KERN_ERR "Could not copy the sense buffer\n" );return -EIO ;
Signature · 1 line
1.5.3.3
Re: [PATCH 21/24][RFC] scsi_tgt: use of sense accessors
[email protected] wrote on Mon, 04 Feb 2008 19:53 +0200:
Quoted 9 lines
FIXME: I need help with this driver (Pete?)
I usedscsi_sense()in a none const way. But sincescsi_tgtis the ULD here, it can just access it's own sense buffer directly. I did not usescsi_eh_cpy_sense()because I did not want the extra copy. Pete will want to use a 260 bytes buffer here.Signed-off-by Boaz Harrosh <[email protected]>Need-help-from: Pete Wyckoff <[email protected]>
FYI, I never use scsi_tgt. Only just pure userspace on the target, and a dumb ethernet NIC that does not know it is speaking any form of SCSI.
People who need scsi_tgt have real target-enabled NICs like the fancy qla4xxx. Those act as SCSI targets across FC or IP or whatever and bring commands into the kernel, which then relays them to a userspace tgtd process, which does the read/write as necessary, and returns a result code to the NIC to ship back across FC.
So sorry, I won't take a guess at what has to happen here. But yeah, you are right that an OSD target implementation would at times need a sense buffer bigger than 96. Protocol maximum length for all sense data is 264.
-- Pete
Re: [PATCH 21/24][RFC] scsi_tgt: use of sense accessors
Quoted 8 lines · On Tue, 5 Feb 2008 11:21:33 -0500 Pete Wyckoff <[email protected]> wrote:
[email protected] wrote on Mon, 04 Feb 2008 19:53 +0200:
Quoted 7 lines
FIXME: I need help with this driver (Pete?)
I usedscsi_sense()in a none const way. But sincescsi_tgtis the ULD here, it can just access it's own sense buffer directly. I did not usescsi_eh_cpy_sense()because I did not want the extra copy. Pete will want to use a 260 bytes buffer here.
Quoted 6 lines
Quoted 2 lines
Signed-off-by Boaz Harrosh <[email protected]>Need-help-from: Pete Wyckoff <[email protected]>
FYI, I never use
scsi_tgt. Only just pure userspace on the target, and a dumb ethernet NIC that does not know it is speaking any form of SCSI.
Seems that many people misunderstand STGT iSCSI (and iSER), FCoE, and SRP (not implemented yet) software target drivers. They don't use the tgt kernel module. They just run in user space like user-space nfs daemon.
Re: [PATCH 21/24][RFC] scsi_tgt: use of sense accessors
Quoted 13 lines · FUJITA Tomonori
Quoted 10 lines · On Tue, 5 Feb 2008 11:21:33 -0500 Pete Wyckoff <[email protected]> wrote:
[email protected] wrote on Mon, 04 Feb 2008 19:53 +0200:
Quoted 9 lines
FIXME: I need help with this driver (Pete?)
I usedscsi_sense()in a none const way. But sincescsi_tgtis the ULD here, it can just access it's own sense buffer directly. I did not usescsi_eh_cpy_sense()because I did not want the extra copy. Pete will want to use a 260 bytes buffer here.Signed-off-by Boaz Harrosh <[email protected]>Need-help-from: Pete Wyckoff <[email protected]>
Quoted 8 lines
Quoted 3 lines
FYI, I never use
scsi_tgt. Only just pure userspace on the target, and a dumb ethernet NIC that does not know it is speaking any form of SCSI.Seems that many people misunderstand STGT iSCSI (and iSER), FCoE, and SRP (not implemented yet) software target drivers. They don't use the tgt kernel module. They just run in user space like user-space nfs daemon.
FWIW, some AHCI and other SATA chips implement ATA target mode. I'm watching this SCSI work with interest, hoping that many of the concepts (and code?) can be applied to SATA as well.
If for no other reason than I can build a cheap ATA protocol analyzer, or bridge.
Jeff
RFC PATCH 22/24 scsi upper layer use of sense accessors
In reply to Boaz Harrosh code that inspects the return sense can use the new scsi_sense() accessor.
--- drivers/scsi/constants.c | 4 ++-- drivers/scsi/sd.c | 4 ++-- drivers/scsi/sr.c | 15 +++++++-------- 3 files changed, 11 insertions(+), 12 deletions(-)
drivers/scsi/constants.c +2 -2
@@ -1349,10 +1349,10 @@ void scsi_print_sense(char *name, struct scsi_cmnd *cmd)struct scsi_sense_hdr sshdr;scmd_printk (KERN_INFO , cmd,"" ); -scsi_decode_sense_buffer (cmd->sense_buffer,SCSI_SENSE_BUFFERSIZE , +scsi_decode_sense_buffer (scsi_sense (cmd),SCSI_SENSE_BUFFERSIZE , &sshdr);scsi_show_sense_hdr (&sshdr); -scsi_decode_sense_extras (cmd->sense_buffer,SCSI_SENSE_BUFFERSIZE , +scsi_decode_sense_extras (scsi_sense (cmd),SCSI_SENSE_BUFFERSIZE , &sshdr);scmd_printk (KERN_INFO , cmd,"" );scsi_show_extd_sense (sshdr.asc, sshdr.ascq);
drivers/scsi/sd.c +2 -2
@@ -960,7 +960,7 @@ static int sd_done(struct scsi_cmnd *SCpnt)case MEDIUM_ERROR :if (!blk_fs_request (SCpnt->request)) goto out; - info_valid =scsi_get_sense_info_fld (SCpnt->sense_buffer, + info_valid =scsi_get_sense_info_fld (scsi_sense (SCpnt),SCSI_SENSE_BUFFERSIZE , &bad_lba);if (!info_valid)
@@ -999,7 +999,7 @@ static int sd_done(struct scsi_cmnd *SCpnt)*/ scsi_print_sense ("sd" , SCpnt); SCpnt->result =0 ; -memset (SCpnt->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); +scsi_eh_reset_sense (SCpnt); good_bytes = xfer_size;break ;case ILLEGAL_REQUEST :
drivers/scsi/sr.c +7 -8
@@ -230,6 +230,7 @@ out:*/ static int sr_done (struct scsi_cmnd *SCpnt) { +const u8 *sense =scsi_sense (SCpnt);int result = SCpnt->result;int this_count =scsi_bufflen (SCpnt);int good_bytes = (result ==0 ? this_count :0 );
@@ -248,17 +249,15 @@ static int sr_done(struct scsi_cmnd *SCpnt)* memcpy's that could be avoided. */ if (driver_byte (result) !=0 &&/* An error occurred */ - (SCpnt->sense_buffer[0 ] &0x7f ) ==0x70 ) {/* Sense current */ -switch (SCpnt->sense_buffer[2 ]) { + (sense[0 ] &0x7f ) ==0x70 ) {/* Sense current */ +switch (sense[2 ]) {case MEDIUM_ERROR :case VOLUME_OVERFLOW :case ILLEGAL_REQUEST : -if (!(SCpnt->sense_buffer[0 ] &0x90 )) +if (!(sense[0 ] &0x90 ))break ; - error_sector = (SCpnt->sense_buffer[3 ] <<24 ) | - (SCpnt->sense_buffer[4 ] <<16 ) | - (SCpnt->sense_buffer[5 ] <<8 ) | - SCpnt->sense_buffer[6 ]; + error_sector = (sense[3 ] <<24 ) | (sense[4 ] <<16 ) | + (sense[5 ] <<8 ) | sense[6 ];if (SCpnt->request->bio !=NULL ) block_sectors =bio_sectors (SCpnt->request->bio);
@@ -292,7 +291,7 @@ static int sr_done(struct scsi_cmnd *SCpnt)*/ scsi_print_sense ("sr" , SCpnt); SCpnt->result =0 ; - SCpnt->sense_buffer[0 ] =0x0 ; +scsi_eh_reset_sense (SCpnt); good_bytes = this_count;break ;
Signature · 1 line
1.5.3.3
RFC PATCH 23/24 block: Minor changes to sense handling
In reply to Boaz Harrosh - It is no longer allowed to call blk_execute_rq_nowait() with out a req->senes buffer. This is not a problem because greping all users shows that this does not happen.
- Add a sense_max_len which indicate the buffer size at req->sense. If zero then SCSI_SENSE_BUFFERSIZE is assumed. (As before)
- SCSI_SENSE_BUFFERSIZE is moved to scsi.h (from scsi_cmnd.h)
--- block/blk-core.c | 1 + block/blk-exec.c | 5 +++++ block/scsi_ioctl.c | 1 - include/linux/blkdev.h | 3 ++- include/scsi/scsi.h | 6 ++++++ include/scsi/scsi_cmnd.h | 1 - include/scsi/scsi_eh.h | 1 + 7 files changed, 15 insertions(+), 3 deletions(-)
block/blk-core.c +1 -0
@@ -120,6 +120,7 @@ void rq_init(struct request_queue *q, struct request *rq) rq->data =NULL ; rq->nr_phys_segments =0 ; rq->sense =NULL ; + rq->sense_max_len =0 ; rq->end_io =NULL ; rq->end_io_data =NULL ; rq->completion_data =NULL ;
block/blk-exec.c +5 -0
@@ -51,6 +51,10 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk, {int where = at_head ?ELEVATOR_INSERT_FRONT :ELEVATOR_INSERT_BACK ; +BUG_ON (!rq->sense); +if (!rq->sense_max_len) + rq->sense_max_len =SCSI_SENSE_BUFFERSIZE ; + rq->rq_disk = bd_disk; rq->cmd_flags |=REQ_NOMERGE ; rq->end_io = done;
@@ -90,6 +94,7 @@ int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk,memset (sense,0 ,sizeof (sense)); rq->sense = sense; rq->sense_len =0 ; + rq->sense_max_len =SCSI_SENSE_BUFFERSIZE ; } rq->end_io_data = &wait;
block/scsi_ioctl.c +0 -1
@@ -30,7 +30,6 @@#include <scsi/scsi.h> #include <scsi/scsi_ioctl.h> -#include <scsi/scsi_cmnd.h> /* Command group 3 is reserved and should never be used. */ const unsigned char scsi_command_size_tbl[8] =
include/linux/blkdev.h +2 -1
@@ -222,7 +222,8 @@ struct request { };unsigned int data_len; -unsigned int sense_len; +unsigned short sense_len; +unsigned short sense_max_len;void *data;void *sense;
include/scsi/scsi.h +6 -0
@@ -415,6 +415,12 @@ struct scsi_lun {#define sense_valid (sense) ((sense) & 0x80);/* +* Some scsi sense constants +*/ +#define SCSI_SENSE_BUFFERSIZE 96 +#define SCSI_SENSE_MAX_SIZE 260 + +/* * default timeouts */ #define FORMAT_UNIT_TIMEOUT (2 * 60 * 60 * HZ)
include/scsi/scsi_cmnd.h +0 -1
@@ -88,7 +88,6 @@ struct scsi_cmnd {struct request *request;/* The command we are working on */ -#define SCSI_SENSE_BUFFERSIZE 96unsigned char *sense_buffer;/* obtained by REQUEST SENSE when * CHECK CONDITION is received on original
include/scsi/scsi_eh.h +1 -0
@@ -4,6 +4,7 @@#include <linux/scatterlist.h> #include <scsi/scsi_cmnd.h> +#include <scsi/scsi.h> struct scsi_device ;struct Scsi_Host ;
Signature · 1 line
1.5.3.3
RFC PATCH 24/24 scsi: New sense handling
In reply to Boaz Harrosh Current code allocates a dma-able sense buffer for every scsi command. This is in 99.9% of cases a waste because:
- Most commands end successfully and do not need any sense space.
- most LLDs have the sense information already in their private data and do not DMA this information. the sense can go directly to ULD's buffer.
- Drivers that do synchronous REQUEST_SENSE through the scs_eh_prep_cmnd mechanism need only one dma buffer per target. (contingent allegiance condition)
This leaves us with a very few drivers that need dma-able sense-buffer pre-allocated per-command. These drivers set the .pre_allocate_sense flag in thier host template and the mid-layer will make sure a sense buffer is allocated for them.
So:
- Removed the global sense mem_cache and sense allocation per command.
- Add a per-host sense mempool_t of buffer-size specified by host template in .sense_buffsize. If .sense_buffsize is not set then no mempool_t is not allocated. (Note: This is true for the majority of drivers.)
- If host does not have .pre_allocate_sense set then a reserved sense buffer is allocated per scsi-target (LUN).
- If host has .pre_allocate_sense set then a sense buffer is allocated in get_command() and the code behaves like today.
- Drivers that need a dma sense buffer call scsi_make_sense()/scsi_return_sense() Theses drivers where already converted. Here the implementation will allocate the sense buffer from above per-host pool. Subsequent calls to scsi_make_sense() will return the same buffer. The first call to scsi_return_sense() will do the scsi_eh_cpy_sense(), and return the buffer to free store. Subsequent calls will do nothing.
- The scsi_eh_prep_cmnd()/scsi_eh_return_cmnd() are converted to use the scsi_make_sense()/scsi_return_sense(). Since these drivers set for .sense_buffsize but with .pre_allocate_sense not set, a pre-allocated sense buffer is guarantied per target even in a low memory condition situation.
- scsi_cmnd->sense_buffer is removed as it is no longer needed. And scsi_eh_cpy_sense()
Will copy directly to ULD's supplied buffer at request->sense. Not maxing ->sense_max_len
and setting ->sense_len to copied count. So scsi_lib sense handling code can also be
removed.--- drivers/scsi/scsi.c | 207 ++++++++++++++++++++++++++++++--------------- drivers/scsi/scsi_error.c | 45 +++++++--- drivers/scsi/scsi_lib.c | 15 +--- drivers/scsi/scsi_priv.h | 3 +- drivers/scsi/scsi_scan.c | 6 ++ include/scsi/scsi_cmnd.h | 5 +- include/scsi/scsi_eh.h | 14 +++- include/scsi/scsi_host.h | 3 + 8 files changed, 194 insertions(+), 104 deletions(-)
drivers/scsi/scsi.c +137 -70
@@ -132,24 +132,20 @@ const char * scsi_device_type(unsigned type)EXPORT_SYMBOL (scsi_device_type);struct scsi_host_cmd_pool { -struct kmem_cache *cmd_slab; -struct kmem_cache *sense_slab; -unsigned int users; -char *cmd_name; -char *sense_name; -unsigned int slab_flags; -gfp_t gfp_mask; +struct kmem_cache *slab; +unsigned int users; +char *name; +unsigned int slab_flags; +gfp_t gfp_mask; };static struct scsi_host_cmd_pool scsi_cmd_pool = { - .cmd_name ="scsi_cmd_cache" , - .sense_name ="scsi_sense_cache" , + .name ="scsi_cmd_cache" , .slab_flags =SLAB_HWCACHE_ALIGN , };static struct scsi_host_cmd_pool scsi_cmd_dma_pool = { - .cmd_name ="scsi_cmd_cache(DMA)" , - .sense_name ="scsi_sense_cache(DMA)" , + .name ="scsi_cmd_cache(DMA)" , .slab_flags =SLAB_HWCACHE_ALIGN |SLAB_CACHE_DMA , .gfp_mask = __GFP_DMA, };
@@ -167,10 +163,9 @@ static DEFINE_MUTEX(host_cmd_pool_mutex);struct scsi_cmnd *__scsi_get_command (struct Scsi_Host *shost,gfp_t gfp_mask) {struct scsi_cmnd *cmd; -unsigned char *buf; - cmd =kmem_cache_alloc (shost->cmd_pool->cmd_slab, - gfp_mask | shost->cmd_pool->gfp_mask); + cmd =kmem_cache_alloc (shost->cmd_pool->slab, + gfp_mask | shost->cmd_pool->gfp_mask);if (unlikely (!cmd)) {unsigned long flags;
@@ -182,22 +177,6 @@ struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask)list_del_init (&cmd->list); }spin_unlock_irqrestore (&shost->free_list_lock, flags); - -if (cmd) { - buf = cmd->sense_buffer; -memset (cmd,0 ,sizeof (*cmd)); - cmd->sense_buffer = buf; - } - }else { - buf =kmem_cache_alloc (shost->cmd_pool->sense_slab, - gfp_mask | shost->cmd_pool->gfp_mask); -if (likely (buf)) { -memset (cmd,0 ,sizeof (*cmd)); - cmd->sense_buffer = buf; - }else { -kmem_cache_free (shost->cmd_pool->cmd_slab, cmd); - cmd =NULL ; - } }return cmd;
@@ -224,6 +203,7 @@ struct scsi_cmnd *scsi_get_command(struct scsi_device *dev, gfp_t gfp_mask)if (likely (cmd !=NULL )) {unsigned long flags; +memset (cmd,0 ,sizeof (*cmd)); cmd->device = dev;init_timer (&cmd->eh_timeout);INIT_LIST_HEAD (&cmd->list);
@@ -231,6 +211,16 @@ struct scsi_cmnd *scsi_get_command(struct scsi_device *dev, gfp_t gfp_mask)list_add_tail (&cmd->list, &dev->cmd_list);spin_unlock_irqrestore (&dev->list_lock, flags); cmd->jiffies_at_alloc = jiffies; + +if (cmd->device->host->hostt->pre_allocate_sense) { +u8 *sb; + + sb =scsi_make_sense (cmd); +if (unlikely (!sb)) { +scsi_put_command (cmd); + cmd =NULL ; + } + } }else put_device (&dev->sdev_gendev);
@@ -258,9 +248,11 @@ void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd,spin_unlock_irqrestore (&shost->free_list_lock, flags);if (likely (cmd !=NULL )) { -kmem_cache_free (shost->cmd_pool->sense_slab, - cmd->sense_buffer); -kmem_cache_free (shost->cmd_pool->cmd_slab, cmd); +if (cmd->dma_sense_check) { +WARN_ON (1 );/* driver did not call scsi_return_sense */ +scsi_return_sense (cmd, cmd->dma_sense_check); + } +kmem_cache_free (shost->cmd_pool->slab, cmd); }put_device (dev);
@@ -290,15 +282,106 @@ void scsi_put_command(struct scsi_cmnd *cmd) }EXPORT_SYMBOL (scsi_put_command); +static int scsi_setup_sense_pool (struct Scsi_Host *shost) +{ +if (!shost->sense_buffsize && shost->hostt->sense_buffsize) + shost->sense_buffsize = shost->hostt->sense_buffsize; +else if (!shost->hostt->sense_buffsize) +return 0 ;/* Host doesn't need a sense_pool */ + + shost->sense_slab =kmem_cache_create (shost->hostt->name, + shost->sense_buffsize,0 , shost->cmd_pool->slab_flags,NULL ); +if (!shost->sense_slab) { +printk (KERN_ERR "SCSI: can't init sense_slab %s\n" , + shost->hostt->name); +return -ENOMEM ; + } + + shost->sense_pool =mempool_create_slab_pool (1 , shost->sense_slab); +if (!shost->sense_pool) { +printk (KERN_ERR "SCSI: can't init sense_pool %s\n" , + shost->hostt->name); +kmem_cache_destroy (shost->sense_slab); + shost->sense_slab =NULL ; +return -ENOMEM ; + } +return 0 ; +} + +static void scsi_destroy_sense_pool (struct Scsi_Host *shost) +{ +if (shost->sense_pool) +mempool_destroy (shost->sense_pool); + shost->sense_pool =NULL ; +if (shost->sense_slab) +kmem_cache_destroy (shost->sense_slab); + shost->sense_slab =NULL ; +} + +int scsi_resize_sense_pool (struct Scsi_Host *shost,int inc_dec) +{ +if (inc_dec >0 ) { +u8 *sense_per_queue; + +if (mempool_resize (shost->sense_pool, + shost->sense_pool->min_nr +1 ,GFP_KERNEL )) + goto out; +/* Resize might not guaranty reserved allocation. +* This here will +*/ + sense_per_queue =mempool_alloc (shost->sense_pool, + shost->cmd_pool->gfp_mask |GFP_KERNEL ); +if (!sense_per_queue) { +mempool_resize (shost->sense_pool, + shost->sense_pool->min_nr -1 , + shost->cmd_pool->gfp_mask |GFP_KERNEL ); + goto out; + } +mempool_free (sense_per_queue, shost->sense_pool); + }else +mempool_resize (shost->sense_pool, + shost->sense_pool->min_nr -1 , + shost->cmd_pool->gfp_mask |GFP_KERNEL ); +return 0 ; + +out: +return -ENOMEM ; +} +u8 *scsi_make_sense (struct scsi_cmnd *cmd) { -return cmd->sense_buffer; +if (!cmd->dma_sense_check) { +struct Scsi_Host *shost = cmd->device->host; +if (!shost->sense_pool) { +/* did you not set .sense_buffsize in host template */ +WARN_ON (1 ); + shost->sense_buffsize =SCSI_SENSE_BUFFERSIZE ; +scsi_setup_sense_pool (shost); +BUG_ON (!shost->sense_pool); + } + + cmd->dma_sense_check = +mempool_alloc (shost->sense_pool, + shost->cmd_pool->gfp_mask); +if (cmd->dma_sense_check) +memset (cmd->dma_sense_check,0 , shost->sense_buffsize); + } +return cmd->dma_sense_check; }EXPORT_SYMBOL (scsi_make_sense);void scsi_return_sense (struct scsi_cmnd *cmd,u8 *sb) { -BUG_ON (cmd->sense_buffer != sb); +struct Scsi_Host *shost = cmd->device->host; + +if (!cmd->dma_sense_check)/* reentrant */ +return ; + +BUG_ON (cmd->dma_sense_check != sb); + +scsi_eh_cpy_sense (cmd, sb, shost->sense_buffsize); + cmd->dma_sense_check =NULL ; +mempool_free (sb, shost->sense_pool); }EXPORT_SYMBOL (scsi_return_sense);
@@ -327,50 +410,36 @@ int scsi_setup_command_freelist(struct Scsi_Host *shost)mutex_lock (&host_cmd_pool_mutex); pool = (shost->unchecked_isa_dma ? &scsi_cmd_dma_pool : &scsi_cmd_pool);if (!pool->users) { - pool->cmd_slab =kmem_cache_create (pool->cmd_name, -sizeof (struct scsi_cmnd ),0 , - pool->slab_flags,NULL ); -if (!pool->cmd_slab) + pool->slab =kmem_cache_create (pool->name, +sizeof (struct scsi_cmnd ),0 , + pool->slab_flags,NULL ); +if (!pool->slab) goto fail; - - pool->sense_slab =kmem_cache_create (pool->sense_name, -SCSI_SENSE_BUFFERSIZE ,0 , - pool->slab_flags,NULL ); -if (!pool->sense_slab) { -kmem_cache_destroy (pool->cmd_slab); - goto fail; - } } pool->users++; shost->cmd_pool = pool;mutex_unlock (&host_cmd_pool_mutex); +if (scsi_setup_sense_pool (shost)) + goto fail2; +/* * Get one backup command for this host. */ - cmd =kmem_cache_alloc (shost->cmd_pool->cmd_slab, -GFP_KERNEL | shost->cmd_pool->gfp_mask); + cmd =kmem_cache_alloc (shost->cmd_pool->slab, +GFP_KERNEL | shost->cmd_pool->gfp_mask);if (!cmd) - goto fail2; - - cmd->sense_buffer =kmem_cache_alloc (shost->cmd_pool->sense_slab, -GFP_KERNEL | - shost->cmd_pool->gfp_mask); -if (!cmd->sense_buffer) - goto fail2; - + goto fail3;list_add (&cmd->list, &shost->free_list);return 0 ; + fail3: +scsi_destroy_sense_pool (shost); fail2: -if (cmd) -kmem_cache_free (shost->cmd_pool->cmd_slab, cmd);mutex_lock (&host_cmd_pool_mutex); -if (!--pool->users) { -kmem_cache_destroy (pool->cmd_slab); -kmem_cache_destroy (pool->sense_slab); - } +if (!--pool->users) +kmem_cache_destroy (pool->slab); fail:mutex_unlock (&host_cmd_pool_mutex);return -ENOMEM ;
@@ -387,16 +456,14 @@ void scsi_destroy_command_freelist(struct Scsi_Host *shost) cmd = list_entry(shost->free_list.next, struct scsi_cmnd, list);list_del_init (&cmd->list); -kmem_cache_free (shost->cmd_pool->sense_slab, - cmd->sense_buffer); -kmem_cache_free (shost->cmd_pool->cmd_slab, cmd); +kmem_cache_free (shost->cmd_pool->slab, cmd); } +scsi_destroy_sense_pool (shost); +mutex_lock (&host_cmd_pool_mutex); -if (!--shost->cmd_pool->users) { -kmem_cache_destroy (shost->cmd_pool->cmd_slab); -kmem_cache_destroy (shost->cmd_pool->sense_slab); - } +if (!--shost->cmd_pool->users) +kmem_cache_destroy (shost->cmd_pool->slab);mutex_unlock (&host_cmd_pool_mutex); }
drivers/scsi/scsi_error.c +32 -13
106 lines not on this page · raw message
drivers/scsi/scsi_lib.c +2 -13
@@ -452,7 +452,7 @@ static void scsi_init_cmd_errh(struct scsi_cmnd *cmd) { cmd->serial_number =0 ;scsi_set_resid (cmd,0 ); -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); +scsi_eh_reset_sense (cmd);if (cmd->cmd_len ==0 ) cmd->cmd_len =scsi_command_size (cmd->cmnd); }
@@ -861,20 +861,9 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)if (blk_pc_request (req)) {/* SG_IO ioctl from block level */ req->errors = result; -if (result) { +if (result) clear_errors =0 ; -if (sense_valid && req->sense) { -/* -* SG_IO wants current and deferred errors -*/ -int len =8 + cmd->sense_buffer[7 ]; -if (len >SCSI_SENSE_BUFFERSIZE ) - len =SCSI_SENSE_BUFFERSIZE ; -memcpy (req->sense, cmd->sense_buffer, len); - req->sense_len = len; - } - }if (scsi_bidi_cmnd (cmd)) {/* will also release_buffers */ scsi_end_bidi_request (cmd);
drivers/scsi/scsi_priv.h +2 -1
@@ -17,7 +17,7 @@ struct scsi_nl_hdr;#define SCSI_EH_CANCEL_CMD 0x0001/* Cancel this cmd */ #define SCSI_SENSE_VALID (scmd) \ - (((scmd)->sense_buffer[0] & 0x70) == 0x70) + ((scsi_sense(scmd)[0] & 0x70) == 0x70)/* hosts.c */ extern int scsi_init_hosts (void );
@@ -37,6 +37,7 @@ static inline void scsi_log_send(struct scsi_cmnd *cmd)static inline void scsi_log_completion (struct scsi_cmnd *cmd,int disposition) { };#endif +extern int scsi_resize_sense_pool (struct Scsi_Host *shost,int inc_dec);/* scsi_scan.c */ int scsi_complete_async_scans (void );
drivers/scsi/scsi_scan.c +6 -0
@@ -311,6 +311,10 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, } } +if (!shost->hostt->pre_allocate_sense && +scsi_resize_sense_pool (shost,1 )) + goto out_device_destroy; +return sdev; out_device_destroy:
@@ -922,6 +926,8 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,static inline void scsi_destroy_sdev (struct scsi_device *sdev) {scsi_device_set_state (sdev,SDEV_DEL ); +if (!sdev->host->hostt->pre_allocate_sense) +scsi_resize_sense_pool (sdev->host,-1 );if (sdev->host->hostt->slave_destroy) sdev->host->hostt->slave_destroy (sdev);transport_destroy_device (&sdev->sdev_gendev);
include/scsi/scsi_cmnd.h +1 -4
@@ -88,10 +88,7 @@ struct scsi_cmnd {struct request *request;/* The command we are working on */ -unsigned char *sense_buffer; -/* obtained by REQUEST SENSE when -* CHECK CONDITION is received on original -* command (auto-sense) */ +u8 *dma_sense_check;/* Low-level done function - can be used by low-level driver to point * to completion function. Not used by mid/upper level code. */
include/scsi/scsi_eh.h +11 -3
@@ -4,7 +4,6 @@#include <linux/scatterlist.h> #include <scsi/scsi_cmnd.h> -#include <scsi/scsi.h> struct scsi_device ;struct Scsi_Host ;
@@ -79,6 +78,7 @@ struct scsi_eh_save {/* new command support */ unsigned char eh_cmnd[BLK_MAX_CDB ];struct scatterlist sense_sgl; +u8 *sense_buff; }; extern void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd,
@@ -97,12 +97,20 @@ extern void scsi_return_sense(struct scsi_cmnd *cmd, u8 *sb);/*FIXME: don't use, it's temporary */ static inline void scsi_eh_reset_sense (struct scsi_cmnd *cmd) { -memset (cmd->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE ); +memset (cmd->request->sense,0 , cmd->request->sense_max_len); }static inline const u8 *scsi_sense (struct scsi_cmnd *cmd) { -return cmd->sense_buffer; +if (cmd->dma_sense_check) { +/* Betwwen the call to scsi_make_sense() +* and scsi_return_sense() +*/ +WARN_ON (1 ); +return cmd->dma_sense_check; + } + +return cmd->request->sense; }#endif /* _SCSI_SCSI_EH_H */
include/scsi/scsi_host.h +3 -0
@@ -6,6 +6,7 @@#include <linux/types.h> #include <linux/workqueue.h> #include <linux/mutex.h> +#include <linux/mempool.h> struct request_queue ;struct block_device ;
@@ -623,6 +624,8 @@ struct Scsi_Host {/* actual allocated sense_buffsize for this host */ unsigned sense_buffsize; +struct kmem_cache *sense_slab; +mempool_t *sense_pool;/* * Optional work queue to be utilized by the transport
Signature · 1 line
1.5.3.3
Quoted 14 lines · Boaz Harrosh
There are 3 usages of sense handling in drivers
1. sense is available in driver internal structure and is mem-copied to upper level
2. ACHECK_CONDITIONstatus was returned and the driver uses thescsi_eh_prep_cmnd()
for aREQUEST_SENSEinvocation to the target. Then
returning the sense in
scsi_eh_return_cmnd(). A variation on this is when the
driver does nothing the queue
is frozen an the scsi watchdog timer does the above.
3. The underline host adapter does theREQUEST_SENSEand a pre-allocated and DMA mapped
sense buffer receives the sense information from HW.
Many years ago when "ACA" had a constructive meaning, so did "Autosense". Then about 5 years ago, "Autosense" disappeared completely since it became the de facto implementation of the then SCSI Execute Command "RPC", now just SCSI Execute Command procedure call.
At that point in time, the SCSI mid-layer decided to embrace this model and give the LLDD a scsi command structure which included the sense data buffer to a size that the SCSI mid-layer was interested in, at the moment 96 bytes, macro defined in include/scsi/scsi_cmnd.h.
The concept of "Autosense" was off-loaded to LLDD to emulate it if the specific target device to which the command was issued, didn't supply the sense data on CHECK CONDITION, and more so relevant to target devices which implemented queuing, thus the ACA.
And the mid-layer would consider extracting the sense data via REQUEST SENSE command
as a _special case_ if the LLDD/transport layer didn't implement the "autosense" model.
Your model is a regression of this as far
as sense data is concerned. Imagine if
your model had existed in the SCSI mid-layer and now someone would be submitting a series of patches a la the far simpler and more straightforward model, incidently currently
in place.
Quoted 5 lines
Now since all IO requests come with a sense set at
request->sensethe buffer atscsi_cmnd->sense_buffercan go away, and case 1 above can copy it's bits directly
into it.
That's ok.
Quoted 3 lines
for cases 2 and 3 above two new members in scsi host template will tell scsi-midlayer
what the driver wants to do.
This should be shunned upon. The LLDD/transport layer should provide sense to the SCSI mid-layer possibly by reissuing _its own_ SCSI command block representation to extract sense directly into the req sense buffer, so when the mid-layer SCSI command structure completes, scsi_done(), the sense data is already present and status is CHECK CONDITION.
(For example, a well written SATL would do this for ATAPI devices which completed the command with a sense key in the Error register different than 0.)
Quoted 36 lines
.sense_buffsize - will instruct a
mempool_tto be allocated per host with sense buffers
of ".sense_buffsize" size..pre_allocate_sense - If set tells the midlayer to pre-allocate a sense buffer for each
command. When set the code will
behave like today. If the sense
allocation fails the command
allocation also fails. (With one extra
sense at mempool)
If .pre_allocate_sense is not set,
thenmempool_tis resized to
accommodate one reserved buffer for
each target. So a call to
scsi_eh_prep_cmnd()will not fail to
allocate a sense buffer even
in low memory condition.Case 1 drivers call -
scsi_eh_cpy_sense()- to transfer their sense information to upper
layer. The drivers are completely abstracted from any
future changes at the scsi and block
sense handling.Case 3 drivers call the new scsi_make_sense()/scsi_return_sense() API to retrieve
a pointer to an extra DMAable sense buffer/return it to
free store.scsi_return_sense()
will callscsi_eh_cpy_sense()to set new sense
information into upper layer before
freeing the buffer back to the mempool. If the driver
properly set the .pre_allocate_sense
flag & .sense_buffsize at host template then these
calls are guaranteed to succeed.
Quoted 10 lines
Case 2 drivers continue to call scsi_eh_prep_cmnd()/scsi_eh_restore_cmnd() to reuse the
failing command forREQUEST_SENSE. Inside these functionsscsi_error.cwill call above
scsi_make_sense()/scsi_return_sense() for the actual
sense buffer allocation. In this
case there is a guarantied, one reserved buffer perscsi_device. (.pre_allocate_sense not set).
Drivers need to set proper size at .sense_buffsize in
host template, for this to work.
Absolutely too complicated for such a trivial thing as supplying sense data at scsi_done() time to the SCSI mid-layer (see previous points I wrote above). Too much complication for a such a small and trivial concept.
Would it be the case that you're trying to accomplish or accommodate a strange target behaviour? If so this should be done at the LLDD/transport protocol layer, and the SCSI mid-layer shouldn't have to see a scsi command complete with CHECK CONDITION set without a valid sense data in the sense buffer.
It's hard to see the advantage of this "new scsi sense handling" patches.
Luben
Quoted 36 lines · On Mon, 4 Feb 2008 18:39:22 -0800 (PST) Luben Tuikov <[email protected]> wrote:
Quoted 14 lines · Boaz Harrosh
There are 3 usages of sense handling in drivers
1. sense is available in driver internal structure and is mem-copied to upper level
2. ACHECK_CONDITIONstatus was returned and the driver uses thescsi_eh_prep_cmnd()
for aREQUEST_SENSEinvocation to the target. Then
returning the sense in
scsi_eh_return_cmnd(). A variation on this is when the
driver does nothing the queue
is frozen an the scsi watchdog timer does the above.
3. The underline host adapter does theREQUEST_SENSEand a pre-allocated and DMA mapped
sense buffer receives the sense information from HW.Many years ago when "ACA" had a constructive meaning, so did "Autosense". Then about 5 years ago, "Autosense" disappeared completely since it became the de facto implementation of the then SCSI Execute Command "RPC", now just SCSI Execute Command procedure call.
At that point in time, the SCSI mid-layer decided to embrace this model and give the LLDD a scsi command structure which included the sense data buffer to a size that the SCSI mid-layer was interested in, at the moment 96 bytes, macro defined in
include/scsi/scsi_cmnd.h.The concept of "Autosense" was off-loaded to LLDD to emulate it if the specific target device to which the command was issued, didn't supply the sense data on CHECK CONDITION, and more so relevant to target devices which implemented queuing, thus the ACA.
Quoted 4 lines
And the mid-layer would consider extracting the sense data via REQUEST SENSE command
as a_specialcase_if the LLDD/transport layer didn't implement the "autosense" model.
Only SPI and USB?
The most of LLDs using the transport protocol that we care about today uses sense buffer in their own internal structure.
I think that the issue to solve to kill scsi_cmnd:sense_buffer is how to share (or export) such sense buffer with the scsi mid-layer.
For the old transport protocols, we could do something that James said in this thread to to kill scsi_cmnd:sense_buffer.
Quoted 51 lines · FUJITA Tomonori
Quoted 1 line · On Mon, 4 Feb 2008 18:39:22 -0800 (PST) Luben Tuikov <[email protected]> wrote:
--- On Mon, 2/4/08, Boaz Harrosh
Quoted 3 lines · <[email protected]> wrote:
Quoted 3 lines
There are 3 usages of sense handling in drivers
1. sense is available in driver internal
structure and is
Quoted 2 lines
Quoted 2 lines
mem-copied to upper level
2. ACHECK_CONDITIONstatus was returned and thedriver
Quoted 2 lines
Quoted 2 lines
uses the
scsi_eh_prep_cmnd()
for aREQUEST_SENSEinvocation to the target.Then
Quoted 2 lines
Quoted 2 lines
returning the sense in
scsi_eh_return_cmnd(). A variation on this iswhen the
Quoted 2 lines
Quoted 2 lines
driver does nothing the queue
is frozen an the scsi watchdog timer does theabove.
Quoted 1 line
Quoted 1 line
3. The underline host adapter does the
REQUEST_SENSEand aQuoted 2 lines
Quoted 2 lines
pre-allocated and DMA mapped
sense buffer receives the sense informationfrom HW.
Quoted 2 lines
Many years ago when "ACA" had a constructive
meaning,
Quoted 1 line
so did "Autosense". Then about 5 years ago,
"Autosense"
Quoted 2 lines
disappeared completely since it became the de facto implementation of the then SCSI Execute Command
"RPC",
Quoted 10 lines
now just SCSI Execute Command procedure call.
At that point in time, the SCSI mid-layer decided to embrace this model and give the LLDD a scsi command structure which included the sense data buffer to a size that the SCSI mid-layer was interested in, at the moment 96 bytes, macro defined in
include/scsi/scsi_cmnd.h.The concept of "Autosense" was off-loaded to
LLDD
Quoted 6 lines
to emulate it if the specific target device to which the command was issued, didn't supply the sense data on CHECK CONDITION, and more so relevant to target devices which implemented queuing, thus the ACA.
Quoted 6 lines
Quoted 4 lines
And the mid-layer would consider extracting the sense data via REQUEST SENSE command
as a_specialcase_if the LLDD/transport layer didn't implement the "autosense" model.Only SPI and USB?
I don't understand this question.
Quoted 4 lines
The most of LLDs using the transport protocol that we care about today
uses sense buffer in their own internal structure.
Yes.
Quoted 5 lines
I think that the issue to solve to kill scsi_cmnd:sense_buffer is how
to share (or export) such sense buffer with the scsi mid-layer.
And therein lies the problem. Sense data is SCSI specific, it should be left to SCSI, unless of course you can stipulate that _all_ block devices return sense data. If that's not the case and you move it to the block layer, then you get a whole bunch of other problems, like does this device want/use it, should we allocate it, etc. OTOH, if that _is_ the case, then you don't have to worry about this and the model is pretty much as the SCSI mid-layer has it, i.e. sense buffer always present. So I guess the question is, can you stipulate that _all_ block devices return sense data?
Luben
Quoted 52 lines · On Tue, 5 Feb 2008 11:43:58 -0800 (PST) Luben Tuikov <[email protected]> wrote:
Quoted 51 lines · FUJITA Tomonori
Quoted 1 line · On Mon, 4 Feb 2008 18:39:22 -0800 (PST) Luben Tuikov <[email protected]> wrote:
--- On Mon, 2/4/08, Boaz Harrosh
Quoted 3 lines · <[email protected]> wrote:
Quoted 3 lines
There are 3 usages of sense handling in drivers
1. sense is available in driver internal
structure and is
Quoted 2 lines
Quoted 2 lines
mem-copied to upper level
2. ACHECK_CONDITIONstatus was returned and thedriver
Quoted 2 lines
Quoted 2 lines
uses the
scsi_eh_prep_cmnd()
for aREQUEST_SENSEinvocation to the target.Then
Quoted 2 lines
Quoted 2 lines
returning the sense in
scsi_eh_return_cmnd(). A variation on this iswhen the
Quoted 2 lines
Quoted 2 lines
driver does nothing the queue
is frozen an the scsi watchdog timer does theabove.
Quoted 1 line
Quoted 1 line
3. The underline host adapter does the
REQUEST_SENSEand aQuoted 2 lines
Quoted 2 lines
pre-allocated and DMA mapped
sense buffer receives the sense informationfrom HW.
Quoted 2 lines
Many years ago when "ACA" had a constructive
meaning,
Quoted 1 line
so did "Autosense". Then about 5 years ago,
"Autosense"
Quoted 2 lines
disappeared completely since it became the de facto implementation of the then SCSI Execute Command
"RPC",
Quoted 10 lines
now just SCSI Execute Command procedure call.
At that point in time, the SCSI mid-layer decided to embrace this model and give the LLDD a scsi command structure which included the sense data buffer to a size that the SCSI mid-layer was interested in, at the moment 96 bytes, macro defined in
include/scsi/scsi_cmnd.h.The concept of "Autosense" was off-loaded to
LLDD
Quoted 6 lines
to emulate it if the specific target device to which the command was issued, didn't supply the sense data on CHECK CONDITION, and more so relevant to target devices which implemented queuing, thus the ACA.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
And the mid-layer would consider extracting the sense data via REQUEST SENSE command
as a_specialcase_if the LLDD/transport layer didn't implement the "autosense" model.Only SPI and USB?
I don't understand this question.
I meant, 'what transport protocols are categorized into the transport protocol that doesn't implement the "autosense" model?'
Quoted 7 lines
Quoted 3 lines
The most of LLDs using the transport protocol that we care about today
uses sense buffer in their own internal structure.Yes.
Quoted 1 line
Quoted 8 lines
Quoted 4 lines
I think that the issue to solve to kill scsi_cmnd:sense_buffer is how
to share (or export) such sense buffer with the scsi mid-layer.And therein lies the problem. Sense data is SCSI specific, it should be left to SCSI, unless of course you can stipulate that _all_ block devices return sense data.
Yeah, sense data is SCSI specific and it should be left to SCSI. But I'm not sure we need to stipulate that _all_ block devices return sense data. Today the block layer users (sg, bsg, etc) use it only when it's appropriate (or only if they want to use it).
Quoted 8 lines
If that's not the case and you move it to the block layer, then you get a whole bunch of other problems, like does this device want/use it, should we allocate it, etc. OTOH, if that _is_ the case, then you don't have to worry about this and the model is pretty much as the SCSI mid-layer has it, i.e. sense buffer always present. So I guess the question is, can you stipulate that _all_ block devices return sense data?
Quoted 79 lines · FUJITA Tomonori
Quoted 1 line
--- On Tue, 2/5/08, FUJITA Tomonori
Quoted 6 lines · <[email protected]> wrote:
Quoted 6 lines
Quoted 1 line · On Mon, 4 Feb 2008 18:39:22 -0800 (PST) Luben Tuikov <[email protected]> wrote:
--- On Mon, 2/4/08, Boaz Harrosh
Quoted 1 line · <[email protected]> wrote:
Quoted 1 line
There are 3 usages of sense handling in
drivers
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
1. sense is available in driver
internal
Quoted 3 lines
Quoted 3 lines
structure and is
Quoted 2 lines
Quoted 2 lines
mem-copied to upper level
2. ACHECK_CONDITIONstatus wasreturned and the
Quoted 3 lines
Quoted 3 lines
driver
Quoted 2 lines
Quoted 2 lines
uses the
scsi_eh_prep_cmnd()
for aREQUEST_SENSEinvocation tothe target.
Quoted 3 lines
Quoted 3 lines
Then
Quoted 2 lines
Quoted 2 lines
returning the sense in
scsi_eh_return_cmnd(). A variationon this is
Quoted 3 lines
Quoted 3 lines
when the
Quoted 2 lines
Quoted 2 lines
driver does nothing the queue
is frozen an the scsi watchdog timerdoes the
Quoted 5 lines
Quoted 5 lines
above.
Quoted 1 line
Quoted 1 line
3. The underline host adapter does the
REQUEST_SENSEand aQuoted 2 lines
Quoted 2 lines
pre-allocated and DMA mapped
sense buffer receives the senseinformation
Quoted 3 lines
Quoted 3 lines
from HW.
Quoted 2 lines
Many years ago when "ACA" had a
constructive
Quoted 2 lines
Quoted 2 lines
meaning,
Quoted 1 line
so did "Autosense". Then about 5
years ago,
Quoted 2 lines
Quoted 2 lines
"Autosense"
Quoted 1 line
disappeared completely since it became the
de facto
Quoted 1 line
Quoted 1 line
Quoted 1 line
implementation of the then SCSI Execute
Command
Quoted 2 lines
Quoted 2 lines
"RPC",
Quoted 1 line
now just SCSI Execute Command procedure
call.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
At that point in time, the SCSI mid-layer
decided
Quoted 1 line
Quoted 1 line
Quoted 1 line
to embrace this model and give the LLDD a
scsi command
Quoted 1 line
Quoted 1 line
Quoted 1 line
structure which included the sense data
buffer to
Quoted 1 line
Quoted 1 line
Quoted 1 line
a size that the SCSI mid-layer was
interested in,
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
at the moment 96 bytes, macro defined in
include/scsi/scsi_cmnd.h.The concept of "Autosense" was
off-loaded to
Quoted 2 lines
Quoted 2 lines
LLDD
Quoted 1 line
to emulate it if the specific target device
to
Quoted 1 line
Quoted 1 line
Quoted 1 line
which the command was issued, didn't
supply the
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
sense data on CHECK CONDITION, and more so relevant to target devices which implemented queuing, thus the ACA.
And the mid-layer would consider extracting the sense data via REQUEST SENSE command as a
_specialcase_if the LLDD/transportlayer
Quoted 1 line
Quoted 1 line
Quoted 1 line
didn't implement the
"autosense" model.
Quoted 1 line
Quoted 1 line
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Only SPI and USB?
I don't understand this question.
I meant, 'what transport protocols are categorized into the transport
protocol that doesn't implement the "autosense" model?'
If any transport protocol conforms to SAM, it supports it. Either emulated in the transport itself or supported by the device (target) itself. But ideally, the SCSI mid-layer shouldn't have to get a CHECK CONDITION and then turn around and send REQUEST SENSE, due to the atomicity (per command) of the sense data, especially if the target supports queuing. There used to be a mechanism to support this in SAM but is now obsolete.
Quoted 22 lines
Quoted 3 lines
Quoted 3 lines
The most of LLDs using the transport protocol
that we care
Quoted 2 lines
Quoted 2 lines
about today
uses sense buffer in their own internalstructure.
Quoted 7 lines
Yes.
Quoted 4 lines
I think that the issue to solve to kill
scsi_cmnd:sense_buffer is how
to share (or export) such sense buffer with thescsi
Quoted 3 lines
Quoted 1 line
mid-layer.
And therein lies the problem. Sense data is SCSI
specific,
Quoted 2 lines
it should be left to SCSI, unless of course you can stipulate that _all_ block devices return sense data.
Quoted 7 lines
Yeah, sense data is SCSI specific and it should be left to SCSI. But
I'm not sure we need to stipulate that _all_ block devices return
sense data. Today the block layer users (sg, bsg, etc) use it only
when it's appropriate (or only if they want to use it).
I agree.
Luben