2024-01-11 20:56:55

by Peter Newman

[permalink] [raw]
Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control

Hi Amit,

On Thu, Aug 24, 2023 at 1:52 AM Amit Singh Tomar <[email protected]> wrote:

> 2) Second approach that we discussed internally is to have schemata for CPOR, and PPART separated by new line as mentioned/suggested by Peter, But it may require to tweak
> the ARM MPAM device driver a bit. It was kind of toss-up between 2nd and 3nd approach :), and we went with the 3rd one.
>
> L3:0=XXXX
> L3:0=PPART=X
>
> Will look into it again.

I've been looking into the structure of the MPAM driver to understand
the difficulties here.

It seems the challenge with DSPRI is trying to stuff two different
control schema (partitioning, prioritization) into the L3
rdt_resource. The rdt_resource is still a mix of a hardware component
and user interface (schema line), which leads to the
__resource_props_mismatch() function[1], which makes an arbitrary
choice (driven by resource index order) of which feature should be the
single control presented for each of the rdt_resources, the properties
of which the fields of its rdt_resource entry should describe.

It only seemed to work out for CDP emulation because the properties of
the two schema (L3CODE, L3DATA) for the L3 resource have the same CBM
properties.

My opinion is that the rdt_resource needs to be removed from
fs/resctrl and replaced with a structure to represent a control schema
and another to represent a monitor so that we don't find ourselves
unable to enumerate controls or monitors because a control or monitor
from the same hardware component has already been enumerated.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tree/drivers/platform/mpam/mpam_devices.c?h=mpam/snapshot/v6.7-rc2#n1810

-Peter


2024-01-11 21:40:44

by Luck, Tony

[permalink] [raw]
Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control

On Thu, Jan 11, 2024 at 12:56:34PM -0800, Peter Newman wrote:
> Hi Amit,
>
> On Thu, Aug 24, 2023 at 1:52 AM Amit Singh Tomar <[email protected]> wrote:
>
> > 2) Second approach that we discussed internally is to have schemata for CPOR, and PPART separated by new line as mentioned/suggested by Peter, But it may require to tweak
> > the ARM MPAM device driver a bit. It was kind of toss-up between 2nd and 3nd approach :), and we went with the 3rd one.
> >
> > L3:0=XXXX
> > L3:0=PPART=X

I'm not sure having multiple lines for the same resource makes anything
clearer. I preferred one of the earlier proposals like this one:

L3:0=XXXX,PPART=X,CCAP=X;1=YYYY,CCAP=Y

This makes the schemata file self enumerate which optional capabilities
are present for each L3 instance (in the above example the second
instance doesn't support PPART, but does support CCAP).

Writes to the schemata file already accept partial information, so
the resctrl schemata_write() function should be coded to allow any of:

Just update CCAP for L3 instance 1":
# echo "L3:1=CCAP=Z" > schemata

Update mask and CCAP for instance 0:
# echo "L3:0=ABCD,CCAP=Q" > schemata

Update PPART on all instances:
# echo "L3:0=PPART=M;1=PPART=N" > schemata

Legacy app that only comprehends partioning updates instance 1:
# echo "L3:1=FFFF" > schemata

-Tony

2024-01-11 22:01:39

by Reinette Chatre

[permalink] [raw]
Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control



On 1/11/2024 1:40 PM, Tony Luck wrote:
> On Thu, Jan 11, 2024 at 12:56:34PM -0800, Peter Newman wrote:
>> Hi Amit,
>>
>> On Thu, Aug 24, 2023 at 1:52 AM Amit Singh Tomar <[email protected]> wrote:
>>
>>> 2) Second approach that we discussed internally is to have schemata for CPOR, and PPART separated by new line as mentioned/suggested by Peter, But it may require to tweak
>>> the ARM MPAM device driver a bit. It was kind of toss-up between 2nd and 3nd approach :), and we went with the 3rd one.
>>>
>>> L3:0=XXXX
>>> L3:0=PPART=X
>
> I'm not sure having multiple lines for the same resource makes anything
> clearer. I preferred one of the earlier proposals like this one:
>
> L3:0=XXXX,PPART=X,CCAP=X;1=YYYY,CCAP=Y

This assumes that all tools (public and private) that currently parse the schemata
file will be able to handle this additional information seamlessly.

Reinette


2024-01-11 23:14:49

by Luck, Tony

[permalink] [raw]
Subject: RE: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control

>> I'm not sure having multiple lines for the same resource makes anything
>> clearer. I preferred one of the earlier proposals like this one:
>>
>> L3:0=XXXX,PPART=X,CCAP=X;1=YYYY,CCAP=Y
>
> This assumes that all tools (public and private) that currently parse the schemata
> file will be able to handle this additional information seamlessly.

Reinette,

Yes. If there are tools that *read* schemata files, they will be surprised by this extra information.

But that also applies if the "extra" information is moved to a second line that also begins with "L3:".

Tools that *write* schemata files should be OK as long as the kernel will still accept:

# echo "L3:1=fff" > schemata

E.g. the Linux selftests in tools/testing/selftests/resctrl/ should still run without
any modification.

The "separate line" option could work if the prefix isn't "L3:". E.g.

L3:0=XXXX;1=YYYY
L3PPART:0=X
L3CCAP:0=X;1=Y

If these options are asymmetrically available on cache instances, these extra
lines won't have every L3 cache instance listed.

-Tony

2024-01-11 23:32:03

by Reinette Chatre

[permalink] [raw]
Subject: Re: [EXT] Re: [RFC 00/12] ARM: MPAM: add support for priority partitioning control

Hi Tony,

On 1/11/2024 3:14 PM, Luck, Tony wrote:
>>> I'm not sure having multiple lines for the same resource makes anything
>>> clearer. I preferred one of the earlier proposals like this one:
>>>
>>> L3:0=XXXX,PPART=X,CCAP=X;1=YYYY,CCAP=Y
>>
>> This assumes that all tools (public and private) that currently parse the schemata
>> file will be able to handle this additional information seamlessly.
>
> Reinette,
>
> Yes. If there are tools that *read* schemata files, they will be surprised by this extra information.
>
> But that also applies if the "extra" information is moved to a second line that also begins with "L3:".
>
> Tools that *write* schemata files should be OK as long as the kernel will still accept:
>
> # echo "L3:1=fff" > schemata
>
> E.g. the Linux selftests in tools/testing/selftests/resctrl/ should still run without
> any modification.
>
> The "separate line" option could work if the prefix isn't "L3:". E.g.
>
> L3:0=XXXX;1=YYYY
> L3PPART:0=X
> L3CCAP:0=X;1=Y
>
> If these options are asymmetrically available on cache instances, these extra
> lines won't have every L3 cache instance listed.

I think we are going in circles here. I shared my concern about user space
breakage a while ago [1] in response to your previous proposal and this new proposal
seems to match where this thread ended [2] last year.

Reinette

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/