Received-SPF: pass (google.com: domain of linux-kernel+bounces-68678-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Fri, 16 Feb 2024 13:58:12 +0000
From: Jonathan Cameron <jic23@kernel.org>
To: Matti Vaittinen <mazziesaccount@gmail.com>
Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>, Lars-Peter Clausen
 <lars@metafoo.de>, linux-kernel@vger.kernel.org, David Laight
 <David.Laight@aculab.com>, Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>,
 linux-iio@vger.kernel.org
Subject: Re: [RESEND PATCH v2] iio: gts-helper: Fix division loop
Message-ID: <20240216135812.07c9b769@jic23-huawei>
In-Reply-To: <Zcn-6e-0-nh2WcfU@drtxq0yyyyyyyyyyyyyby-3.rev.dnainternet.fi>
References: <Zcn-6e-0-nh2WcfU@drtxq0yyyyyyyyyyyyyby-3.rev.dnainternet.fi>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Mon, 12 Feb 2024 13:20:09 +0200
Matti Vaittinen <mazziesaccount@gmail.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@gmail.com>
> Fixes: 38416c28e168 ("iio: light: Add gain-time-scale helpers")
> 
> ---
> This is a resend. Only change is the base which is now the v6.8-rc4 and
> not the v6.8-rc1
Given I'm not rushing this in, it is going via my togreg tree, so the
rebase wasn't really helpful (thankfully didn't stop it applying).
Would have been fine to send a ping response to the first posting of it.

I was leaving some time for David or Subhajit to have time to take
another look, but guess they are either happy with this or busy.

Applied to the togreg branch of iio.git and pushed out as testing for
all the normal reasons.

Jonathan

> 
> This change was earlier applied and reverted as it confusingly lacked of
> the removal of the overflow check (which is only needed when we do
> looping "while (full > scale * (u64)tmp)". As this loop got removed, the
> check got also obsolete and leaving it to the code caused some
> confusion.
> 
> So, I marked this as a v2, where v1 is the reverted change discussed
> here:
> https://lore.kernel.org/linux-iio/ZZZ7pJBGkTdFFqiY@dc78bmyyyyyyyyyyyyydt-3.rev.dnainternet.fi/
> 
> Revision history:
> v1 => v2:
>  - Drop the obsolete overflow check
>  - Rebased on top of the v6.8-rc4
> 
> iio: gts: loop fix fix
> ---
>  drivers/iio/industrialio-gts-helper.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
> index 7653261d2dc2..b51eb6cb766f 100644
> --- a/drivers/iio/industrialio-gts-helper.c
> +++ b/drivers/iio/industrialio-gts-helper.c
> @@ -34,24 +34,11 @@
>  static int iio_gts_get_gain(const u64 max, const u64 scale)
>  {
>  	u64 full = max;
> -	int tmp = 1;
>  
>  	if (scale > full || !scale)
>  		return -EINVAL;
>  
> -	if (U64_MAX - full < scale) {
> -		/* Risk of overflow */
> -		if (full - scale < scale)
> -			return 1;
> -
> -		full -= scale;
> -		tmp++;
> -	}
> -
> -	while (full > scale * (u64)tmp)
> -		tmp++;
> -
> -	return tmp;
> +	return div64_u64(full, scale);
>  }
>  
>  /**
> 
> base-commit: 841c35169323cd833294798e58b9bf63fa4fa1de