netdev - Re: [PATCH 1/2] net: davinci_cpdma: reduce time holding ctlr->lock in cpdma_control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4ca5db2b-9d81-8157-aec5-f7d8cf183981@ti.com>
Date:	Thu, 4 Aug 2016 18:22:03 +0300
From:	Grygorii Strashko <grygorii.strashko@...com>
To:	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
	Mugunthan V N <mugunthanvnm@...com>
CC:	<linux-omap@...r.kernel.org>, <netdev@...r.kernel.org>,
	<kernel@...gutronix.de>
Subject: Re: [PATCH 1/2] net: davinci_cpdma: reduce time holding ctlr->lock in
 cpdma_control_set

On 07/26/2016 03:02 PM, Uwe Kleine-König wrote:
> The only user of cpdma_control_set (i.e. cpsw_ndo_open) doesn't check
> the return code, so it doesn't matter, which error triggers. So the
> checks that are independant of the fields protected by ctlr->lock can be
> moved out of the critical section.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>


Reviewed-by: Grygorii Strashko <grygorii.strashko@...com>

> ---
>  drivers/net/ethernet/ti/davinci_cpdma.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
> index 73638f7a55d4..5ffa04a306c6 100644
> --- a/drivers/net/ethernet/ti/davinci_cpdma.c
> +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
> @@ -877,23 +877,21 @@ int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value)
>  	int ret;
>  	u32 val;
>
> -	spin_lock_irqsave(&ctlr->lock, flags);
> -
> -	ret = -ENOTSUPP;
>  	if (!ctlr->params.has_ext_regs)
> -		goto unlock_ret;
> +		return -ENOTSUPP;
>
> -	ret = -EINVAL;
> -	if (ctlr->state != CPDMA_STATE_ACTIVE)
> -		goto unlock_ret;
> -
> -	ret = -ENOENT;
>  	if (control < 0 || control >= ARRAY_SIZE(controls))
> -		goto unlock_ret;
> +		return -ENOENT;
>
> -	ret = -EPERM;
>  	if ((info->access & ACCESS_WO) != ACCESS_WO)
> +		return -EPERM;
> +
> +	spin_lock_irqsave(&ctlr->lock, flags);
> +
> +	if (ctlr->state != CPDMA_STATE_ACTIVE) {
> +		ret = -EINVAL;
>  		goto unlock_ret;
> +	}
>
>  	val  = dma_reg_read(ctlr, info->reg);
>  	val &= ~(info->mask << info->shift);
>


-- 
regards,
-grygorii