linux-kernel - Re: [PATCH 2/4] iopoll: Avoid evaluating 'cond' twice in poll_timeout

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: <514069318ce4ae0746af507aa24321ddfe4dc399@intel.com>
Date: Thu, 03 Jul 2025 14:55:24 +0300
From: Jani Nikula <jani.nikula@...el.com>
To: Ville Syrjala <ville.syrjala@...ux.intel.com>, linux-kernel@...r.kernel.org
Cc: Lucas De Marchi <lucas.demarchi@...el.com>, Dibin Moolakadan
 Subrahmanian <dibin.moolakadan.subrahmanian@...el.com>, Imre Deak
 <imre.deak@...el.com>, David Laight <david.laight.linux@...il.com>, Geert
 Uytterhoeven <geert+renesas@...der.be>, Matt Wagantall
 <mattw@...eaurora.org>, Dejin Zheng <zhengdejin5@...il.com>,
 intel-gfx@...ts.freedesktop.org, intel-xe@...ts.freedesktop.org, Ville
 Syrjälä <ville.syrjala@...ux.intel.com>
Subject: Re: [PATCH 2/4] iopoll: Avoid evaluating 'cond' twice in
 poll_timeout_us()

On Thu, 03 Jul 2025, Ville Syrjala <ville.syrjala@...ux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@...ux.intel.com>
>
> Currently poll_timeout_us() evaluates 'cond' twice at the end
> of the success case. This not desirable in case 'cond' itself
> is expensive.
>
> Avoid the double evaluation by tracking the return value in
> a variable. Need to use a triple undescore '___ret' name to
> avoid a conflict with an existing double undescore '__ret'
> variable in the regmap code.
>
> Cc: Jani Nikula <jani.nikula@...el.com>
> Cc: Lucas De Marchi <lucas.demarchi@...el.com>
> Cc: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@...el.com>
> Cc: Imre Deak <imre.deak@...el.com>
> Cc: David Laight <david.laight.linux@...il.com>
> Cc: Geert Uytterhoeven <geert+renesas@...der.be>
> Cc: Matt Wagantall <mattw@...eaurora.org>
> Cc: Dejin Zheng <zhengdejin5@...il.com>
> Cc: intel-gfx@...ts.freedesktop.org
> Cc: intel-xe@...ts.freedesktop.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@...ux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@...el.com>

> ---
>  include/linux/iopoll.h | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
> index 0d8186d3df03..69296e6adbf3 100644
> --- a/include/linux/iopoll.h
> +++ b/include/linux/iopoll.h
> @@ -36,23 +36,30 @@
>  	u64 __timeout_us = (timeout_us); \
>  	unsigned long __sleep_us = (sleep_us); \
>  	ktime_t __timeout = ktime_add_us(ktime_get(), __timeout_us); \
> +	int ___ret; \
>  	might_sleep_if((__sleep_us) != 0); \
>  	if ((sleep_before_op) && __sleep_us) \
>  		usleep_range((__sleep_us >> 2) + 1, __sleep_us); \
>  	for (;;) { \
>  		op; \
> -		if (cond) \
> +		if (cond) { \
> +			___ret = 0; \
>  			break; \
> +		} \
>  		if (__timeout_us && \
>  		    ktime_compare(ktime_get(), __timeout) > 0) { \
>  			op; \
> +			if (cond) \
> +				___ret = 0; \
> +			else \
> +				___ret = -ETIMEDOUT; \
>  			break; \
>  		} \
>  		if (__sleep_us) \
>  			usleep_range((__sleep_us >> 2) + 1, __sleep_us); \
>  		cpu_relax(); \
>  	} \
> -	(cond) ? 0 : -ETIMEDOUT; \
> +	___ret; \
>  })
>  
>  /**
> @@ -83,6 +90,7 @@
>  	s64 __left_ns = __timeout_us * NSEC_PER_USEC; \
>  	unsigned long __delay_us = (delay_us); \
>  	u64 __delay_ns = __delay_us * NSEC_PER_USEC; \
> +	int ___ret; \
>  	if ((delay_before_op) && __delay_us) { \
>  		udelay(__delay_us); \
>  		if (__timeout_us) \
> @@ -90,10 +98,16 @@
>  	} \
>  	for (;;) { \
>  		op; \
> -		if (cond) \
> +		if (cond) { \
> +			___ret = 0; \
>  			break; \
> +		} \
>  		if (__timeout_us && __left_ns < 0) { \
>  			op; \
> +			if (cond) \
> +				___ret = 0; \
> +			else \
> +				___ret = -ETIMEDOUT; \
>  			break; \
>  		} \
>  		if (__delay_us) { \
> @@ -105,7 +119,7 @@
>  		if (__timeout_us) \
>  			__left_ns--; \
>  	} \
> -	(cond) ? 0 : -ETIMEDOUT; \
> +	___ret; \
>  })
>  
>  /**

-- 
Jani Nikula, Intel