> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Wednesday, July 29, 2015 4:28 AM
> To: Romer, Benjamin M
> Cc: gre...@linuxfoundation.org; jes.soren...@redhat.com; *S-Par-
> Maintainer; driverdev-devel@linuxdriverproject.org; Sell, Timothy C
> Subject: Re: [PATCH 2/6] staging: unisys: visornic - prevent lock recursion
> after IO recovery
>
> On Tue, Jul 28, 2015 at 12:29:07PM -0400, Benjamin Romer wrote:
> > diff --git a/drivers/staging/unisys/visornic/visornic_main.c
> b/drivers/staging/unisys/visornic/visornic_main.c
> > index 18b0465..d5095c9 100644
> > --- a/drivers/staging/unisys/visornic/visornic_main.c
> > +++ b/drivers/staging/unisys/visornic/visornic_main.c
> > @@ -417,6 +417,7 @@ visornic_serverdown(struct visornic_devdata
> *devdata,
> > }
> > devdata->server_change_state = true;
> > devdata->server_down_complete_func = complete_func;
> > + spin_unlock_irqrestore(&devdata->priv_lock, flags);
> > visornic_serverdown_complete(devdata);
> > } else if (devdata->server_change_state) {
> > dev_dbg(&devdata->dev->device, "%s changing state\n",
> > @@ -424,7 +425,6 @@ visornic_serverdown(struct visornic_devdata
> *devdata,
> > spin_unlock_irqrestore(&devdata->priv_lock, flags);
> > return -EINVAL;
> > }
> > - spin_unlock_irqrestore(&devdata->priv_lock, flags);
> > return 0;
>
> Don't we still need a spin_unlock if devdata->server_down is true and
> devdata->server_change_state is false? Or can that not happen?
>
> A naive reading of the function implies that it can. I checked to see
> if Smatch would have complained here, and it only complains if when
> --spammy is enabled. These warnings have a too high false positive
> ratio for normal people.
>
> regards,
> dan carpenter
Yikes.. you're exactly right Dan; I can't believe I missed that! Thanks.
I will re-submit with the appropriate correction, i.e.:
@@ -410,7 +410,8 @@ visornic_serverdown(struct visornic_devdata *devdata,
__func__);
spin_unlock_irqrestore(&devdata->priv_lock, flags);
return -EINVAL;
- }
+ } else
+ spin_unlock_irqrestore(&devdata->priv_lock, flags);
return 0;
}
Tim Sell
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
