| Message ID | 20160311163942.4791.47157.stgit@taos (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On 03/11/2016 10:40 AM, Gary R Hook wrote: > This patch fixes a coccinelle warning about reusing a flags > variable in nested lock acquisition. > > Signed-off-by: Gary R Hook <gary.hook@amd.com> Acked-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > drivers/crypto/ccp/ccp-dev.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c > index 336e5b7..9c7bce8 100644 > --- a/drivers/crypto/ccp/ccp-dev.c > +++ b/drivers/crypto/ccp/ccp-dev.c > @@ -120,7 +120,7 @@ void ccp_del_device(struct ccp_device *ccp) > > static struct ccp_device *ccp_get_device(void) > { > - unsigned long flags; > + unsigned long flags, rrflags; > struct ccp_device *dp = NULL; > > /* We round-robin through the unit list. > @@ -128,14 +128,14 @@ static struct ccp_device *ccp_get_device(void) > */ > read_lock_irqsave(&ccp_unit_lock, flags); > if (!list_empty(&ccp_units)) { > - write_lock_irqsave(&ccp_rr_lock, flags); > + write_lock_irqsave(&ccp_rr_lock, rrflags); > dp = ccp_rr; > if (list_is_last(&ccp_rr->entry, &ccp_units)) > ccp_rr = list_first_entry(&ccp_units, struct ccp_device, > entry); > else > ccp_rr = list_next_entry(ccp_rr, entry); > - write_unlock_irqrestore(&ccp_rr_lock, flags); > + write_unlock_irqrestore(&ccp_rr_lock, rrflags); > } > read_unlock_irqrestore(&ccp_unit_lock, flags); > > -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 11, 2016 at 10:40:11AM -0600, Gary R Hook wrote: > > @@ -128,14 +128,14 @@ static struct ccp_device *ccp_get_device(void) > */ > read_lock_irqsave(&ccp_unit_lock, flags); > if (!list_empty(&ccp_units)) { > - write_lock_irqsave(&ccp_rr_lock, flags); > + write_lock_irqsave(&ccp_rr_lock, rrflags); The right thing to do is to drop the _irqsave on the inner lock. Also why is this a write lock at all as nobody seems to take it as a read lock? Cheers,
On 03/11/2016 08:22 PM, Herbert Xu wrote: > On Fri, Mar 11, 2016 at 10:40:11AM -0600, Gary R Hook wrote: >> @@ -128,14 +128,14 @@ static struct ccp_device *ccp_get_device(void) >> */ >> read_lock_irqsave(&ccp_unit_lock, flags); >> if (!list_empty(&ccp_units)) { >> - write_lock_irqsave(&ccp_rr_lock, flags); >> + write_lock_irqsave(&ccp_rr_lock, rrflags); > The right thing to do is to drop the _irqsave on the inner lock. > > Also why is this a write lock at all as nobody seems to take it > as a read lock? Roger on the _irqsave. As for this being a read-write lock: an optimization during development removed the need for a read acquisition. This use of the lock was overlooked, and now only needs to be a spin lock. Since the function of this patch has changed, and the subject line should be different, do you prefer a v2 patch, or a new patch? This one can be ignored, of course. Gary -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c index 336e5b7..9c7bce8 100644 --- a/drivers/crypto/ccp/ccp-dev.c +++ b/drivers/crypto/ccp/ccp-dev.c @@ -120,7 +120,7 @@ void ccp_del_device(struct ccp_device *ccp) static struct ccp_device *ccp_get_device(void) { - unsigned long flags; + unsigned long flags, rrflags; struct ccp_device *dp = NULL; /* We round-robin through the unit list. @@ -128,14 +128,14 @@ static struct ccp_device *ccp_get_device(void) */ read_lock_irqsave(&ccp_unit_lock, flags); if (!list_empty(&ccp_units)) { - write_lock_irqsave(&ccp_rr_lock, flags); + write_lock_irqsave(&ccp_rr_lock, rrflags); dp = ccp_rr; if (list_is_last(&ccp_rr->entry, &ccp_units)) ccp_rr = list_first_entry(&ccp_units, struct ccp_device, entry); else ccp_rr = list_next_entry(ccp_rr, entry); - write_unlock_irqrestore(&ccp_rr_lock, flags); + write_unlock_irqrestore(&ccp_rr_lock, rrflags); } read_unlock_irqrestore(&ccp_unit_lock, flags);
This patch fixes a coccinelle warning about reusing a flags variable in nested lock acquisition. Signed-off-by: Gary R Hook <gary.hook@amd.com> --- drivers/crypto/ccp/ccp-dev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html