Re: [External] : [PATCH] scsi_error: do not queue pointless abort workqueue functions

[Hannes Reinecke <hare@suse.de>]

On 11/9/22 09:28, John Garry wrote:
> On 09/11/2022 07:47, Hannes Reinecke wrote:
>> If a host template doesn't implement the .eh_abort_handler()
>> there is no point in queueing the abort workqueue function;
>> all it does is invoking SCSI EH anyway.
>> So return 'FAILED' from scsi_abort_command() if the .eh_abort_handler()
>> is not implemented and save us from having to wait for the
>> abort workqueue function to complete.
> 
> Do we ever use shost->tmf_work_q in this case? Doesn't seem much point 
> in allocating it, apart from keeping the code simpler
> 
Actually, no. Guess we can skip allocating it.

>>
>> Cc: Niklas Cassel <Niklas.Cassel@wdc.com>
>> Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com>
>> Cc: John Garry <john.garry@oracle.com>
> 
> That's someone else :)
> 
Oh. Sorry, John :-)

>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   drivers/scsi/scsi_error.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
>> index be2a70c5ac6d..e9f9c8f52c59 100644
>> --- a/drivers/scsi/scsi_error.c
>> +++ b/drivers/scsi/scsi_error.c
>> @@ -242,6 +242,11 @@ scsi_abort_command(struct scsi_cmnd *scmd)
>>           return FAILED;
>>       }
>> +    if (!shost->hostt->eh_abort_handler) {
> 
> nit: no need for {}, but maybe better put comment above the check if 
> removing it. However maybe it's also a bit obvious comment.
> 
Yeah, will do.

>> +        /* No abort handler, fail command directly */
>> +        return FAILED;
>> +    }
>> +
>>       spin_lock_irqsave(shost->host_lock, flags);
>>       if (shost->eh_deadline != -1 && !shost->last_reset)
>>           shost->last_reset = jiffies;
> 
Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer