Message-ID: <SJ1PR11MB6083CB61BDCABA2930155C2AFC759@SJ1PR11MB6083.namprd11.prod.outlook.com>
Date:   Fri, 12 May 2023 20:35:26 +0000
From:   "Luck, Tony" <tony.luck@...el.com>
To:     "Chatre, Reinette" <reinette.chatre@...el.com>,
        "Yu, Fenghua" <fenghua.yu@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Shaopeng Tan <tan.shaopeng@...itsu.com>,
        Jamie Iles <quic_jiles@...cinc.com>,
        James Morse <james.morse@....com>,
        Babu Moger <babu.moger@....com>
CC:     "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH 0/7] Add driver registration i/f to resctrl

>> You asked for examples of resources that don't fit well into the
>> existing schemata model.
>Primarily about me trying to understand what about the existing schemata
>model makes these new features hard to support. My first confusion is
>that it is not clear to me what you mean with "schemata
>model". I think there are two parts to this: the schemata model as
>the user interface and the schemata model as implemented within
>resctrl.
>
>It is not clear in your motivation whether you are talking about the
>former or the latter and thus dug through the patches to figure this
>out. At first it seemed as though the driver aims for a totally
>different configuration mechanism with the support for driver specific
>writable files in every group ... but then then the driver pivots and
>hooks into schemata files so it then seems that the driver mechanism does
>aim to maintain the schemata model as user interface (perhaps with
>additional configuration files to give more meaning to the values
>written to the schemata file).

Different drivers may choose different subsets of the hooks available.

I have one case where a new file is, IMHO, required. There's no way to
fit what needs to be done into schemata. Another where I thought
the schemata hook makes sense (though that could be handled with
a file ... but it would look very schemata-like). 

>So, it seems to me that the concern is not with the user interface, but
>how the schemata parsing is done in resctrl. I just seem to keep guessing
>here so I would appreciate more insight.
>
>> Here's the details behind one of those cases.
>>
>> Existing resources have a single value per domain (e.g. a cache-way
>> bitmask, or a memory bandwidth percentage or target). One of my new
>> resources has several parameters. At first glance this might be
>> solved by just listing a comma separated list of these parameters
>> for each domain. So the schemata entry for feature XYZ that has two
>> domains might look like this:
>
>
>> XYZ:0=param1,param2,param3;1=param1,param2,param3
>>
>> But this feature has a second problem. The hardware supports a very
>> limited set of variations. This could be handled by reporting
>> num_closid for this resource to that low number. But then resctrl
>> core code would limit all resources to that value. Instead the h/w
>> allows programming a mapping feature from closid numbers to resource
>> instances (as the saying goes "any computer science problem can be
>> solved with one extra level of indirection").
>>
>> So if the driver named these instances: A, B, C, D. Then a schemata
>> file might look like:
>>
>> XYZ:0=B;1=C
>>
>> meaning that the driver will set up so CLOSID for this resctrl
>> resource is mapped to instance "B" on domain 0 and to instance "C" on
>> domain 1
>Apologies but it is still not clear to me how this cannot be handled
>in resctrl. For example, consider this hypothetical snippet that uses
>a unique callback to parse a resource's schemata entry (similar to the
>s->update() callback you introduce, but not have it be optional):
>
>       struct resctrl_schema *s;
>
>       list_for_each_entry(s, &resctrl_schema_all, list) {
>               if (!strcmp(resname, s->name) && rdtgrp->closid < s->num_closid)
>                       return s->parse_line(tok, s, rdtgrp);
>       }
>       rdt_last_cmd_printf("Unknown or unsupported resource name '%s'\n", resname);

That is much cleaner. It requires patching up the existing resctrl_schema to provide
a ->parser_line()  ... also a ->show_line() function.

If this proceeds, I can make the changes to provide this consistent interface. Thanks.

>> The driver provides a way to set param1, param2, param3 for each of
>> the A, B, C, D instances.
>
>Seems like these could be some new RFTYPE_RES_XYZ files?

In my current implementation the driver can make some/all of the files added
under the info/ directory writable. So this XYZ driver uses info/XYZ/param* files
as the interface for the user to set the parameters.

>
>On the model specific motivation topic: If the goal of the driver
>interface is to support model specific changes to resctrl there is a
>publicly available use case for consideration:
>https://lore.kernel.org/lkml/20230421141723.2405942-1-peternewman@google.com/
>
>It is not obvious to me how that model specific problem could be solved
>with this driver interface. Your insight here would be appreciated but it
>seems to me that we will have to keep accommodating model specific code
>in resctrl.

All my drivers are currently related to the control features of resctrl rather than the
monitor features. I don't see a way for a model specific driver to step in and mitigate
that particular problem that Peter has.

Seems quite hacky though. The trick with the "hard" RMID per core is going to miscount
LLC eviction traffic. If a core is switching rapidly between tasks with different RMIDs
that could be a significant difference from actual values.


But requests for some model specific quirks to work around h/w limitations doesn't
mean that we shouldn't create a driver layer for cases where it is possible to
separate s/w into layers.

-Tony

Powered by blists - more mailing lists