Re: [PATCH 6/6] tick/common: optimize cpumask_equal() usage

Thomas Gleixner
2024-05-14T08:42:48Z

On Tue, May 14 2024 at 10:31, Thomas Gleixner wrote:
> On Mon, May 13 2024 at 15:01, Yury Norov wrote:
>> Some functions in the file call cpumask_equal() with src1p == src2p.
>> We can obviously just skip comparison entirely in this case.
>>
>> This patch fixes cpumask_equal invocations when boot-test or LTP detect
>> such condition.
>
> Please write your changelogs in imperative mood.
>
> git grep 'This patch' Documentation/process/
>
> Also please see Documentation/process/maintainer-tip.rst
>
>> Signed-off-by: Yury Norov <yury.norov@gmail.com>
>> ---
>> kernel/time/tick-common.c | 15 +++++++++++----
>> 1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
>> index d88b13076b79..b31fef292833 100644
>> --- a/kernel/time/tick-common.c
>> +++ b/kernel/time/tick-common.c
>> @@ -253,7 +253,8 @@ static void tick_setup_device(struct tick_device *td,
>> * When the device is not per cpu, pin the interrupt to the
>> * current cpu:
>> */
>> - if (!cpumask_equal(newdev->cpumask, cpumask))
>> + if (newdev->cpumask != cpumask &&
>> + !cpumask_equal(newdev->cpumask, cpumask))
>> irq_set_affinity(newdev->irq, cpumask);
>
> I'm not seeing the benefit. This is slow path setup code so the extra
> comparison does not really buy anything aside of malformatted line
> breaks.

Instead of sprinkling these conditional all over the place, can't you
just do the obvious and check for ptr1 == ptr2 in bitmap_copy() and
bitmap_equal()?

Thanks,

tglx