linux-kernel - Re: [PATCH 0/7] platform/chrome: Implement quickselect for median calculation

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: <ZU3GbHTA3x19h2Zi@google.com>
Date:   Fri, 10 Nov 2023 13:58:04 +0800
From:   Tzung-Bi Shih <tzungbi@...nel.org>
To:     Kuan-Wei Chiu <visitorckw@...il.com>
Cc:     bleung@...omium.org, groeck@...omium.org,
        chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/7] platform/chrome: Implement quickselect for median
 calculation

On Fri, Nov 10, 2023 at 02:54:32AM +0800, Kuan-Wei Chiu wrote:
> In addition to the algorithmic improvement, this series includes
> several typo fixes to enhance the overall code quality and maintain
> consistency.

The typo fixes are not necessary to be in the same series.  I would suggest
you separate the typo fixes to "an" (squash them) independent patch.

Please also use more specific prefix (e.g. "platform/chrome: sensorhub: ") to
make the title more clear.

> static int quickselect_test(void)
> {
> 	s64 *arr;
> 	s64 median_old, median_new;
> 	ktime_t start, end;
> 	s64 delta;
> 	const size_t array_length = 1000;
> 	const s64 seed = 1;
> 
> 	arr = kmalloc(array_length * sizeof(s64), GFP_KERNEL);
> 	if (!arr)
> 		return -ENOMEM;
> 
> 	init_array(arr, array_length, seed);
> 	start = ktime_get();
> 	median_old = cros_ec_sensor_ring_median(arr, array_length);
> 	end = ktime_get();
> 	delta = ktime_us_delta(end, start);
> 	printk(KERN_ALERT "time of original function: %lld\n", delta);
> 
> 	init_array(arr, array_length, seed);
> 	start = ktime_get();
> 	median_new = cros_ec_sensor_ring_median_new(arr, array_length);
> 	end = ktime_get();
> 	delta = ktime_us_delta(end, start);
> 	printk(KERN_ALERT "time of new function: %lld\n", delta);
> 
> 	kfree(arr);
> 
> 	/* return 0 on success */
> 	return median_old != median_new;
> }
> 
> /* Result:
>  * time of original function: 897
>  * time of new function: 16
>  */

Could you also run the micro-benchmark for n = 64[2][3]?

[2] https://elixir.bootlin.com/linux/v6.6/source/include/linux/platform_data/cros_ec_sensorhub.h#L64
[3] https://elixir.bootlin.com/linux/v6.6/source/drivers/platform/chrome/cros_ec_sensorhub_ring.c#L154