scsi_debug: call resp_*() function after setting host_scribble
PATCH v2 1/1 scsi_debug: call resp_*() function after setting host_scribble
Error injection in scsi_debug (e.g. opts=16, SDEBUG_OPT_TRANSPORT_ERR) currently doesn't work correctly because the test for sqcp in resp_read_dt0() and similar resp_*() functions always fails. sqcp is set from cmnd->host_scribble, which is set in schedule_resp(), which is called from scsi_debug_queuecommand() after calling the resp_* function.
Defer calling resp_*() until after cmnd->host_scribble is set in schedule_resp().
Changes in v2: Adapted to code changes after 80c49563e250
"scsi: scsi_debug: implement IMMED bit"
Notes about this adaptation:
The "flags &= ~F_LONG_DELAY" statement in scsi_debug_queuecommand() from 80c49563e250 had no effect. Dropped it.
Because we call the resp_*() function later now, the code flow in schedule_resp() is slightly different now for the IMMED case - instead of falling through to the "respond_in_thread" label immediately, the command will be put in the work queue with zero delay.
--- drivers/scsi/scsi_debug.c | 53 +++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 20 deletions(-)
drivers/scsi/scsi_debug.c +33 -20
@@ -4315,7 +4315,10 @@ static void setup_inject(struct sdebug_queue *sqp,* SCSI_MLQUEUE_HOST_BUSY if temporarily out of resources. */ static int schedule_resp (struct scsi_cmnd *cmnd,struct sdebug_dev_info *devip, -int scsi_result,int delta_jiff,int ndelay) +int scsi_result, +int (*pfp)(struct scsi_cmnd *, +struct sdebug_dev_info *), +int delta_jiff,int ndelay) {unsigned long iflags;int k, num_in_q, qdepth, inject;
@@ -4331,9 +4334,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, } sdp = cmnd->device; -if (unlikely (sdebug_verbose && scsi_result)) -sdev_printk (KERN_INFO , sdp,"%s: non-zero result=0x%x\n" , - __func__, scsi_result);if (delta_jiff ==0 ) goto respond_in_thread;
@@ -4388,7 +4388,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, sqcp = &sqp->qc_arr[k]; sqcp->a_cmnd = cmnd; cmnd->host_scribble = (unsigned char *)sqcp; - cmnd->result = scsi_result; sd_dp = sqcp->sd_dp;spin_unlock_irqrestore (&sqp->qc_lock, iflags);if (unlikely (sdebug_every_nth && sdebug_any_injecting_opt))
@@ -4398,6 +4397,22 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,if (sd_dp ==NULL )return SCSI_MLQUEUE_HOST_BUSY ; } + + cmnd->result = pfp !=NULL ?pfp (cmnd, devip) :0 ; +if (cmnd->result &SDEG_RES_IMMED_MASK ) { +/* +* This is the F_DELAY_OVERR case. No delay. +*/ + cmnd->result &= ~SDEG_RES_IMMED_MASK ; + delta_jiff = ndelay =0 ; + } +if (cmnd->result ==0 && scsi_result !=0 ) + cmnd->result = scsi_result; + +if (unlikely (sdebug_verbose && cmnd->result)) +sdev_printk (KERN_INFO , sdp,"%s: non-zero result=0x%x\n" , + __func__, cmnd->result); +if (delta_jiff >0 || ndelay >0 ) {ktime_t kt;
@@ -4440,7 +4455,10 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,return 0 ; respond_in_thread:/* call back to mid-layer using invocation thread */ - cmnd->result = scsi_result; + cmnd->result = pfp !=NULL ?pfp (cmnd, devip) :0 ; + cmnd->result &= ~SDEG_RES_IMMED_MASK ; +if (cmnd->result ==0 && scsi_result !=0 ) + cmnd->result = scsi_result; cmnd->scsi_done (cmnd);return 0 ; }
@@ -5628,6 +5646,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,struct sdebug_dev_info *devip;u8 *cmd = scp->cmnd;int (*r_pfp)(struct scsi_cmnd *,struct sdebug_dev_info *); +int (*pfp)(struct scsi_cmnd *,struct sdebug_dev_info *) =NULL ;int k, na;int errsts =0 ;u32 flags;
@@ -5749,19 +5768,13 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,return 0 ;/* ignore command: make trouble */ }if (likely (oip->pfp)) - errsts = oip->pfp (scp, devip);/* calls a resp_* function */ -else if (r_pfp)/* if leaf function ptr NULL, try the root's */ - errsts =r_pfp (scp, devip); -if (errsts &SDEG_RES_IMMED_MASK ) { - errsts &= ~SDEG_RES_IMMED_MASK ; - flags |=F_DELAY_OVERR ; - flags &= ~F_LONG_DELAY ; - } - + pfp = oip->pfp;/* calls a resp_* function */ +else + pfp = r_pfp;/* if leaf function ptr NULL, try the root's */ fini:if (F_DELAY_OVERR & flags) -return schedule_resp (scp, devip, errsts,0 ,0 ); +return schedule_resp (scp, devip, errsts, pfp,0 ,0 );else if ((sdebug_jdelay || sdebug_ndelay) && (flags &F_LONG_DELAY )) {/* * If any delay is active, want F_LONG_DELAY to be at least 1
@@ -5771,14 +5784,14 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,int jdelay = (sdebug_jdelay <2 ) ?1 : sdebug_jdelay; jdelay =mult_frac (USER_HZ * jdelay,HZ ,USER_HZ ); -return schedule_resp (scp, devip, errsts, jdelay,0 ); +return schedule_resp (scp, devip, errsts, pfp, jdelay,0 ); }else -return schedule_resp (scp, devip, errsts, sdebug_jdelay, +return schedule_resp (scp, devip, errsts, pfp, sdebug_jdelay, sdebug_ndelay); check_cond: -return schedule_resp (scp, devip, check_condition_result,0 ,0 ); +return schedule_resp (scp, devip, check_condition_result,NULL ,0 ,0 ); err_out: -return schedule_resp (scp,NULL ,DID_NO_CONNECT <<16 ,0 ,0 ); +return schedule_resp (scp,NULL ,DID_NO_CONNECT <<16 ,NULL ,0 ,0 ); }static struct scsi_host_template sdebug_driver_template = {
Signature · 1 line
2.16.1
Quoted 16 lines · Martin Wilck
Error injection in
scsi_debug(e.g. opts=16,SDEBUG_OPT_TRANSPORT_ERR) currently doesn't work correctly because the test for sqcp inresp_read_dt0()and similar resp_*() functions always fails. sqcp is set fromcmnd->host_scribble, which is set inschedule_resp(), which is called fromscsi_debug_queuecommand()after calling the resp_* function.Defer calling resp_*() until after
cmnd->host_scribbleis set inschedule_resp().Fixes c483739430f1 "scsi_debug: add multiple queue support"Changes in v2: Adapted to code changes after
80c49563e250
"scsi:scsi_debug: implement IMMED bit"Notes about this adaptation:
Quoted 8 lines
The "flags &= ~F_LONG_DELAY" statement in
scsi_debug_queuecommand()from80c49563e250had no effect. Dropped it.
Because we call the resp_*() function later now, the code flow inschedule_resp()is slightly different now for the IMMED case - instead of falling through to the "respond_in_thread" label immediately, the command will be put in the work queue with zero delay.Signed-off-by Martin Wilck <[email protected]>
Ack-ed by: Douglas Gilbert <[email protected]>
Quoted 125 lines
--- drivers/scsi/scsi_debug.c | 53 +++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 20 deletions(-)drivers/scsi/scsi_debug.c +33
-20@@ -4315,7 +4315,10 @@ static void setup_inject(struct sdebug_queue *sqp,* SCSI_MLQUEUE_HOST_BUSY if temporarily out of resources. */ static int schedule_resp (struct scsi_cmnd *cmnd,struct sdebug_dev_info *devip, -int scsi_result,int delta_jiff,int ndelay) +int scsi_result, +int (*pfp)(struct scsi_cmnd *, +struct sdebug_dev_info *), +int delta_jiff,int ndelay) {unsigned long iflags;int k, num_in_q, qdepth, inject;@@ -4331,9 +4334,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, } sdp = cmnd->device; -if (unlikely (sdebug_verbose && scsi_result)) -sdev_printk (KERN_INFO , sdp,"%s: non-zero result=0x%x\n" , - __func__, scsi_result);if (delta_jiff ==0 ) goto respond_in_thread;@@ -4388,7 +4388,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, sqcp = &sqp->qc_arr[k]; sqcp->a_cmnd = cmnd; cmnd->host_scribble = (unsigned char *)sqcp; - cmnd->result = scsi_result; sd_dp = sqcp->sd_dp;spin_unlock_irqrestore (&sqp->qc_lock, iflags);if (unlikely (sdebug_every_nth && sdebug_any_injecting_opt))@@ -4398,6 +4397,22 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,if (sd_dp ==NULL )return SCSI_MLQUEUE_HOST_BUSY ; } + + cmnd->result = pfp !=NULL ?pfp (cmnd, devip) :0 ; +if (cmnd->result &SDEG_RES_IMMED_MASK ) { +/* +* This is the F_DELAY_OVERR case. No delay. +*/ + cmnd->result &= ~SDEG_RES_IMMED_MASK ; + delta_jiff = ndelay =0 ; + } +if (cmnd->result ==0 && scsi_result !=0 ) + cmnd->result = scsi_result; + +if (unlikely (sdebug_verbose && cmnd->result)) +sdev_printk (KERN_INFO , sdp,"%s: non-zero result=0x%x\n" , + __func__, cmnd->result); +if (delta_jiff >0 || ndelay >0 ) {ktime_t kt;@@ -4440,7 +4455,10 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,return 0 ; respond_in_thread:/* call back to mid-layer using invocation thread */ - cmnd->result = scsi_result; + cmnd->result = pfp !=NULL ?pfp (cmnd, devip) :0 ; + cmnd->result &= ~SDEG_RES_IMMED_MASK ; +if (cmnd->result ==0 && scsi_result !=0 ) + cmnd->result = scsi_result; cmnd->scsi_done (cmnd);return 0 ; }@@ -5628,6 +5646,7 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,struct sdebug_dev_info *devip;u8 *cmd = scp->cmnd;int (*r_pfp)(struct scsi_cmnd *,struct sdebug_dev_info *); +int (*pfp)(struct scsi_cmnd *,struct sdebug_dev_info *) =NULL ;int k, na;int errsts =0 ;u32 flags;@@ -5749,19 +5768,13 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,return 0 ;/* ignore command: make trouble */ }if (likely (oip->pfp)) - errsts = oip->pfp (scp, devip);/* calls a resp_* function */ -else if (r_pfp)/* if leaf function ptr NULL, try the root's */ - errsts =r_pfp (scp, devip); -if (errsts &SDEG_RES_IMMED_MASK ) { - errsts &= ~SDEG_RES_IMMED_MASK ; - flags |=F_DELAY_OVERR ; - flags &= ~F_LONG_DELAY ; - } - + pfp = oip->pfp;/* calls a resp_* function */ +else + pfp = r_pfp;/* if leaf function ptr NULL, try the root's */ fini:if (F_DELAY_OVERR & flags) -return schedule_resp (scp, devip, errsts,0 ,0 ); +return schedule_resp (scp, devip, errsts, pfp,0 ,0 );else if ((sdebug_jdelay || sdebug_ndelay) && (flags &F_LONG_DELAY )) {/* * If any delay is active, want F_LONG_DELAY to be at least 1 @@ -5771,14 +5784,14 @@ static int scsi_debug_queuecommand(struct Scsi_Host *shost,int jdelay = (sdebug_jdelay <2 ) ?1 : sdebug_jdelay; jdelay =mult_frac (USER_HZ * jdelay,HZ ,USER_HZ ); -return schedule_resp (scp, devip, errsts, jdelay,0 ); +return schedule_resp (scp, devip, errsts, pfp, jdelay,0 ); }else -return schedule_resp (scp, devip, errsts, sdebug_jdelay, +return schedule_resp (scp, devip, errsts, pfp, sdebug_jdelay, sdebug_ndelay); check_cond: -return schedule_resp (scp, devip, check_condition_result,0 ,0 ); +return schedule_resp (scp, devip, check_condition_result,NULL ,0 ,0 ); err_out: -return schedule_resp (scp,NULL ,DID_NO_CONNECT <<16 ,0 ,0 ); +return schedule_resp (scp,NULL ,DID_NO_CONNECT <<16 ,NULL ,0 ,0 ); }static struct scsi_host_template sdebug_driver_template = {
Doug,
Quoted 1 line
Ack-ed by: Douglas Gilbert <[email protected]>
s/Ack-ed by:/Acked-by:/
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Martin K. Petersen
AppliedMartin,
Applied to 4.17/scsi-queue.
Minor patch submission nits below (for next time, I fixed them up).
Quoted 11 lines
Error injection in
scsi_debug(e.g. opts=16,SDEBUG_OPT_TRANSPORT_ERR) currently doesn't work correctly because the test for sqcp inresp_read_dt0()and similar resp_*() functions always fails. sqcp is set fromcmnd->host_scribble, which is set inschedule_resp(), which is called fromscsi_debug_queuecommand()after calling the resp_* function.Defer calling resp_*() until after
cmnd->host_scribbleis set inschedule_resp().Fixes c483739430f1 "scsi_debug: add multiple queue support"
Your Signed-off-by: needs to go here. And then you need a "---" separator before the change log.
Quoted 5 lines
Changes in v2: Adapted to code changes after
80c49563e250
"scsi:scsi_debug: implement IMMED bit"Notes about this adaptation:
Quoted 8 lines
The "flags &= ~F_LONG_DELAY" statement in
scsi_debug_queuecommand()from80c49563e250had no effect. Dropped it.
Because we call the resp_*() function later now, the code flow inschedule_resp()is slightly different now for the IMMED case - instead of falling through to the "respond_in_thread" label immediately, the command will be put in the work queue with zero delay.Signed-off-by Martin Wilck <[email protected]>
Thanks!
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering