| Message ID | 20191118092208.54521-8-hare@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: remove legacy cmd_list implementation | expand |
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? Thanks, Bart.
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
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 */
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(-)