linux-kernel - Re: [git pull] timer fix

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]

Date:	Thu, 5 Feb 2009 08:04:15 -0800
From:	Ray Lee <ray-lk@...rabbit.org>
To:	Pavel Emelyanov <xemul@...nvz.org>
Cc:	Kirill Korotaev <dev@...allels.com>, Ingo Molnar <mingo@...e.hu>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Kirill Korotaev <dev@...nvz.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [git pull] timer fix

On Thu, Feb 5, 2009 at 1:58 AM, Pavel Emelyanov <xemul@...nvz.org> wrote:
> Kirill Korotaev wrote:
>> ACK, it works. Why not save another 4 bytes of .bss then by changing
>> hept_t1_cmp to u32? ;-)
>
> Hm... .bss you say? :) The bloat-o-meter results would be:
>
> For the original fix with casting hpet_t1_cmp to u32 inside the loop
> add/remove: 0/0 grow/shrink: 1/0 up/down: 2/0 (2)
> function                                     old     new   delta
> hpet_rtc_interrupt                           741     743      +2
>
> For the fix with casting the substitution result to s32
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-2 (-2)
> function                                     old     new   delta
> hpet_rtc_interrupt                           741     739      -2
>
> For the proposed by Kirill type change of the hpet_t1_cmp
> add/remove: 0/0 grow/shrink: 0/3 up/down: 0/-6 (-6)
> function                                     old     new   delta
> hpet_rtc_timer_init                          186     185      -1
> hpet_rtc_interrupt                           741     740      -1
> hpet_t1_cmp                                    8       4      -4
>
> That's the fix:
>
> diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
> index 64d5ad0..dfbbb94 100644
> --- a/arch/x86/kernel/hpet.c
> +++ b/arch/x86/kernel/hpet.c
> @@ -897,7 +897,7 @@ static unsigned long hpet_rtc_flags;
>  static int hpet_prev_update_sec;
>  static struct rtc_time hpet_alarm_time;
>  static unsigned long hpet_pie_count;
> -static unsigned long hpet_t1_cmp;
> +static u32 hpet_t1_cmp;
>  static unsigned long hpet_default_delta;
>  static unsigned long hpet_pie_delta;
>  static unsigned long hpet_pie_limit;
>
> Reported-by: Kirill Korotaev <dev@...nvz.org>
> Signed-off-by: Pavel Emelyanov <xemul@...nvz.org>
>
> This one also works. Should I re-send this patch in a proper way?

Ugh. This version seems too subtle for a new reader coming into this
code. I'd prefer the fix be kept in the comparison site, where people
already expect wraparound issues.

If you don't have time to add the helper, I'd really rather see the
version with the explicit cast acting as documentation, rather than a
test that magically works due to type issues.

Dunno, maybe it's just me.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/