[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <40e9934d-4d19-5cea-e21f-d287584b71f4@huawei.com>
Date: Tue, 4 Nov 2025 21:48:47 +0800
From: Zeng Heng <zengheng4@...wei.com>
To: Ben Horgan <ben.horgan@....com>, <james.morse@....com>
CC: <amitsinght@...vell.com>, <baisheng.gao@...soc.com>,
<baolin.wang@...ux.alibaba.com>, <carl@...amperecomputing.com>,
<catalin.marinas@....com>, <dakr@...nel.org>, <dave.martin@....com>,
<david@...hat.com>, <dfustini@...libre.com>, <fenghuay@...dia.com>,
<gregkh@...uxfoundation.org>, <gshan@...hat.com>, <guohanjun@...wei.com>,
<jeremy.linton@....com>, <jonathan.cameron@...wei.com>, <kobak@...dia.com>,
<lcherian@...vell.com>, <lenb@...nel.org>, <linux-acpi@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<lpieralisi@...nel.org>, <peternewman@...gle.com>, <quic_jiles@...cinc.com>,
<rafael@...nel.org>, <robh@...nel.org>, <rohit.mathew@....com>,
<scott@...amperecomputing.com>, <sdonthineni@...dia.com>,
<sudeep.holla@....com>, <tan.shaopeng@...itsu.com>, <will@...nel.org>,
<xhao@...ux.alibaba.com>, <wangkefeng.wang@...wei.com>,
<sunnanyong@...wei.com>
Subject: Re: [PATCH v2] arm64/mpam: Clean MBWU monitor overflow bit
Hi Ben,
On 2025/11/4 18:24, Ben Horgan wrote:
> Hi Zeng,
>
> On 11/3/25 03:47, Zeng Heng wrote:
>> Hi Ben,
>>
>> On 2025/10/30 17:52, Ben Horgan wrote:
>>> Hi Zeng,
>>>
>>> On 10/29/25 07:56, Zeng Heng wrote:
>>>> The MSMON_MBWU register accumulates counts monotonically forward and
>>>> would not automatically cleared to zero on overflow. The overflow
>>>> portion
>>>> is exactly what mpam_msmon_overflow_val() computes, there is no need to
>>>> additionally subtract mbwu_state->prev_val.
>>>>
>>>> Before invoking write_msmon_ctl_flt_vals(), the overflow bit of the
>>>> MSMON_MBWU register must first be read to prevent it from being
>>>> inadvertently cleared by the write operation.
>>>>
>>>> Finally, use the overflow bit instead of relying on counter wrap-around
>>>> to determine whether an overflow has occurred, that avoids the case
>>>> where
>>>> a wrap-around (now > prev_val) is overlooked. So with this, prev_val no
>>>> longer has any use and remove it.
>>>>
>>>> CC: Ben Horgan <ben.horgan@....com>
>>>> Signed-off-by: Zeng Heng <zengheng4@...wei.com>
>>>> ---
>>>> drivers/resctrl/mpam_devices.c | 22 +++++++++++++++++-----
>>>> drivers/resctrl/mpam_internal.h | 3 ---
>>>> 2 files changed, 17 insertions(+), 8 deletions(-)
>>>
>>> This all looks fine for overflow, but what we've been forgetting about
>>> is the power management. As James mentioned in his commit message, the
>>> prev_val is after now check is doing double duty. If an msc is powered
>>> down and reset then we lose the count. Hence, to keep an accurate count,
>>> we should be considering this case too.
>>>
>>
>>
>> Regarding CPU power management and CPU on-/off-line scenarios, this
>> should and already has been handled by mpam_save_mbwu_state():
>>
>> 1. Freezes the current MSMON_MBWU counter into the
>> mbwu_state->correction;
>> 2. Clears the MSMON_MBWU counter;
>>
>> After the CPU is powered back on, the total bandwidth traffic is
>> MSMON_MBWU(the `now` variable) + correction.
>>
>> So the above solution also covers CPU power-down scenarios, and no
>> additional code is needed to adapt to this case.
>>
>> If I've missed anything, thanks in advance to point it out.
>>
>
> No, I don't think you missed anything. You just didn't mention in your commit message
> that this is also fixing the power management case.
>
> I'm going to post the next version of this series for James as he is otherwise engaged.
> I've taken your patch and adapted it to fit in with the order of patches.
> Does this look ok to you? The support for the long counters will be added later.
>
Yes, I have reviewed the patch, and the related adaptations look good to
me.
> @@ -1016,6 +1025,9 @@ static void __ris_msmon_read(void *arg)
> if (config_mismatch) {
> write_msmon_ctl_flt_vals(m, ctl_val, flt_val);
> overflow = false;
> + } else if (overflow) {
> + mpam_write_monsel_reg(msc, CFG_MBWU_CTL,
> + cur_ctl & ~MSMON_CFG_x_CTL_OFLOW_STATUS);
> }
Yes, the clear register operation is added here.
Best Regards,
Zeng Heng
Powered by blists - more mailing lists