Message ID | 1637117108-230103-12-git-send-email-chenxiang66@hisilicon.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | Add runtime PM support for libsas | expand |
Please consider these: On 17/11/2021 02:45, chenxiang wrote: I'd have "scsi: libsas: Refactor sas_queue_deferred_work()" > From: Xiang Chen <chenxiang66@hisilicon.com> > > 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(). Thanks, John > > Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com> > Reviewed-by: John Garry <john.garry@huawei.com> > --- > 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); >
在 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@hisilicon.com> >> >> 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@hisilicon.com> >> Reviewed-by: John Garry <john.garry@huawei.com> >> --- >> 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); >> > > > . >
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);