[PATCH] crypto: ccp - Use different flag vars for nested locks

[PATCH] crypto: ccp - Use different flag vars for nested locks

[Gary R Hook]

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(-)

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);
 

Re: [PATCH] crypto: ccp - Use different flag vars for nested locks

[Tom Lendacky]

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);
>  
> 

Re: [PATCH] crypto: ccp - Use different flag vars for nested locks

[Herbert Xu]

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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: ccp - Use different flag vars for nested locks

[Gary R Hook]

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