Re: [PATCH 11/15] scsi: libsas: Refactor out sas_queue_deferred_work()

["chenxiang (M)" <chenxiang66@xxxxxxxxxxxxx>]

在 2021/12/13 19:20, John Garry 写道:
> Please consider these:
>
> On 17/11/2021 02:45, chenxiang wrote:
>
> I'd have "scsi: libsas: Refactor sas_queue_deferred_work()"

Ok, i will change it.

> > From: Xiang Chen <chenxiang66@xxxxxxxxxxxxx>
> >
> > In the 2rd part of function __sas_drain_work, it queues defer work. 
> > And it
> > will be used in other places, so refactor out sas_queue_deferred_work().
>
> The second part of the function __sas_drain_work() queues the deferred 
> work. This functionality would be duplicated in other places, so 
> factor out into sas_queue_deferred_work().

I will rewrite them in next verison.

> Thanks,
> John
>
> > Signed-off-by: Xiang Chen <chenxiang66@xxxxxxxxxxxxx>
> > Reviewed-by: John Garry <john.garry@xxxxxxxxxx>
> > ---
> >   drivers/scsi/libsas/sas_event.c    | 25 ++++++++++++++-----------
> >   drivers/scsi/libsas/sas_internal.h |  1 +
> >   2 files changed, 15 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/scsi/libsas/sas_event.c 
> > b/drivers/scsi/libsas/sas_event.c
> > index af605620ea13..01e544ca518a 100644
> > --- a/drivers/scsi/libsas/sas_event.c
> > +++ b/drivers/scsi/libsas/sas_event.c
> > @@ -41,12 +41,23 @@ static int sas_queue_event(int event, struct 
> > sas_work *work,
> >       return rc;
> >   }
> >   -
> > -void __sas_drain_work(struct sas_ha_struct *ha)
> > +void sas_queue_deferred_work(struct sas_ha_struct *ha)
> >   {
> >       struct sas_work *sw, *_sw;
> >       int ret;
> >   +    spin_lock_irq(&ha->lock);
> > +    list_for_each_entry_safe(sw, _sw, &ha->defer_q, drain_node) {
> > +        list_del_init(&sw->drain_node);
> > +        ret = sas_queue_work(ha, sw);
> > +        if (ret != 1)
> > +            sas_free_event(to_asd_sas_event(&sw->work));
> > +    }
> > +    spin_unlock_irq(&ha->lock);
> > +}
> > +
> > +void __sas_drain_work(struct sas_ha_struct *ha)
> > +{
> >       set_bit(SAS_HA_DRAINING, &ha->state);
> >       /* flush submitters */
> >       spin_lock_irq(&ha->lock);
> > @@ -55,16 +66,8 @@ void __sas_drain_work(struct sas_ha_struct *ha)
> >       drain_workqueue(ha->event_q);
> >       drain_workqueue(ha->disco_q);
> >   -    spin_lock_irq(&ha->lock);
> >       clear_bit(SAS_HA_DRAINING, &ha->state);
> > -    list_for_each_entry_safe(sw, _sw, &ha->defer_q, drain_node) {
> > -        list_del_init(&sw->drain_node);
> > -        ret = sas_queue_work(ha, sw);
> > -        if (ret != 1)
> > -            sas_free_event(to_asd_sas_event(&sw->work));
> > -
> > -    }
> > -    spin_unlock_irq(&ha->lock);
> > +    sas_queue_deferred_work(ha);
> >   }
> >     int sas_drain_work(struct sas_ha_struct *ha)
> > diff --git a/drivers/scsi/libsas/sas_internal.h 
> > b/drivers/scsi/libsas/sas_internal.h
> > index ad9764a976c3..acd515c01861 100644
> > --- a/drivers/scsi/libsas/sas_internal.h
> > +++ b/drivers/scsi/libsas/sas_internal.h
> > @@ -57,6 +57,7 @@ void sas_unregister_ports(struct sas_ha_struct 
> > *sas_ha);
> >     void sas_disable_revalidation(struct sas_ha_struct *ha);
> >   void sas_enable_revalidation(struct sas_ha_struct *ha);
> > +void sas_queue_deferred_work(struct sas_ha_struct *ha);
> >   void __sas_drain_work(struct sas_ha_struct *ha);
> >     void sas_deform_port(struct asd_sas_phy *phy, int gone);
>
>
> .