Re: [PATCH 2/2] scsi: libsas: Use bool for queue_work() return code

[John Garry <john.garry@xxxxxxxxxx>]

> > -int sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
> > +bool sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
> >   {
> >   	/* it's added to the defer_q when draining so return succeed */
> > -	int rc = 1;
> > +	bool rc = true;
> >   
> >   	if (!test_bit(SAS_HA_REGISTERED, &ha->state))
> > -		return 0;
> > +		return false;
> >   
> >   	if (test_bit(SAS_HA_DRAINING, &ha->state)) {
> >   		/* add it to the defer list, if not already pending */
> > @@ -28,11 +28,11 @@ int sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
> >   	return rc;
>
> While at it, I would cleanup this function like this:

ok, fine

> diff --git a/drivers/scsi/libsas/sas_event.c
> b/drivers/scsi/libsas/sas_event.c
> index 3613b9b315bc..38e6e91aaf36 100644
> --- a/drivers/scsi/libsas/sas_event.c
> +++ b/drivers/scsi/libsas/sas_event.c
> @@ -12,20 +12,17 @@
>
>   int sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
>   {
> -       /* it's added to the defer_q when draining so return succeed */
> -       int rc = 1;
> -
>          if (!test_bit(SAS_HA_REGISTERED, &ha->state))
> -               return 0;
> +               return false;
>
>          if (test_bit(SAS_HA_DRAINING, &ha->state)) {
>                  /* add it to the defer list, if not already pending */
>                  if (list_empty(&sw->drain_node))
>                          list_add_tail(&sw->drain_node, &ha->defer_q);
> -       } else
> -               rc = queue_work(ha->event_q, &sw->work);
> +               return true;
> +       }
>
> -       return rc;
> +       return queue_work(ha->event_q, &sw->work);
>   }
>
> No local variable :)
>
> >   }
> >   
> > -static int sas_queue_event(int event, struct sas_work *work,
> > +static bool sas_queue_event(int event, struct sas_work *work,
> >   			    struct sas_ha_struct *ha)
> >   {
> >   	unsigned long flags;
> > -	int rc;
> > +	bool rc;
> >   
> >   	spin_lock_irqsave(&ha->lock, flags);
> >   	rc = sas_queue_work(ha, work);
> > @@ -44,13 +44,12 @@ static int sas_queue_event(int event, struct sas_work *work,
> >   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) {
> > +
> > +		if (sas_queue_work(ha, sw) == false) {
>
> if (!sas_queue_work(ha, sw)) ?

ok, yeah, that's the pattern I see elsehwhere in the kernel

> >   			pm_runtime_put(ha->dev);
> >   			sas_free_event(to_asd_sas_event(&sw->work));
> >   		}
> > @@ -170,7 +169,6 @@ void sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event,
> >   {
> >   	struct sas_ha_struct *ha = phy->ha;
> >   	struct asd_sas_event *ev;
> > -	int ret;
> >   
> >   	BUG_ON(event >= PORT_NUM_EVENTS);
> >   
> > @@ -186,8 +184,7 @@ void sas_notify_port_event(struct asd_sas_phy *phy, enum port_event event,
> >   	if (sas_defer_event(phy, ev))
> >   		return;
> >   
> > -	ret = sas_queue_event(event, &ev->work, ha);
> > -	if (ret != 1) {
> > +	if (sas_queue_event(event, &ev->work, ha) == false) {
>
> Same.
>
> >   		pm_runtime_put(ha->dev);
> >   		sas_free_event(ev);
> >   	}
> > @@ -199,7 +196,6 @@ void sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event,
> >   {
> >   	struct sas_ha_struct *ha = phy->ha;
> >   	struct asd_sas_event *ev;
> > -	int ret;
> >   
> >   	BUG_ON(event >= PHY_NUM_EVENTS);
> >   
> > @@ -215,8 +211,7 @@ void sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event,
> >   	if (sas_defer_event(phy, ev))
> >   		return;
> >   
> > -	ret = sas_queue_event(event, &ev->work, ha);
> > -	if (ret != 1) {
> > +	if (sas_queue_event(event, &ev->work, ha) == false) {
>
> And again.

ok, thanks