linux-kernel - Re: [PATCH v2] kernel/time: Improve performance of time64_to

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] [day] [month] [year] [list]

Message-ID: <ef68a1ece1a2db3dca73c326f65304fd640c6a7e.camel@perches.com>
Date:   Wed, 02 Jun 2021 11:05:54 -0700
From:   Joe Perches <joe@...ches.com>
To:     Cassio Neri <cassio.neri@...il.com>, john.stultz@...aro.org,
        tglx@...utronix.de
Cc:     sboyd@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] kernel/time: Improve performance of time64_to_tm.
 Add tests.

On Wed, 2021-06-02 at 18:46 +0100, Cassio Neri wrote:
> The current implementation of time64_to_tm contains unnecessary loops,
> branches and look-up tables. The new one uses an arithmetic-based algorithm
> appeared in [1] and is ~3.2 times faster (YMMV).

trivia:

> diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c
[]
>  void time64_to_tm(time64_t totalsecs, int offset, struct tm *result)
>  {
> -	long days, rem, y;
> +	long days, rem;
>  	int remainder;
> -	const unsigned short *ip;
> +
> +	u64 u64tmp, udays, century, year;
> +	u32 u32tmp, day_of_century, year_of_century, day_of_year, month,
> +		day, janOrFeb, is_leap;

janOrFeb is an odd name choice and it and is_leap could be bool
which _might_ improve performance in some memory access cases.

> +	year_of_century = (u32) (u64tmp >> 32);

Perhaps
	year_of_century = upper_32_bits(u64tmp);

> +	day_of_year     = ((u32) u64tmp) / 2939745 / 4;

and
	day_of_year = lower_32_bits(u64tmp) / 2939745 / 4;

> +	is_leap         = year_of_century != 0 ?
> +		year_of_century % 4 == 0 : century % 4 == 0;
> +
> +	u32tmp          = 2141 * day_of_year + 132377;
> +	month           = u32tmp >> 16;
> +	day             = ((u16) u32tmp) / 2141;
> +
> +	/* Recall that January 01 is the 306-th day of the year in the
> +	 * computational (not Gregorian) calendar.
> +	 */
> +	janOrFeb        = day_of_year >= 306;
> +
> +	/* Converts to the Gregorian calendar and adjusts to Unix time. */
> +	year            = year + janOrFeb - 6313183731940000ULL;
> +	month           = janOrFeb ? month - 12 : month;
> +	day             = day + 1;
> +	day_of_year     = janOrFeb ?
> +		day_of_year - 306 : day_of_year + 31 + 28 + is_leap;

I believe the extended naming improves readability, thanks.