Message-ID: <108fa543-a086-2143-47bd-d525a58513c1@huawei.com>
Date: Sat, 4 Jan 2025 17:15:05 +0800
From: Zeng Heng <zengheng4@...wei.com>
To: Dave Martin <Dave.Martin@....com>
CC: <james.morse@....com>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <jonathan.cameron@...wei.com>,
	<xiexiuqi@...wei.com>, "Wangshaobo (bobo)" <bobo.shaobowang@...wei.com>
Subject: Re: [RFC PATCH mpam mpam/snapshot/v6.12-rc1 v3 2/5] arm_mpam: Read
 monitor value with new closid/rmid pair



On 2025/1/3 23:31, Dave Martin wrote:
> On Fri, Jan 03, 2025 at 02:55:17PM +0800, Zeng Heng wrote:
>>
>>
>> On 2024/12/13 0:18, Dave Martin wrote:
>>> Hi,
>>>
>>> On Sat, Dec 07, 2024 at 05:21:33PM +0800, Zeng Heng wrote:
> 
> [...]
> 
>>>> Based on the example provided, the conversion relationship between
>>>> closid/rmid and (req)PARTID/PMG is:
>>>>
>>>>       (req)PARTID = (rmid.req_idx * n) + closid,
>>>>       PMG = rmid.pmg.
>>>
>>> It seemed more natural to me for the PARTIDs assigned to a particular
>>> CLOSID to be consecutively numbered (see [1]), though it works either
>>> way.
>>>
>>> Otherwise, the approach makes sense.
>>>
>>
>>
>> After attempting to change the mapping method in practice, I found that
>> there are some following advantages of the current method which keeps
>> intPARTIDs are mapped to the first n IDs:
> 
> Thanks for having a go.
> 
>> 1. Because closid is exactly equal to intPARTID, and the conversion
>> relationship between closid and intPARTID remains unchanged under the
>> current method (still only call the resctrl_get_config_index() for
>> conversion), maintaining the original semantics during the MPAM
>> configuration updating;
> 
> You are right about this, but I think this is just moving complexity
> around rather than eliminating it?
> 
> I've tried various approaches, and there there always seems to be one
> ugly step somewhere; either something in mpam_devices.c that feels like
> it should be in mpam_resctrl.c, or something in mpam_resctrl.c that
> feels like it should be in mpam_devices.c.
> 
>> 2. Since there is no need to create a new transformation (like
>> closid2intpartid()) between closid and intPARTID, this can reduce the
>> work of function adaptations, such as in resctrl_arch_update_one(),
>> resctrl_arch_get_config(), and so on, which doesn't need any extra
>> adaptions and keeps things as simple as possible.
>>
>> Looking forward to your comments.
>>
>>
>> Greeting for new year,
>> Zeng Heng
>>
> 
> Happy New Year to you too!
> 
> What you say is true, but I think the runtime cost of the conversions
> is going to be trivial compared with the cost of the actual MSC
> programming.
> 
> For context: I'm hoping to factor the code so that the conversion is as
> cleanly separated out as possible, so that it would be straightforward
> to move to an arbitrary mapping in the future if it is possible to
> agree changes in the core resctrl interface so that the PARTID/PMG
> allocations can be dynamic.  If we do that, the conversion would
> probably become a simple table lookup.
> 
> This factoring seems more important than which precise mapping we
> choose right now.
> 
> But in the interests of improving PARTID Narrowing support sooner,
> I think that going straight to dynamic allocation is not the best
> approach -- so my idea is to prepare for that on the MPAM driver side,
> but not prioritise developing a dynamic approach until after the
> resctrlfs refactoring and the MPAM driver are merged upstream.
> 
> Does that make sense?
> 

One of the main reasons for only being able to use static allocation is
the constraint of the alloc_rmid(). Unless we switch to using another
interface (such as resctrl_arch_alloc_rmid()) for rmid allocation, doing
so would definitely break the existing public interface of resctrl.

Consequently, this would increase the difficulty of merging the MPAM
driver into the mainline. (Of course, even after merging into the
upstream, expectations for MPAM to change the common interface still
remain cautious.)

But for how to support dynamic allocation in the future, I have the
following preliminary views and possible directions for evolution:

1. The mapping between closid and intpartid will continue to be
maintained as it is, supporting the NP feature, which should not lead to
changes in the definition of closid, because essentially NP is just an
enhanced feature for monitoring;

2. rmid will no longer only contain pmg information. rmid would be
composed of rmid.req_idx (or directly rmid.reqpartid) and rmid.pmg.

Once the mapping between closid/rmid is determined, the allocation
method will also be determined, and the dynamic allocation process can
be like table lookup.

Currently, I am prioritizing updating the issues you discussed
previously on the basis of v3 and releasing it as v4 for further review.

Both v3 and v4 include the above ideas, and we can continue discussions 
in any direction based on v4.

Thank you in advance.


Best regards,
Zeng Heng

Powered by blists - more mailing lists