Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH v3] perf/core: Fix reentry problem in
 perf_output_read_group
To:     Peter Zijlstra <peterz@infradead.org>
CC:     <rostedt@goodmis.org>, <mingo@redhat.com>, <acme@kernel.org>,
        <mark.rutland@arm.com>, <alexander.shishkin@linux.intel.com>,
        <jolsa@kernel.org>, <namhyung@kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-perf-users@vger.kernel.org>
References: <20220816091103.257702-1-yangjihong1@huawei.com>
 <YvumDL1qz1NjpfEC@worktop.programming.kicks-ass.net>
From:   Yang Jihong <yangjihong1@huawei.com>
Message-ID: <42e62907-2895-1267-8942-fcad544dca35@huawei.com>
Date:   Thu, 18 Aug 2022 10:23:56 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.1
MIME-Version: 1.0
In-Reply-To: <YvumDL1qz1NjpfEC@worktop.programming.kicks-ass.net>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hello,

On 2022/8/16 22:13, Peter Zijlstra wrote:
> On Tue, Aug 16, 2022 at 05:11:03PM +0800, Yang Jihong wrote:
>> perf_output_read_group may respond to IPI request of other cores and invoke
>> __perf_install_in_context function. As a result, hwc configuration is modified.
>> As a result, the hwc configuration is modified, causing inconsistency and
>> unexpected consequences.
> 
>>   read_pmevcntrn+0x1e4/0x1ec arch/arm64/kernel/perf_event.c:423
>>   armv8pmu_read_evcntr arch/arm64/kernel/perf_event.c:467 [inline]
>>   armv8pmu_read_hw_counter arch/arm64/kernel/perf_event.c:475 [inline]
>>   armv8pmu_read_counter+0x10c/0x1f0 arch/arm64/kernel/perf_event.c:528
>>   armpmu_event_update+0x9c/0x1bc drivers/perf/arm_pmu.c:247
>>   armpmu_read+0x24/0x30 drivers/perf/arm_pmu.c:264
>>   perf_output_read_group+0x4cc/0x71c kernel/events/core.c:6806
>>   perf_output_read+0x78/0x1c4 kernel/events/core.c:6845
>>   perf_output_sample+0xafc/0x1000 kernel/events/core.c:6892
>>   __perf_event_output kernel/events/core.c:7273 [inline]
>>   perf_event_output_forward+0xd8/0x130 kernel/events/core.c:7287
>>   __perf_event_overflow+0xbc/0x20c kernel/events/core.c:8943
>>   perf_swevent_overflow kernel/events/core.c:9019 [inline]
>>   perf_swevent_event+0x274/0x2c0 kernel/events/core.c:9047
>>   do_perf_sw_event kernel/events/core.c:9160 [inline]
>>   ___perf_sw_event+0x150/0x1b4 kernel/events/core.c:9191
>>   __perf_sw_event+0x58/0x7c kernel/events/core.c:9203
>>   perf_sw_event include/linux/perf_event.h:1177 [inline]
> 
>> Interrupts is not disabled when perf_output_read_group reads PMU counter.
> 
> s/is/are/ due to 'interrupts' being plural
> 
> Anyway, yes, I suppose this is indeed so. That code expects to run with
> IRQs disabled but in the case of software events that isn't so.
> 
The v4 patch only corrects the commit message because it is not 
determined whether to check the software event.
If processing is improper, please tell me and will resend the patch.

Thanks,
Yang