Re: New sparse warning in lpfc_sli.c

[James Smart <James.Smart@xxxxxxxxxx>]

Thanks Matt.

ACK

-- james s


Matthew Wilcox wrote:
> On Wed, Feb 20, 2008 at 11:30:19PM -0800, Harvey Harrison wrote:
> > James, somewhere between 2.6.25-rc1 and -rc2 this new warning appeared, care
> > to give the locking a quick once-over here?
> >
> > drivers/scsi/lpfc/lpfc_sli.c:645:1: warning: context imbalance in 'lpfc_sli_hbqbuf_fill_hbqs' - wrong count at exit
>
> Easy enough to spot by hand:
>
>         /* Check whether HBQ is still in use */
>         spin_lock_irqsave(&phba->hbalock, flags);
>         if (!phba->hbq_in_use) {
>                 spin_unlock_irqrestore(&phba->hbalock, flags);
>                 return 0;
>         }
>
>         /* Populate HBQ entries */
>         for (i = start; i < end; i++) {
>                 hbq_buffer = (phba->hbqs[hbqno].hbq_alloc_buffer)(phba);
>                 if (!hbq_buffer)
> --->                    return 1;
>
> Here's a patch:
>
> lpfc: Balance locking
>
> Commit 3163f725a5d071eea1830bbbfab78cfe3fc9baaf introduced locking in
> lpfc_sli_hbqbuf_fill_hbqs, but missed unlocking on one exit.
>
> Reported-by: Harvey Harrison <harvey.harrison@xxxxxxxxx>
> Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxxxx>
> Cc: James Smart <james.smart@xxxxxxxxxx>
>
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> index f532064..fc0d950 100644
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -648,28 +648,24 @@ lpfc_sli_hbqbuf_fill_hbqs(struct lpfc_hba *phba, uint32_t hbqno, uint32_t count)
>  	unsigned long flags;
>  	struct hbq_dmabuf *hbq_buffer;
>  
> -	if (!phba->hbqs[hbqno].hbq_alloc_buffer) {
> +	if (!phba->hbqs[hbqno].hbq_alloc_buffer)
>  		return 0;
> -	}
>  
>  	start = phba->hbqs[hbqno].buffer_count;
>  	end = count + start;
> -	if (end > lpfc_hbq_defs[hbqno]->entry_count) {
> +	if (end > lpfc_hbq_defs[hbqno]->entry_count)
>  		end = lpfc_hbq_defs[hbqno]->entry_count;
> -	}
>  
>  	/* Check whether HBQ is still in use */
>  	spin_lock_irqsave(&phba->hbalock, flags);
> -	if (!phba->hbq_in_use) {
> -		spin_unlock_irqrestore(&phba->hbalock, flags);
> -		return 0;
> -	}
> +	if (!phba->hbq_in_use)
> +		goto out;
>  
>  	/* Populate HBQ entries */
>  	for (i = start; i < end; i++) {
>  		hbq_buffer = (phba->hbqs[hbqno].hbq_alloc_buffer)(phba);
>  		if (!hbq_buffer)
> -			return 1;
> +			goto err;
>  		hbq_buffer->tag = (i | (hbqno << 16));
>  		if (lpfc_sli_hbq_to_firmware(phba, hbqno, hbq_buffer))
>  			phba->hbqs[hbqno].buffer_count++;
> @@ -677,8 +673,12 @@ lpfc_sli_hbqbuf_fill_hbqs(struct lpfc_hba *phba, uint32_t hbqno, uint32_t count)
>  			(phba->hbqs[hbqno].hbq_free_buffer)(phba, hbq_buffer);
>  	}
>  
> + out:
>  	spin_unlock_irqrestore(&phba->hbalock, flags);
>  	return 0;
> + err:
> +	spin_unlock_irqrestore(&phba->hbalock, flags);
> +	return 1;
>  }
>  
>  int
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html