linux-kernel - Re: [RESEND PATCH v2] 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, 18 Feb 2024 01:09:33 +1030
From: Subhajit Ghosh <subhajit.ghosh@...aklogic.com>
To: Jonathan Cameron <jic23@...nel.org>,
 Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
 Lars-Peter Clausen <lars@...afoo.de>, linux-kernel@...r.kernel.org,
 David Laight <David.Laight@...lab.com>, linux-iio@...r.kernel.org
Subject: Re: [RESEND PATCH v2] iio: gts-helper: Fix division loop

On 17/2/24 00:28, Jonathan Cameron wrote:
> On Mon, 12 Feb 2024 13:20:09 +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")
>>
>> ---
>> 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);
>>   }
>>   
>>   /**
Hi Matti and Jonathan,

I somehow missed testing this patch earlier. The above patch works fine with apds9306 v7 driver(which work in progress!).
There are no errors.
My test script is simple:
#!/bin/bash
D=0
S=`cat /sys/bus/iio/devices/iio:device${D}/in_illuminance_scale_available`

for s in $S; do
	echo $s
	echo $s > /sys/bus/iio/devices/iio:device${D}/in_illuminance_scale
	sleep 5
done

One question - if I test a patch like this, do I put a "Tested-by" tag or just mention that I have tested it?

Regards,
Subhajit Ghosh

>>
>> base-commit: 841c35169323cd833294798e58b9bf63fa4fa1de
>