Re: [PATCH 7/9] aacraid: use scsi_host_busy_iter() in aac_wait_for_io_completion()

[Hannes Reinecke <hare@suse.de>]

On 11/19/19 12:05 AM, Bart Van Assche wrote:
> On 11/18/19 1:22 AM, Hannes Reinecke wrote:
>> Use the midlayer helper function to traverse outstanding commands.
>>
>> Cc: Balsundar P <balsundar.p@microsemi.com>
>> Cc: Adaptec OEM Raid Solutions <aacraid@microsemi.com>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   drivers/scsi/aacraid/comminit.c | 30 +++++++++++++-----------------
>>   1 file changed, 13 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/scsi/aacraid/comminit.c
>> b/drivers/scsi/aacraid/comminit.c
>> index f75878d773cf..89c0ca339ef5 100644
>> --- a/drivers/scsi/aacraid/comminit.c
>> +++ b/drivers/scsi/aacraid/comminit.c
>> @@ -272,29 +272,25 @@ static void aac_queue_init(struct aac_dev * dev,
>> struct aac_queue * q, u32 *mem,
>>       q->entries = qsize;
>>   }
>>   +static bool wait_for_io_iter(struct scsi_cmnd *cmd, void *data,
>> bool reserved)
>> +{
>> +    int *active = data;
>> +
>> +    if (cmd->SCp.phase == AAC_OWNER_FIRMWARE)
>> +        *active = 1;
>> +    return true;
>> +}
>> +
>> +/* scsi_block_requests() has been called, so no new request can be
>> issued */
>>   static void aac_wait_for_io_completion(struct aac_dev *aac)
>>   {
>> -    unsigned long flagv = 0;
>> -    int i = 0;
>> +    int i;
>>         for (i = 60; i; --i) {
>> -        struct scsi_device *dev;
>> -        struct scsi_cmnd *command;
>>           int active = 0;
>>   -        __shost_for_each_device(dev, aac->scsi_host_ptr) {
>> -            spin_lock_irqsave(&dev->list_lock, flagv);
>> -            list_for_each_entry(command, &dev->cmd_list, list) {
>> -                if (command->SCp.phase == AAC_OWNER_FIRMWARE) {
>> -                    active++;
>> -                    break;
>> -                }
>> -            }
>> -            spin_unlock_irqrestore(&dev->list_lock, flagv);
>> -            if (active)
>> -                break;
>> -
>> -        }
>> +        scsi_host_busy_iter(aac->scsi_host_ptr,
>> +                    wait_for_io_iter, &active);
>>           /*
>>            * We can exit If all the commands are complete
>>            */
> 
> Would using scsi_device_quiesce() and scsi_device_resume() allow to
> eliminate the busy-waiting loop from the aacraid driver?
> 
I've actually looked into it, but then figured that it'll be more
cumbersome as I'd have to iterate across all devices here, calling
scsi_device_quiesce() on each one.
But looking closer into it, we could have a helper scsi_host_quiesce()
and scsi_host_resume() ... yes, indeed.
Will be updating the patchset.

Cheers,

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