netdev - RE: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr

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: <063D6719AE5E284EB5DD2968C1650D6D1CCD5486@AcuExch.aculab.com>
Date:	Wed, 3 Feb 2016 14:41:26 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Byeoungwook Kim' <quddnr145@...il.com>,
	"Larry.Finger@...inger.net" <Larry.Finger@...inger.net>
CC:	"kvalo@...eaurora.org" <kvalo@...eaurora.org>,
	"chaoming_li@...lsil.com.cn" <chaoming_li@...lsil.com.cn>,
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in
 core.c

From: Byeoungwook Kim
> Sent: 03 February 2016 02:00
> Conditional codes in rtl_addr_delay() were improved in readability and
> performance by using switch codes.

I'd like to see the performance data :-)

> diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c
> index 4ae421e..05f432c 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/core.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/core.c
> @@ -37,18 +37,26 @@
> 
>  void rtl_addr_delay(u32 addr)
>  {
> -	if (addr == 0xfe)
> +	switch (addr) {
> +	case 0xfe:
>  		mdelay(50);
> -	else if (addr == 0xfd)
> +		break;
> +	case 0xfd:
>  		mdelay(5);
> -	else if (addr == 0xfc)
> +		break;
> +	case 0xfc:
>  		mdelay(1);
> -	else if (addr == 0xfb)
> +		break;
> +	case 0xfb:
>  		udelay(50);
> -	else if (addr == 0xfa)
> +		break;
> +	case 0xfa:
>  		udelay(5);
> -	else if (addr == 0xf9)
> +		break;
> +	case 0xf9:
>  		udelay(1);
> +		break;
> +	};

Straight 'performance' can't matter here, not with mdelay(50)!
The most likely effect is from speeding up the 'don't delay' path
and reducing the number of conditionals (and hence accuracy of) udelay(1).
Reversing the if-chain might be better still.

	David