> -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > Sent: Wednesday, July 29, 2015 4:28 AM > To: Romer, Benjamin M > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; Jes.Sorensen@xxxxxxxxxx; *S-Par- > Maintainer; driverdev-devel@xxxxxxxxxxxxxxxxxxxxxx; 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 devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
