linux-kernel - Re: [PATCH] iio: gts-helper: Fix division loop

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]

Date: Sun, 7 Jan 2024 16:22:53 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>, Lars-Peter Clausen
 <lars@...afoo.de>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: gts-helper: Fix division loop

On Thu, 4 Jan 2024 11:34:28 +0200
Matti Vaittinen <mazziesaccount@...il.com> wrote:

> The loop based 64bit division may run for a long time when dividend is a
> lot bigger than the divider. Replace the division loop by the
> div64_u64() which implementation may be significantly faster.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
> Fixes: 38416c28e168 ("iio: light: Add gain-time-scale helpers")

Hmm. Fix or not perf improvement?  I'm going to take the middle ground
and leave the fixes tag, but not rush this in.

So applied to the togreg branch of iio.git and for now just pushed out
as testing for 0-day etc to take a look before I rebase that tree after
rc1.



> ---
> 
> I've implemented also a fixup series for supporting rounding of
> gains/scales:
> https://lore.kernel.org/lkml/37d3aa193e69577353d314e94463a08d488ddd8d.1701780964.git.mazziesaccount@gmail.com/
> 
> That series does also remove the offending loop.
> 
> We don't currently have any in-tree users of GTS helpers which would
> need the rounding support so pushing the rounding is not urgent (and I
> haven't heard of Subjahit whose driver required the rounding). Hence, we
> may want to only take this loop fix in for now (?) and reconsider
> rounding when someone need that.
> 
> Jonathan, what's your take on this?
Agreed - let us wait for the rounding to have a user, but makes sense
to tidy this corner up in the meantime.

Thanks,

Jonathan

> 
>  drivers/iio/industrialio-gts-helper.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
> index 7653261d2dc2..abcab2d38589 100644
> --- a/drivers/iio/industrialio-gts-helper.c
> +++ b/drivers/iio/industrialio-gts-helper.c
> @@ -34,7 +34,7 @@
>  static int iio_gts_get_gain(const u64 max, const u64 scale)
>  {
>  	u64 full = max;
> -	int tmp = 1;
> +	int tmp = 0;
>  
>  	if (scale > full || !scale)
>  		return -EINVAL;
> @@ -48,8 +48,7 @@ static int iio_gts_get_gain(const u64 max, const u64 scale)
>  		tmp++;
>  	}
>  
> -	while (full > scale * (u64)tmp)
> -		tmp++;
> +	tmp += div64_u64(full, scale);
>  
>  	return tmp;
>  }
> 
> base-commit: 2cc14f52aeb78ce3f29677c2de1f06c0e91471ab