linux-kernel - Re: [PATCH RESEND v3] perf/core: Fix hardlockup failure caused by perf throttle

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]

Date:   Wed, 22 Mar 2023 15:36:22 +0800
From:   Yang Jihong <yangjihong1@...wei.com>
To:     <peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
        <mark.rutland@....com>, <alexander.shishkin@...ux.intel.com>,
        <jolsa@...nel.org>, <namhyung@...nel.org>, <irogers@...gle.com>,
        <eranian@...gle.com>, <linux-perf-users@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RESEND v3] perf/core: Fix hardlockup failure caused by
 perf throttle

Hello,

PING.

This patch has not been responded.
Please take time to check whether the fix solution is OK.
Look forward to reviewing the patch. Thanks :)

Thanks,
Yang.

On 2023/3/6 9:14, Yang Jihong wrote:
> Hello,
> 
> PING.
> 
> Thanks,
> Yang.
> 
> On 2023/2/27 10:35, Yang Jihong wrote:
>> commit e050e3f0a71bf ("perf: Fix broken interrupt rate throttling")
>> introduces a change in throttling threshold judgment. Before this,
>> compare hwc->interrupts and max_samples_per_tick, then increase
>> hwc->interrupts by 1, but this commit reverses order of these two
>> behaviors, causing the semantics of max_samples_per_tick to change.
>> In literal sense of "max_samples_per_tick", if hwc->interrupts ==
>> max_samples_per_tick, it should not be throttled, therefore, the judgment
>> condition should be changed to "hwc->interrupts > max_samples_per_tick".
>>
>> In fact, this may cause the hardlockup to fail, The minimum value of
>> max_samples_per_tick may be 1, in this case, the return value of
>> __perf_event_account_interrupt function is 1.
>> As a result, nmi_watchdog gets throttled, which would stop PMU (Use x86
>> architecture as an example, see x86_pmu_handle_irq).
>>
>> Fixes: e050e3f0a71b ("perf: Fix broken interrupt rate throttling")
>> Signed-off-by: Yang Jihong <yangjihong1@...wei.com>
>> ---
>>
>> Changes since v2:
>>    - Add fixed commit.
>>
>> Changes since v1:
>>    - Modify commit title.
>>
>>   kernel/events/core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index f79fd8b87f75..0540a8653906 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -9434,7 +9434,7 @@ __perf_event_account_interrupt(struct perf_event 
>> *event, int throttle)
>>       } else {
>>           hwc->interrupts++;
>>           if (unlikely(throttle
>> -                 && hwc->interrupts >= max_samples_per_tick)) {
>> +                 && hwc->interrupts > max_samples_per_tick)) {
>>               __this_cpu_inc(perf_throttled_count);
>>               tick_dep_set_cpu(smp_processor_id(), 
>> TICK_DEP_BIT_PERF_EVENTS);
>>               hwc->interrupts = MAX_INTERRUPTS;
>>
> 
> .