[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cb045717-5eb8-456f-aa50-667e9f8aabfd@acm.org>
Date: Sun, 6 Oct 2024 17:45:02 -0700
From: Bart Van Assche <bvanassche@....org>
To: David Laight <David.Laight@...LAB.COM>,
Thomas Gleixner <tglx@...utronix.de>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Russell King <linux@...linux.org.uk>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Clemens Ladisch <clemens@...isch.de>
Subject: Re: [PATCH 07/21] hpet: Switch to number_of_interrupts()
On 10/6/24 10:13 AM, David Laight wrote:
> From: Bart Van Assche
>> Sent: 30 September 2024 19:16
>> --- a/drivers/char/hpet.c
>> +++ b/drivers/char/hpet.c
>> @@ -195,7 +195,7 @@ static void hpet_timer_set_irq(struct hpet_dev *devp)
>> v &= ~0xffff;
>>
>> for_each_set_bit(irq, &v, HPET_MAX_IRQ) {
>> - if (irq >= nr_irqs) {
>> + if (irq >= number_of_interrupts()) {
>> irq = HPET_MAX_IRQ;
>> break;
>> }
>
> This is horrid.
> You've replaced the read of a global variable (which, in some cases the
> compiler might be able to pull outside the loop) with a real function
> call in every loop iteration.
>
> With all the mitigations for cpu speculative execution 'issues' you
> pretty much don't want trivial function calls.
>
> If you are worried about locals shadowing globals just change one of the names.
Since HPET_MAX_IRQ == 32 and since the lower 16 bits of 'v' are cleared
on modern systems, would it be such a big deal if number_of_interrupts()
is called 16 times?
Since number_of_interrupts() has been marked as __pure, and since the
kernel is built with -O2, do you agree that this should be sufficient to
let the compiler CSE optimization step move function calls like the
above from inside a loop out of the loop?
Thanks,
Bart.
Powered by blists - more mailing lists