On 8/17/21 2:26 PM, Christoph Hellwig wrote:
> On Tue, Aug 17, 2021 at 11:14:12AM +0200, Hannes Reinecke wrote:
>> When calling a host reset we shouldn't rely on the command triggering
>> the reset, so allow megaraid_abort_and_reset() to be called with a
>> NULL scb.
>> And drop the pointless 'bus_reset' and 'target_reset' handlers, which
>> just call the same function as host_reset.
>>
>> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
>> ---
>> drivers/scsi/megaraid.c | 42 ++++++++++++++++-------------------------
>> 1 file changed, 16 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
>> index 56910e94dbf2..7c53933fb1b4 100644
>> --- a/drivers/scsi/megaraid.c
>> +++ b/drivers/scsi/megaraid.c
>> @@ -1905,7 +1905,7 @@ megaraid_reset(struct scsi_cmnd *cmd)
>>
>> spin_lock_irq(&adapter->lock);
>>
>> - rval = megaraid_abort_and_reset(adapter, cmd, SCB_RESET);
>> + rval = megaraid_abort_and_reset(adapter, NULL, SCB_RESET);
>>
>> /*
>> * This is required here to complete any completed requests
>> @@ -1944,7 +1944,7 @@ megaraid_abort_and_reset(adapter_t *adapter, struct scsi_cmnd *cmd, int aor)
>>
>> scb = list_entry(pos, scb_t, list);
>
> Ther's a dev_warn before this, which dereferences cmd.
>
>> - if (scb->cmd == cmd) { /* Found command */
>> + if (!cmd || scb->cmd == cmd) { /* Found command */
>>
>> scb->state |= aor;
>
> But more importantly, this function doesn't make much sense for the
> !cmd case, as it doesn't really do anything when not matched. It
> seems like the legacy megaraid driver should just stop calling it
> for resets.
>
Well, it does the usual RAID HBA host reset: wait for commands to
complete (That's the '!cmd' case).
So as such I guess there is a value in having a host reset function;
we might, however, look at separating the functions.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
