scsi: Convert struct Scsi_Host->cmd_serial_number to atomic_t
Subsystems (6)
PATCH v2 1/11 scsi: Convert struct Scsi_Host->cmd_serial_number to atomic_t
From: Nicholas Bellinger <[email protected]>
This patch converts struct Scsi_Host->cmd_serial_number to an atomic_t so that scsi_cmd_get_serial() can be safely called without struct Scsi_Host->host_lock in scsi_dispatch_cmd(). This patch also adds a struct Scsi_Host->use_serial_number to signal serial_number usage, but this is now disabled by default in scsi_host_alloc().
One special item in scsi_cmd_get_serial() recommended by Joe Eykholt is to start struct Scsi_Host->cmd_serial_number at 1, and increment each serial_number by 2 so that the serial is odd, and wraps to 1 instead of 0.
--- drivers/scsi/hosts.c | 4 ++++ drivers/scsi/scsi.c | 13 ++++++++++--- include/scsi/scsi_host.h | 8 +++++--- 3 files changed, 19 insertions(+), 6 deletions(-)
drivers/scsi/hosts.c +4 -0
@@ -380,6 +380,10 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) shost->unchecked_isa_dma = sht->unchecked_isa_dma; shost->use_clustering = sht->use_clustering; shost->ordered_tag = sht->ordered_tag; +/* +* Set the default shost->cmd_serial_number to 1. +*/ +atomic_set (&shost->cmd_serial_number,1 );if (sht->supported_mode ==MODE_UNKNOWN )/* means we didn't set it ... default to INITIATOR */
drivers/scsi/scsi.c +10 -3
@@ -636,9 +636,16 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)*/ static inline void scsi_cmd_get_serial (struct Scsi_Host *host,struct scsi_cmnd *cmd) { - cmd->serial_number = host->cmd_serial_number++; -if (cmd->serial_number ==0 ) - cmd->serial_number = host->cmd_serial_number++; +/* +* The use of per struct scsi_cmnd->serial_number is disabled by default +*/ +if (!(host->use_serial_number)) +return ; +/* +* Increment the host->cmd_serial_number by 2 so cmd->serial_number +* is always odd and wraps to 1 instead of 0. +*/ + cmd->serial_number =atomic_add_return (2 , &host->cmd_serial_number); }/**
include/scsi/scsi_host.h +5 -3
@@ -602,10 +602,9 @@ struct Scsi_Host {short unsigned int max_sectors;unsigned long dma_boundary;/* -* Used to assign serial numbers to the cmds. -* Protected by the host lock. +* Used to assign serial numbers to the cmds in scsi_cmd_get_serial() */ -unsigned long cmd_serial_number; +atomic_t cmd_serial_number; unsigned active_mode:2; unsigned unchecked_isa_dma:1;
@@ -636,6 +635,9 @@ struct Scsi_Host {/* Asynchronous scan in progress */ unsigned async_scan:1; +/* Signal usage of per struct scsi_cmnd->serial_number */ + unsigned use_serial_number:1; +/* * Optional work queue to be utilized by the transport */
Signature · 1 line
1.7.2.3
Quoted 24 lines · Nicholas A. Bellinger
From: Nicholas Bellinger <[email protected]>
This patch converts struct
Scsi_Host->cmd_serial_numberto anatomic_tso thatscsi_cmd_get_serial()can be safely called without structScsi_Host->host_lockinscsi_dispatch_cmd(). This patch also adds a structScsi_Host->use_serial_numberto signalserial_numberusage, but this is now disabled by default inscsi_host_alloc().One special item in
scsi_cmd_get_serial()recommended by Joe Eykholt is to start structScsi_Host->cmd_serial_numberat 1, and increment eachserial_numberby 2 so that the serial is odd, and wraps to 1 instead of 0.Signed-off-by Nicholas A. Bellinger <[email protected]>--- drivers/scsi/hosts.c | 4 ++++ drivers/scsi/scsi.c | 13 ++++++++++--- include/scsi/scsi_host.h | 8 +++++--- 3 files changed, 19 insertions(+), 6 deletions(-)drivers/scsi/hosts.c +0
-0
drivers/scsi/hosts.c struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) · line 385
Quoted 6 lines
drivers/scsi/hosts.c +3
-0@@ -380,6 +380,10 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) shost->unchecked_isa_dma = sht->unchecked_isa_dma; shost->use_clustering = sht->use_clustering; shost->ordered_tag = sht->ordered_tag; +/* +* Set the default shost->cmd_serial_number to 1. +*/
Comment not necessary.
Quoted 46 lines
drivers/scsi/hosts.c +1
-0+atomic_set (&shost->cmd_serial_number,1 );if (sht->supported_mode ==MODE_UNKNOWN )/* means we didn't set it ... default to INITIATOR */ drivers/scsi/scsi.c +10
-3@@ -636,9 +636,16 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)*/ static inline void scsi_cmd_get_serial (struct Scsi_Host *host,struct scsi_cmnd *cmd) { - cmd->serial_number = host->cmd_serial_number++; -if (cmd->serial_number ==0 ) - cmd->serial_number = host->cmd_serial_number++; +/* +* The use of per struct scsi_cmnd->serial_number is disabled by default +*/ +if (!(host->use_serial_number)) +return ; +/* +* Increment the host->cmd_serial_number by 2 so cmd->serial_number +* is always odd and wraps to 1 instead of 0. +*/ + cmd->serial_number =atomic_add_return (2 , &host->cmd_serial_number); }/** include/scsi/scsi_host.h +2
-3@@ -602,10 +602,9 @@ struct Scsi_Host {short unsigned int max_sectors;unsigned long dma_boundary;/* -* Used to assign serial numbers to the cmds. -* Protected by the host lock. +* Used to assign serial numbers to the cmds in scsi_cmd_get_serial() */ -unsigned long cmd_serial_number; +atomic_t cmd_serial_number; unsigned active_mode:2; unsigned unchecked_isa_dma:1;
include/scsi/scsi_host.h struct Scsi_Host { · line 643
Quoted 9 lines
include/scsi/scsi_host.h +3
-0@@ -636,6 +635,9 @@ struct Scsi_Host {/* Asynchronous scan in progress */ unsigned async_scan:1; +/* Signal usage of per struct scsi_cmnd->serial_number */ + unsigned use_serial_number:1; +/* * Optional work queue to be utilized by the transport */
Simple code is so much fun to critique! So here goes:
This patch would break any drivers that care about serial_numbers because it never sets use_serial_number in any drivers. So that should be done in the same patch so it's bisect-able.
How about instead of adding use_serial_number, let's just have the drivers that want a serial number call scsi_cmd_get_serial()
and stop calling it from scsi_dispatch_cmd()? AFAICT, it's only
used in debug messages in some drivers. I didn't find other usages but didn't do an exhaustive search. If the drivers do it themselves, maybe they're already under a lock when they get the serial number and then we wouldn't need the atomic.
Thanks for using my increment by 2 idea.
Joe
Quoted 20 lines · Joe Eykholt
Quoted 19 lines · Nicholas A. Bellinger
From: Nicholas Bellinger <[email protected]>
This patch converts struct
Scsi_Host->cmd_serial_numberto anatomic_tso thatscsi_cmd_get_serial()can be safely called without structScsi_Host->host_lockinscsi_dispatch_cmd(). This patch also adds a structScsi_Host->use_serial_numberto signalserial_numberusage, but this is now disabled by default inscsi_host_alloc().One special item in
scsi_cmd_get_serial()recommended by Joe Eykholt is to start structScsi_Host->cmd_serial_numberat 1, and increment eachserial_numberby 2 so that the serial is odd, and wraps to 1 instead of 0.Signed-off-by Nicholas A. Bellinger <[email protected]>--- drivers/scsi/hosts.c | 4 ++++ drivers/scsi/scsi.c | 13 ++++++++++--- include/scsi/scsi_host.h | 8 +++++--- 3 files changed, 19 insertions(+), 6 deletions(-)
Quoted 13 lines
drivers/scsi/hosts.c struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) · line 385
Quoted 11 lines
drivers/scsi/hosts.c +3
-0@@ -380,6 +380,10 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) shost->unchecked_isa_dma = sht->unchecked_isa_dma; shost->use_clustering = sht->use_clustering; shost->ordered_tag = sht->ordered_tag; +/* +* Set the default shost->cmd_serial_number to 1. +*/ Comment not necessary.
<nod>
Quoted 59 lines
Quoted 47 lines
drivers/scsi/hosts.c +1
-0+atomic_set (&shost->cmd_serial_number,1 );if (sht->supported_mode ==MODE_UNKNOWN )/* means we didn't set it ... default to INITIATOR */ drivers/scsi/scsi.c +10
-3@@ -636,9 +636,16 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)*/ static inline void scsi_cmd_get_serial (struct Scsi_Host *host,struct scsi_cmnd *cmd) { - cmd->serial_number = host->cmd_serial_number++; -if (cmd->serial_number ==0 ) - cmd->serial_number = host->cmd_serial_number++; +/* +* The use of per struct scsi_cmnd->serial_number is disabled by default +*/ +if (!(host->use_serial_number)) +return ; +/* +* Increment the host->cmd_serial_number by 2 so cmd->serial_number +* is always odd and wraps to 1 instead of 0. +*/ + cmd->serial_number =atomic_add_return (2 , &host->cmd_serial_number); }/** include/scsi/scsi_host.h +2
-3@@ -602,10 +602,9 @@ struct Scsi_Host {short unsigned int max_sectors;unsigned long dma_boundary;/* -* Used to assign serial numbers to the cmds. -* Protected by the host lock. +* Used to assign serial numbers to the cmds in scsi_cmd_get_serial() */ -unsigned long cmd_serial_number; +atomic_t cmd_serial_number; unsigned active_mode:2; unsigned unchecked_isa_dma:1;include/scsi/scsi_host.h struct Scsi_Host { · line 643
Quoted 9 lines
include/scsi/scsi_host.h +3
-0@@ -636,6 +635,9 @@ struct Scsi_Host {/* Asynchronous scan in progress */ unsigned async_scan:1; +/* Signal usage of per struct scsi_cmnd->serial_number */ + unsigned use_serial_number:1; +/* * Optional work queue to be utilized by the transport */ Simple code is so much fun to critique! So here goes:
Quoted 4 lines
This patch would break any drivers that care about
serial_numbersbecause it never setsuse_serial_numberin any drivers. So that should be done in the same patch so it's bisect-able.
<nod>
Quoted 3 lines
How about instead of adding
use_serial_number, let's just have the drivers that want a serial number callscsi_cmd_get_serial()and stop calling it fromscsi_dispatch_cmd()?
This makes sense to me...
Quoted 3 lines
AFAICT, it's only
used in debug messages in some drivers. I didn't find other usages but didn't do an exhaustive search.
This is the same conclusion that I reached when I had a first glance, that ->serial_number is only used for informational purposes in about 1/2 dozen or so mostly old and semi-obsecure LLDs.
James and Co, would this be acceptable to move scsi_cmd_get_serial() directly into LLD SHT->queuecommand() callers for the old drivers that still use struct scsi_cmnd->serial_number for whatever purposes..?
Quoted 4 lines
If the drivers do it themselves,
maybe they're already under a lock when they get the serial number and then we wouldn't need the atomic.
Yeah, not sure on this part yet..
Quoted 1 line
Thanks for using my increment by 2 idea.
Thanks Joe!
--nab
Quoted 3 lines · Joe Eykholt
How about instead of adding
use_serial_number, let's just have the drivers that want a serial number callscsi_cmd_get_serial()
I think this sounds better.
You could also convert drivers to the host tagging if you needed a unique id for each command sent to a host.
Quoted 3 lines
and stop calling it from
scsi_dispatch_cmd()? AFAICT, it's only
used in debug messages in some drivers. I didn't find other usages but didn't do an exhaustive search.
The comments for serial_number say that it is only supposed to be used for debugging printks and most drivers use it for that. However, it looks like mpt and dpt_i2o are using it for error handling and/or lookup type of operations. I think the mpt* uses are not needed in the abort checks.
And eata is using it for ordering and tracking or something. It could probably be converted to the host tagging if or what you suggested if it needs the uniqueue id.
zfcp looks like it copies it. It does not look like the driver needs it.
scsi_error.c uses it in scsi_try_to_abort_cmd to check if a command has completed, but I think that can be done by checking if REQ_ATOM_COMPLETE is set.
Quoted 5 lines · Mike Christie
The comments for
serial_numbersay that it is only supposed to be used for debugging printks and most drivers use it for that. However, it looks like mpt anddpt_i2oare using it for error handling and/or lookup type of operations. I think the mpt* uses are not needed in the abort checks.
Oh yeah, forgot to say, for dpt I think you could just replace the serial_number with the host wide tagging.
Quoted 7 lines · Mike Christie
Quoted 3 lines · Joe Eykholt
How about instead of adding
use_serial_number, let's just have the drivers that want a serial number callscsi_cmd_get_serial()I think this sounds better.
In that case I will go ahead and add explict scsi_cmd_get_serial() calls to the LLDs that use struct scsi_cmnd->serial_number in anything beyond an obvious and simple informational purpose.
Quoted 2 lines
You could also convert drivers to the host tagging if you needed a unique id for each command sent to a host.
Hmmm, what does this entail again..?
Quoted 7 lines
Quoted 4 lines
and stop calling it from
scsi_dispatch_cmd()? AFAICT, it's only
used in debug messages in some drivers. I didn't find other usages but didn't do an exhaustive search.The comments for
serial_numbersay that it is only supposed to be used for debugging printks and most drivers use it for that.
So I would suppose it would be OK for those drivers to continue to printk serial_number to show the internal serial_number allocation is now disabled by default.
Quoted 4 lines
However, it looks like mpt and
dpt_i2oare using it for error handling and/or lookup type of operations. I think the mpt* uses are not needed in the abort checks.
In that case, then adding an explict scsi_cmd_get_serial() call in mpt* and dpt_i20 ->queuecommand() callers would be a good first step.
Quoted 3 lines
And eata is using it for ordering and tracking or something. It could probably be converted to the host tagging if or what you suggested if it needs the uniqueue id.
Adding an explict scsi_cmd_get_serial() for eata for now, and we can consider LLD canidates for host tag conversion as a future step.
Quoted 3 lines
zfcp looks like it copies it. It does not look like the driver needs it.
Ok, I will look at removing it's usage in zfcp or if necessary add an the explict scsi_cmd_get_serial() call.
Quoted 3 lines
scsi_error.cuses it inscsi_try_to_abort_cmdto check if a command has completed, but I think that can be done by checking ifREQ_ATOM_COMPLETEis set.
Hmmm, good catch here. Jejb and hch, does this item work for you..?
If so then I will take another peek for any ML uses of a struct scsi_cmnd->serial_number that need to be addressed, and include Joe's and Mike's recommendations into a v3 series.
Thanks for your comments here Mike!
--nab
Quoted 56 lines · Nicholas A. Bellinger
Quoted 7 lines · Mike Christie
Quoted 3 lines · Joe Eykholt
How about instead of adding
use_serial_number, let's just have the drivers that want a serial number callscsi_cmd_get_serial()I think this sounds better.
In that case I will go ahead and add explict
scsi_cmd_get_serial()calls to the LLDs that use structscsi_cmnd->serial_numberin anything beyond an obvious and simple informational purpose.Quoted 2 lines
You could also convert drivers to the host tagging if you needed a unique id for each command sent to a host.
Hmmm, what does this entail again..?
Quoted 7 lines
Quoted 4 lines
and stop calling it from
scsi_dispatch_cmd()? AFAICT, it's only
used in debug messages in some drivers. I didn't find other usages but didn't do an exhaustive search.The comments for
serial_numbersay that it is only supposed to be used for debugging printks and most drivers use it for that.So I would suppose it would be OK for those drivers to continue to printk
serial_numberto show the internalserial_numberallocation is now disabled by default.Quoted 4 lines
However, it looks like mpt and
dpt_i2oare using it for error handling and/or lookup type of operations. I think the mpt* uses are not needed in the abort checks.In that case, then adding an explict
scsi_cmd_get_serial()call in mpt* anddpt_i20->queuecommand() callers would be a good first step.Quoted 3 lines
And eata is using it for ordering and tracking or something. It could probably be converted to the host tagging if or what you suggested if it needs the uniqueue id.
Adding an explict
scsi_cmd_get_serial()for eata for now, and we can consider LLD canidates for host tag conversion as a future step.Quoted 3 lines
zfcp looks like it copies it. It does not look like the driver needs it.
Ok, I will look at removing it's usage in zfcp or if necessary add an the explict
scsi_cmd_get_serial()call.Quoted 3 lines
scsi_error.cuses it inscsi_try_to_abort_cmdto check if a command has completed, but I think that can be done by checking ifREQ_ATOM_COMPLETEis set.
Quoted 6 lines
Hmmm, good catch here. Jejb and hch, does this item work for you..?
If so then I will take another peek for any ML uses of a struct
scsi_cmnd->serial_numberthat need to be addressed, and include Joe's and Mike's recommendations into a v3 series.
Greetings Mike and Co,
I was doing some followup on these items for a v3 series and started with a patch following mnc's recommendations for dropping the scsi_error.c codes depending upon struct scsi_cmnd->serial_number:
drivers/scsi/scsi_error.c +1 -5
@@ -644,11 +644,7 @@ static int __scsi_try_to_abort_cmd(struct scsi_cmnd *scmd)*/ static int scsi_try_to_abort_cmd (struct scsi_cmnd *scmd) { -/* -* scsi_done was called just after the command timed out and before -* we had a chance to process it. (db) -*/ -if (scmd->serial_number ==0 ) +if (test_bit (REQ_ATOM_COMPLETE , &scmd->request->atomic_flags))return SUCCESS ;return __scsi_try_to_abort_cmd (scmd); }
and while building I noticed that the simple single enum REQ_ATOM_COMPLETE=0 is currently located in block/blk.h and along with blk_mark_rq_complete() and blk_clear_rq_complete() for setting this bit within struct request->atomic_flags.
jens, hch, tejun, and co, do you guys have a preference how this should be handled so that scsi_try_to_abort_cmd() can use proper atomic struct request bits here and we can get rid of this (racy..?) method of using struct scsi_cmnd->serial_number for anything wrt to per struct scsi_cmnd context timeout handling.
Thanks!
--nab
Quoted 95 lines · Nicholas A. Bellinger
Quoted 56 lines · Nicholas A. Bellinger
Quoted 7 lines · Mike Christie
Quoted 3 lines · Joe Eykholt
How about instead of adding
use_serial_number, let's just have the drivers that want a serial number callscsi_cmd_get_serial()I think this sounds better.
In that case I will go ahead and add explict
scsi_cmd_get_serial()calls to the LLDs that use structscsi_cmnd->serial_numberin anything beyond an obvious and simple informational purpose.Quoted 2 lines
You could also convert drivers to the host tagging if you needed a unique id for each command sent to a host.
Hmmm, what does this entail again..?
Quoted 7 lines
Quoted 4 lines
and stop calling it from
scsi_dispatch_cmd()? AFAICT, it's only
used in debug messages in some drivers. I didn't find other usages but didn't do an exhaustive search.The comments for
serial_numbersay that it is only supposed to be used for debugging printks and most drivers use it for that.So I would suppose it would be OK for those drivers to continue to printk
serial_numberto show the internalserial_numberallocation is now disabled by default.Quoted 4 lines
However, it looks like mpt and
dpt_i2oare using it for error handling and/or lookup type of operations. I think the mpt* uses are not needed in the abort checks.In that case, then adding an explict
scsi_cmd_get_serial()call in mpt* anddpt_i20->queuecommand() callers would be a good first step.Quoted 3 lines
And eata is using it for ordering and tracking or something. It could probably be converted to the host tagging if or what you suggested if it needs the uniqueue id.
Adding an explict
scsi_cmd_get_serial()for eata for now, and we can consider LLD canidates for host tag conversion as a future step.Quoted 3 lines
zfcp looks like it copies it. It does not look like the driver needs it.
Ok, I will look at removing it's usage in zfcp or if necessary add an the explict
scsi_cmd_get_serial()call.Quoted 3 lines
scsi_error.cuses it inscsi_try_to_abort_cmdto check if a command has completed, but I think that can be done by checking ifREQ_ATOM_COMPLETEis set.Quoted 6 lines
Hmmm, good catch here. Jejb and hch, does this item work for you..?
If so then I will take another peek for any ML uses of a struct
scsi_cmnd->serial_numberthat need to be addressed, and include Joe's and Mike's recommendations into a v3 series.Greetings Mike and Co,
I was doing some followup on these items for a v3 series and started with a patch following mnc's recommendations for dropping the
scsi_error.ccodes depending upon structscsi_cmnd->serial_number:drivers/scsi/scsi_error.c +1
-5@@ -644,11 +644,7 @@ static int __scsi_try_to_abort_cmd(struct scsi_cmnd *scmd)*/ static int scsi_try_to_abort_cmd (struct scsi_cmnd *scmd) { -/* -* scsi_done was called just after the command timed out and before -* we had a chance to process it. (db) -*/ -if (scmd->serial_number ==0 ) +if (test_bit (REQ_ATOM_COMPLETE , &scmd->request->atomic_flags))return SUCCESS ;return __scsi_try_to_abort_cmd (scmd); }and while building I noticed that the simple single enum REQ_ATOM_COMPLETE=0 is
currently located in
block/blk.hand along withblk_mark_rq_complete()andblk_clear_rq_complete()for setting this bit within structrequest->atomic_flags.
Quoted 5 lines
jens, hch, tejun, and co, do you guys have a preference how this should be handled so that
scsi_try_to_abort_cmd()can use proper atomic struct request bits here and we can get rid of this (racy..?) method of using structscsi_cmnd->serial_numberfor anything wrt to per structscsi_cmndcontext timeout handling.
Just add a
static inline int blk_test_rq_complete(struct request *rq)
{
return test_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags);
}in block/blk.h
I need this too for some lockless completion patches I am playing with.
Signature · 1 line
Jens Axboe
Quoted 104 lines · Jens Axboe
Quoted 95 lines · Nicholas A. Bellinger
Quoted 56 lines · Nicholas A. Bellinger
Quoted 7 lines · Mike Christie
Quoted 3 lines · Joe Eykholt
How about instead of adding
use_serial_number, let's just have the drivers that want a serial number callscsi_cmd_get_serial()I think this sounds better.
In that case I will go ahead and add explict
scsi_cmd_get_serial()calls to the LLDs that use structscsi_cmnd->serial_numberin anything beyond an obvious and simple informational purpose.Quoted 2 lines
You could also convert drivers to the host tagging if you needed a unique id for each command sent to a host.
Hmmm, what does this entail again..?
Quoted 7 lines
Quoted 4 lines
and stop calling it from
scsi_dispatch_cmd()? AFAICT, it's only
used in debug messages in some drivers. I didn't find other usages but didn't do an exhaustive search.The comments for
serial_numbersay that it is only supposed to be used for debugging printks and most drivers use it for that.So I would suppose it would be OK for those drivers to continue to printk
serial_numberto show the internalserial_numberallocation is now disabled by default.Quoted 4 lines
However, it looks like mpt and
dpt_i2oare using it for error handling and/or lookup type of operations. I think the mpt* uses are not needed in the abort checks.In that case, then adding an explict
scsi_cmd_get_serial()call in mpt* anddpt_i20->queuecommand() callers would be a good first step.Quoted 3 lines
And eata is using it for ordering and tracking or something. It could probably be converted to the host tagging if or what you suggested if it needs the uniqueue id.
Adding an explict
scsi_cmd_get_serial()for eata for now, and we can consider LLD canidates for host tag conversion as a future step.Quoted 3 lines
zfcp looks like it copies it. It does not look like the driver needs it.
Ok, I will look at removing it's usage in zfcp or if necessary add an the explict
scsi_cmd_get_serial()call.Quoted 3 lines
scsi_error.cuses it inscsi_try_to_abort_cmdto check if a command has completed, but I think that can be done by checking ifREQ_ATOM_COMPLETEis set.Quoted 6 lines
Hmmm, good catch here. Jejb and hch, does this item work for you..?
If so then I will take another peek for any ML uses of a struct
scsi_cmnd->serial_numberthat need to be addressed, and include Joe's and Mike's recommendations into a v3 series.Greetings Mike and Co,
I was doing some followup on these items for a v3 series and started with a patch following mnc's recommendations for dropping the
scsi_error.ccodes depending upon structscsi_cmnd->serial_number:drivers/scsi/scsi_error.c +1
-5@@ -644,11 +644,7 @@ static int __scsi_try_to_abort_cmd(struct scsi_cmnd *scmd)*/ static int scsi_try_to_abort_cmd (struct scsi_cmnd *scmd) { -/* -* scsi_done was called just after the command timed out and before -* we had a chance to process it. (db) -*/ -if (scmd->serial_number ==0 ) +if (test_bit (REQ_ATOM_COMPLETE , &scmd->request->atomic_flags))return SUCCESS ;return __scsi_try_to_abort_cmd (scmd); }and while building I noticed that the simple single enum REQ_ATOM_COMPLETE=0 is
currently located in
block/blk.hand along withblk_mark_rq_complete()andblk_clear_rq_complete()for setting this bit within structrequest->atomic_flags.Quoted 5 lines
jens, hch, tejun, and co, do you guys have a preference how this should be handled so that
scsi_try_to_abort_cmd()can use proper atomic struct request bits here and we can get rid of this (racy..?) method of using structscsi_cmnd->serial_numberfor anything wrt to per structscsi_cmndcontext timeout handling.Just add a
Quoted 8 lines
static inline int
blk_test_rq_complete(struct request *rq){ return test_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags); }in
block/blk.hI need this too for some lockless completion patches I am playing with.
In reviewing the current code paths wasn't the serial_number also used to avoid calling __scsi_try_to_abort_cmd for START_UNIT case also.
If we skip __scsi_try_to_abort_cmd when REQ_ATOM_COMPLETE is set it would be correct for the scsi_decide_disposition cases but it would appear this would stop __scsi_try_to_abort_cmd from being called in the time out case as REQ_ATOM_COMPLETE is set prior to calling blk_rq_timed_out.
1.) Request timed out path to scsi_eh_scmd_add.
blk_rq_timed_out_timer
... if (blk_mark_rq_complete(rq)) continue; blk_rq_timed_out q->rq_timed_out_fn "scsi_times_out" scsi_times_out scsi_eh_scmd_add
2.) Request completion path to scsi_eh_scmd_add based on scsi_decide_disposition disposition set to FAILED (actually scsi_eh_scmd_add called from the default case with the disposition of FAILED appearing to be the only disposition returned that would hit this case vs. the other three handled cases). This should not be a common path outside of handling allow_restart.
blk_complete_request
... if (!blk_mark_rq_complete(req)) __blk_complete_request ... raise_softirq_irqoff(BLOCK_SOFTIRQ); blk_done_softirq rq->q->softirq_done_fn "scsi_softirq_done" scsi_softirq_done scsi_eh_scmd_add
3.) Call path to scsi_try_to_abort_cmd.scsi_error_handler
scsi_unjam_host
scsi_eh_abort_cmds
scsi_try_to_abort_cmd
Thanks,
-andmike
--
Michael Anderson
[email protected]
Quoted 4 lines · Mike Anderson
Quoted 3 lines · Jens Axboe
Quoted 2 lines · Nicholas A. Bellinger
Quoted 1 line · Nicholas A. Bellinger
On Fri, 2010-09-17 at 21:45 -0500, Mike Christie wrote:
<SNIP>
Quoted 39 lines
Quoted 39 lines
Quoted 31 lines
Greetings Mike and Co,
I was doing some followup on these items for a v3 series and started with a patch following mnc's recommendations for dropping the
scsi_error.ccodes depending upon structscsi_cmnd->serial_number:drivers/scsi/scsi_error.c +1
-5@@ -644,11 +644,7 @@ static int __scsi_try_to_abort_cmd(struct scsi_cmnd *scmd)*/ static int scsi_try_to_abort_cmd (struct scsi_cmnd *scmd) { -/* -* scsi_done was called just after the command timed out and before -* we had a chance to process it. (db) -*/ -if (scmd->serial_number ==0 ) +if (test_bit (REQ_ATOM_COMPLETE , &scmd->request->atomic_flags))return SUCCESS ;return __scsi_try_to_abort_cmd (scmd); }and while building I noticed that the simple single enum REQ_ATOM_COMPLETE=0 is
currently located in
block/blk.hand along withblk_mark_rq_complete()andblk_clear_rq_complete()for setting this bit within structrequest->atomic_flags.Quoted 5 lines
jens, hch, tejun, and co, do you guys have a preference how this should be handled so that
scsi_try_to_abort_cmd()can use proper atomic struct request bits here and we can get rid of this (racy..?) method of using structscsi_cmnd->serial_numberfor anything wrt to per structscsi_cmndcontext timeout handling.Just add a
Quoted 9 lines
Quoted 8 lines
static inline int
blk_test_rq_complete(struct request *rq){ return test_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags); }in
block/blk.hI need this too for some lockless completion patches I am playing with.
Greetings Mike,
Quoted 2 lines
In reviewing the current code paths wasn't the
serial_numberalso used to avoid calling__scsi_try_to_abort_cmdforSTART_UNITcase also.
Not sure on this item, but checking on this now..
Quoted 5 lines
If we skip
__scsi_try_to_abort_cmdwhenREQ_ATOM_COMPLETEis set it would be correct for thescsi_decide_dispositioncases but it would appear this would stop__scsi_try_to_abort_cmdfrom being called in the time out case asREQ_ATOM_COMPLETEis set prior to callingblk_rq_timed_out.
Hmmmmmmm..
Quoted 30 lines
1.) Request timed out path to
scsi_eh_scmd_add.
blk_rq_timed_out_timer... if (blk_mark_rq_complete(rq)) continue; blk_rq_timed_out q->rq_timed_out_fn "scsi_times_out" scsi_times_out scsi_eh_scmd_add2.) Request completion path to
scsi_eh_scmd_addbased onscsi_decide_dispositiondisposition set to FAILED (actuallyscsi_eh_scmd_addcalled from the default case with the disposition of FAILED appearing to be the only disposition returned that would hit this case vs. the other three handled cases). This should not be a common path outside of handlingallow_restart.
blk_complete_request... if (!blk_mark_rq_complete(req)) __blk_complete_request ... raise_softirq_irqoff(BLOCK_SOFTIRQ); blk_done_softirq rq->q->softirq_done_fn "scsi_softirq_done" scsi_softirq_done scsi_eh_scmd_add
Quoted 7 lines
3.) Call path to
scsi_try_to_abort_cmd.scsi_error_handler
scsi_unjam_host
scsi_eh_abort_cmds
scsi_try_to_abort_cmd
Thanks,
Ok, what I think is being said here is that the usage of blk_test_rq_complete() in scsi_try_to_abort_cmd() completly breaks __scsi_try_to_abort_cmd from being called from within the struct request timeout handler, right..?
Not being exquistiely fimilar with the scsi_error.c generic LLD logic w/ struct request timeouts code, I assume this means we need to revisit scsi_unjam_host() once again to take into account the new blk_test_rq_complete() calls from the normal fast path block softirq in order to be able to handle the individual struct request timeout cases for outstanding struct scsi_cmnd when they individual timer contexts fire.
Perhaps we should be setting another bit in struct request->atomic_flags to signal the REQ_BLKSOFTIRQ_COMPLETE to let scsi_unjam_host() know what is going on..?
Thanks for your comments Mike!
--nab
Quoted 2 lines · Mike Anderson
In reviewing the current code paths wasn't the
serial_numberalso used to avoid calling__scsi_try_to_abort_cmdforSTART_UNITcase also.
You mean from scsi_eh_try_stu....->__scsi_try_to_abort_cmd? How does it work for that case or what is the code path? Is it when it is called from the sas code through scsi_eh_ready_devs?
Quoted 8 lines
If we skip
__scsi_try_to_abort_cmdwhenREQ_ATOM_COMPLETEis set it would be correct for thescsi_decide_dispositioncases but it would appear this would stop__scsi_try_to_abort_cmdfrom being called in the time out case asREQ_ATOM_COMPLETEis set prior to callingblk_rq_timed_out.1.) Request timed out path to
scsi_eh_scmd_add.
Quoted 9 lines
blk_rq_timed_out_timer... if (blk_mark_rq_complete(rq)) continue; blk_rq_timed_out q->rq_timed_out_fn "scsi_times_out" scsi_times_out scsi_eh_scmd_add
You are right.
Quoted 2 lines · Jens Axboe
Quoted 1 line · Nicholas A. Bellinger
On Sat, 2010-09-18 at 12:58 -0700, Nicholas A. Bellinger wrote:
<SNIP>
Quoted 33 lines
Quoted 25 lines
drivers/scsi/scsi_error.c +1
-5@@ -644,11 +644,7 @@ static int __scsi_try_to_abort_cmd(struct scsi_cmnd *scmd)*/ static int scsi_try_to_abort_cmd (struct scsi_cmnd *scmd) { -/* -* scsi_done was called just after the command timed out and before -* we had a chance to process it. (db) -*/ -if (scmd->serial_number ==0 ) +if (test_bit (REQ_ATOM_COMPLETE , &scmd->request->atomic_flags))return SUCCESS ;return __scsi_try_to_abort_cmd (scmd); }and while building I noticed that the simple single enum REQ_ATOM_COMPLETE=0 is
currently located in
block/blk.hand along withblk_mark_rq_complete()andblk_clear_rq_complete()for setting this bit within structrequest->atomic_flags.Quoted 5 lines
jens, hch, tejun, and co, do you guys have a preference how this should be handled so that
scsi_try_to_abort_cmd()can use proper atomic struct request bits here and we can get rid of this (racy..?) method of using structscsi_cmnd->serial_numberfor anything wrt to per structscsi_cmndcontext timeout handling.Just add a
Quoted 9 lines
static inline int
blk_test_rq_complete(struct request *rq){ return test_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags); }in
block/blk.hI need this too for some lockless completion patches I am playing with.
Hi Jens,
Perfect, thanks for the heads up on this wrapper. I will go ahead and include this into a RFCv4 series and update scsi_try_to_abort_cmd() accordingly.
Best,
--nab
Quoted 46 lines · Nicholas A. Bellinger
Quoted 7 lines · Mike Christie
Quoted 3 lines · Joe Eykholt
How about instead of adding
use_serial_number, let's just have the drivers that want a serial number callscsi_cmd_get_serial()I think this sounds better.
In that case I will go ahead and add explict
scsi_cmd_get_serial()calls to the LLDs that use structscsi_cmnd->serial_numberin anything beyond an obvious and simple informational purpose.Quoted 2 lines
You could also convert drivers to the host tagging if you needed a unique id for each command sent to a host.
Hmmm, what does this entail again..?
Quoted 7 lines
Quoted 4 lines
and stop calling it from
scsi_dispatch_cmd()? AFAICT, it's only
used in debug messages in some drivers. I didn't find other usages but didn't do an exhaustive search.The comments for
serial_numbersay that it is only supposed to be used for debugging printks and most drivers use it for that.So I would suppose it would be OK for those drivers to continue to printk
serial_numberto show the internalserial_numberallocation is now disabled by default.Quoted 4 lines
However, it looks like mpt and
dpt_i2oare using it for error handling and/or lookup type of operations. I think the mpt* uses are not needed in the abort checks.In that case, then adding an explict
scsi_cmd_get_serial()call in mpt* anddpt_i20->queuecommand() callers would be a good first step.Quoted 3 lines
And eata is using it for ordering and tracking or something. It could probably be converted to the host tagging if or what you suggested if it needs the uniqueue id.
Adding an explict
scsi_cmd_get_serial()for eata for now, and we can consider LLD canidates for host tag conversion as a future step.Quoted 1 line
Quoted 5 lines
Quoted 2 lines
zfcp looks like it copies it. It does not look like the driver needs it.
Ok, I will look at removing it's usage in zfcp or if necessary add an the explict
scsi_cmd_get_serial()call.
In zfcp, the serial_number is only accessed for writing it to debug traces. With the change that the driver has to request the serial number through use_serial_number, simply remove the serial number from zfcp.
Thanks,
Christof
Quoted 14 lines
Quoted 4 lines
scsi_error.cuses it inscsi_try_to_abort_cmdto check if a command has completed, but I think that can be done by checking ifREQ_ATOM_COMPLETEis set.Hmmm, good catch here. Jejb and hch, does this item work for you..?
If so then I will take another peek for any ML uses of a struct
scsi_cmnd->serial_numberthat need to be addressed, and include Joe's and Mike's recommendations into a v3 series.Thanks for your comments here Mike!
--nab
Quoted 31 lines · Christof Schmitt
Quoted 30 lines · Nicholas A. Bellinger
Quoted 7 lines · Mike Christie
Quoted 3 lines · Joe Eykholt
How about instead of adding
use_serial_number, let's just have the drivers that want a serial number callscsi_cmd_get_serial()I think this sounds better.
In that case I will go ahead and add explict
scsi_cmd_get_serial()calls to the LLDs that use structscsi_cmnd->serial_numberin anything beyond an obvious and simple informational purpose.Quoted 2 lines
You could also convert drivers to the host tagging if you needed a unique id for each command sent to a host.
Hmmm, what does this entail again..?
Quoted 7 lines
Quoted 4 lines
and stop calling it from
scsi_dispatch_cmd()? AFAICT, it's only
used in debug messages in some drivers. I didn't find other usages but didn't do an exhaustive search.The comments for
serial_numbersay that it is only supposed to be used for debugging printks and most drivers use it for that.So I would suppose it would be OK for those drivers to continue to printk
serial_numberto show the internalserial_numberallocation is now disabled by default.
<SNIP>
Quoted 11 lines
Quoted 6 lines
Quoted 3 lines
zfcp looks like it copies it. It does not look like the driver needs it.
Ok, I will look at removing it's usage in zfcp or if necessary add an the explict
scsi_cmd_get_serial()call.In zfcp, the
serial_numberis only accessed for writing it to debug traces. With the change that the driver has to request the serial number throughuse_serial_number, simply remove the serial number from zfcp.
Hi Christof,
So since the use of struct scsi_cmnd->serial_number is purely informational for some LLDs (including zfcp) , I have been putting the
conversion of these LLDs as a low priority ATM. I would happily accept
a patch to remove these. 8-)
Best,
--nab
PATCH 1/1 zfcp: Remove scsi_cmnd->serial_number from debug traces
From: Christof Schmitt <[email protected]>
With the change that drivers have to explicitly request the serial number for SCSI commands, this field should not be part of the zfcp traces. It is not worth the effort to request the serial number only for tracing purposes, so simply remove this field from the debug traces.
--- drivers/s390/scsi/zfcp_dbf.c | 4 ---- drivers/s390/scsi/zfcp_dbf.h | 2 -- 2 files changed, 6 deletions(-)
drivers/s390/scsi/zfcp_dbf.c +0 -4
@@ -154,7 +154,6 @@ void _zfcp_dbf_hba_fsf_response(const ch scsi_cmnd = (struct scsi_cmnd *)fsf_req->data;if (scsi_cmnd) { response->u.fcp.cmnd = (unsigned long )scsi_cmnd; - response->u.fcp.serial = scsi_cmnd->serial_number; response->u.fcp.data_dir = qtcb->bottom.io.data_direction; }
@@ -330,7 +329,6 @@ static void zfcp_dbf_hba_view_response(cbreak ;zfcp_dbf_out (p,"data_direction" ,"0x%04x" , r->u.fcp.data_dir);zfcp_dbf_out (p,"scsi_cmnd" ,"0x%0Lx" , r->u.fcp.cmnd); -zfcp_dbf_out (p,"scsi_serial" ,"0x%016Lx" , r->u.fcp.serial); *p +=sprintf (*p,"\n" );break ;
@@ -881,7 +879,6 @@ void _zfcp_dbf_scsi(const char *tag, con } rec->scsi_result = scsi_cmnd->result; rec->scsi_cmnd = (unsigned long )scsi_cmnd; - rec->scsi_serial = scsi_cmnd->serial_number;memcpy (rec->scsi_opcode, scsi_cmnd->cmnd,min ((int )scsi_cmnd->cmd_len,ZFCP_DBF_SCSI_OPCODE ));
@@ -950,7 +947,6 @@ static int zfcp_dbf_scsi_view_format(debzfcp_dbf_out (&p,"scsi_lun" ,"0x%08x" , r->scsi_lun);zfcp_dbf_out (&p,"scsi_result" ,"0x%08x" , r->scsi_result);zfcp_dbf_out (&p,"scsi_cmnd" ,"0x%0Lx" , r->scsi_cmnd); -zfcp_dbf_out (&p,"scsi_serial" ,"0x%016Lx" , r->scsi_serial);zfcp_dbf_outd (&p,"scsi_opcode" , r->scsi_opcode,ZFCP_DBF_SCSI_OPCODE ,0 ,ZFCP_DBF_SCSI_OPCODE );zfcp_dbf_out (&p,"scsi_retries" ,"0x%02x" , r->scsi_retries);
drivers/s390/scsi/zfcp_dbf.h +0 -2
@@ -110,7 +110,6 @@ struct zfcp_dbf_hba_record_response {union {struct {u64 cmnd; -u64 serial;u32 data_dir; } fcp;struct {
@@ -206,7 +205,6 @@ struct zfcp_dbf_scsi_record {u32 scsi_lun;u32 scsi_result;u64 scsi_cmnd; -u64 scsi_serial;#define ZFCP_DBF_SCSI_OPCODE 16u8 scsi_opcode[ZFCP_DBF_SCSI_OPCODE ];u8 scsi_retries;
Re: [PATCH] zfcp: Remove scsi_cmnd->serial_number from debug traces
Quoted 11 lines · Christof Schmitt
From: Christof Schmitt <[email protected]>
With the change that drivers have to explicitly request the serial number for SCSI commands, this field should not be part of the zfcp traces. It is not worth the effort to request the serial number only for tracing purposes, so simply remove this field from the debug traces.
Reviewed-by Swen Schillig <[email protected]>Signed-off-by Christof Schmitt <[email protected]>---
Hi Christof,
This patch has been commited into the drop-host_lock-v5 branch and will be included in the next RFC posting.
Thanks!
--nab
Quoted 56 lines
drivers/s390/scsi/zfcp_dbf.c | 4 ---- drivers/s390/scsi/zfcp_dbf.h | 2 -- 2 files changed, 6 deletions(-)drivers/s390/scsi/zfcp_dbf.c +0
-4@@ -154,7 +154,6 @@ void _zfcp_dbf_hba_fsf_response(const ch scsi_cmnd = (struct scsi_cmnd *)fsf_req->data;if (scsi_cmnd) { response->u.fcp.cmnd = (unsigned long )scsi_cmnd; - response->u.fcp.serial = scsi_cmnd->serial_number; response->u.fcp.data_dir = qtcb->bottom.io.data_direction; }@@ -330,7 +329,6 @@ static void zfcp_dbf_hba_view_response(cbreak ;zfcp_dbf_out (p,"data_direction" ,"0x%04x" , r->u.fcp.data_dir);zfcp_dbf_out (p,"scsi_cmnd" ,"0x%0Lx" , r->u.fcp.cmnd); -zfcp_dbf_out (p,"scsi_serial" ,"0x%016Lx" , r->u.fcp.serial); *p +=sprintf (*p,"\n" );break ;@@ -881,7 +879,6 @@ void _zfcp_dbf_scsi(const char *tag, con } rec->scsi_result = scsi_cmnd->result; rec->scsi_cmnd = (unsigned long )scsi_cmnd; - rec->scsi_serial = scsi_cmnd->serial_number;memcpy (rec->scsi_opcode, scsi_cmnd->cmnd,min ((int )scsi_cmnd->cmd_len,ZFCP_DBF_SCSI_OPCODE ));@@ -950,7 +947,6 @@ static int zfcp_dbf_scsi_view_format(debzfcp_dbf_out (&p,"scsi_lun" ,"0x%08x" , r->scsi_lun);zfcp_dbf_out (&p,"scsi_result" ,"0x%08x" , r->scsi_result);zfcp_dbf_out (&p,"scsi_cmnd" ,"0x%0Lx" , r->scsi_cmnd); -zfcp_dbf_out (&p,"scsi_serial" ,"0x%016Lx" , r->scsi_serial);zfcp_dbf_outd (&p,"scsi_opcode" , r->scsi_opcode,ZFCP_DBF_SCSI_OPCODE ,0 ,ZFCP_DBF_SCSI_OPCODE );zfcp_dbf_out (&p,"scsi_retries" ,"0x%02x" , r->scsi_retries);drivers/s390/scsi/zfcp_dbf.h +0
-2@@ -110,7 +110,6 @@ struct zfcp_dbf_hba_record_response {union {struct {u64 cmnd; -u64 serial;u32 data_dir; } fcp;struct {@@ -206,7 +205,6 @@ struct zfcp_dbf_scsi_record {u32 scsi_lun;u32 scsi_result;u64 scsi_cmnd; -u64 scsi_serial;#define ZFCP_DBF_SCSI_OPCODE 16u8 scsi_opcode[ZFCP_DBF_SCSI_OPCODE ];u8 scsi_retries;
Quoted 2 lines · Mike Christie
You could also convert drivers to the host tagging if you needed a unique id for each command sent to a host.
I just wanted to make people aware of a problem with the host tagging -- scsi_cmnd->tag is always 0!
Watch this:
scsi_prep_fn:
ret = scsi_setup_blk_pc_cmnd(sdev, req);
scsi_setup_blk_pc_cmnd:
cmd = scsi_get_cmd_from_req(sdev, req);
scsi_get_cmd_from_req:
cmd->tag = req->tag;
scsi_request_fn:
if (!(blk_queue_tagged(q) && !blk_queue_start_tag(q, req))) blk_start_request(req);
blk_queue_start_tag
rq->tag = tag;
(and yes, they're called in this order)
I encountered this while writing the UAS driver, and decided to just use the rq->tag directly instead. I would favour deleting scmd->tag in order to save others the confusion.
In fact, looking at the users of scmd->tag, it appears there is a common misconception that it contains the tag type (HEAD / SIMPLE / ORDERED). I have no idea where this comes from, but it means the following patch to delete scmd's ->tag is almost certainly wrong ... but it preserves the existing behaviour!
drivers/scsi/NCR5380.c +3 -4
@@ -1531,7 +1531,6 @@ part2: tmp[0 ] =IDENTIFY (((instance->irq ==SCSI_IRQ_NONE ) ?0 :1 ), cmd->device->lun); len =1 ; - cmd->tag =0 ;/* Send message(s) */ data = tmp;
@@ -2240,7 +2239,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance) { }initialize_SCp (cmd->next_link);/* The next command is still part of this process */ - cmd->next_link->tag = cmd->tag; + cmd->next_link->tag =0 ; cmd->result = cmd->SCp.Status | (cmd->SCp.Message <<8 );dprintk (NDEBUG_LINKED , ("scsi%d : target %d lun %d linked request done, calling scsi_done().\n" , instance->host_no, cmd->device->id, cmd->device->lun));collect_stats (hostdata, cmd);
@@ -2604,7 +2603,7 @@ static void NCR5380_reselect(struct Scsi_Host *instance) {do_abort (instance); }else { hostdata->connected = tmp; -dprintk (NDEBUG_RESELECTION , ("scsi%d : nexus established, target = %d, lun = %d, tag = %d\n" , instance->host_no, tmp->target, tmp->lun, tmp->tag)); +dprintk (NDEBUG_RESELECTION , ("scsi%d : nexus established, target = %d, lun = %d, tag = %d\n" , instance->host_no, tmp->target, tmp->lun,0 )); } }
@@ -2789,7 +2788,7 @@ static int NCR5380_abort(Scsi_Cmnd * cmd) {if (cmd == tmp) {dprintk (NDEBUG_ABORT , ("scsi%d : aborting disconnected command.\n" , instance->host_no)); -if (NCR5380_select (instance, cmd, (int ) cmd->tag)) +if (NCR5380_select (instance, cmd,0 ))return FAILED ;dprintk (NDEBUG_ABORT , ("scsi%d : nexus reestablished.\n" , instance->host_no));
drivers/scsi/bfa/bfa_cb_ioim_macros.h +2 -13
@@ -143,19 +143,8 @@ bfa_cb_ioim_get_taskattr(struct bfad_ioim_s *dio)struct scsi_cmnd *cmnd = (struct scsi_cmnd *)dio;u8 task_attr =UNTAGGED ; -if (cmnd->device->tagged_supported) { -switch (cmnd->tag) { -case HEAD_OF_QUEUE_TAG : - task_attr =HEAD_OF_Q ; -break ; -case ORDERED_QUEUE_TAG : - task_attr =ORDERED_Q ; -break ; -default : - task_attr =SIMPLE_Q ; -break ; - } - } +if (cmnd->device->tagged_supported) + task_attr =SIMPLE_Q ;return task_attr; }
drivers/scsi/libsrp.c +1 -1
@@ -431,7 +431,7 @@ int srp_cmd_queue(struct Scsi_Host *shost, struct srp_cmd *cmd, void *info,memcpy (sc->cmnd, cmd->cdb,MAX_COMMAND_SIZE ); sc->sdb.length = len; sc->sdb.table.sgl = (void *) (unsigned long ) addr; - sc->tag = tag; + sc->request->tag = tag; err =scsi_tgt_queue_command (sc, itn_id, (struct scsi_lun *)&cmd->lun, cmd->tag);if (err)
drivers/scsi/scsi_lib.c +0 -2
@@ -1031,8 +1031,6 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev, cmd = req->special; } -/* pull a tag out of the request if we have one */ - cmd->tag = req->tag; cmd->request = req; cmd->cmnd = req->cmd;
drivers/scsi/scsi_tgt_if.c +1 -1
@@ -117,7 +117,7 @@ int scsi_tgt_uspace_send_cmd(struct scsi_cmnd *cmd, u64 itn_id, ev.p.cmd_req.data_len =scsi_bufflen (cmd);memcpy (ev.p.cmd_req.scb, cmd->cmnd,sizeof (ev.p.cmd_req.scb));memcpy (ev.p.cmd_req.lun, lun,sizeof (ev.p.cmd_req.lun)); - ev.p.cmd_req.attribute = cmd->tag; + ev.p.cmd_req.attribute =0 ; ev.p.cmd_req.tag = tag; dprintk("%p %d %u %x %llx\n", cmd, shost->host_no,
drivers/scsi/vmw_pvscsi.c +0 -4
@@ -665,10 +665,6 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter,memcpy (e->cdb, cmd->cmnd, e->cdbLen); e->tag =SIMPLE_QUEUE_TAG ; -if (sdev->tagged_supported && - (cmd->tag ==HEAD_OF_QUEUE_TAG || - cmd->tag ==ORDERED_QUEUE_TAG )) - e->tag = cmd->tag;if (cmd->sc_data_direction ==DMA_FROM_DEVICE ) e->flags =PVSCSI_FLAG_CMD_DIR_TOHOST ;
include/scsi/scsi_cmnd.h +0 -2
@@ -127,8 +127,6 @@ struct scsi_cmnd {* to be at an address < 16Mb). */ int result;/* Status code from lower level driver */ - -unsigned char tag;/* SCSI-II queued command tag */ };extern struct scsi_cmnd *scsi_get_command (struct scsi_device *,gfp_t );