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: <20240107162253.66c1f0f1@jic23-huawei>
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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ