[PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl

James Morse posted 31 patches 1 year, 10 months ago
There is a newer version of this series
MAINTAINERS                               |    2 +
arch/Kconfig                              |    8 +
arch/x86/Kconfig                          |    5 +-
arch/x86/include/asm/resctrl.h            |   45 +-
arch/x86/kernel/cpu/resctrl/Makefile      |    5 +-
arch/x86/kernel/cpu/resctrl/core.c        |  119 +-
arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  506 +--
arch/x86/kernel/cpu/resctrl/internal.h    |  433 +--
arch/x86/kernel/cpu/resctrl/monitor.c     |  813 +----
arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 1126 +-----
arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 3929 +-------------------
arch/x86/kernel/process_32.c              |    2 +-
arch/x86/kernel/process_64.c              |    2 +-
fs/Kconfig                                |    1 +
fs/Makefile                               |    1 +
fs/resctrl/Kconfig                        |   23 +
fs/resctrl/Makefile                       |    3 +
fs/resctrl/ctrlmondata.c                  |  527 +++
fs/resctrl/internal.h                     |  340 ++
fs/resctrl/monitor.c                      |  843 +++++
fs/resctrl/psuedo_lock.c                  | 1122 ++++++
fs/resctrl/rdtgroup.c                     | 4013 +++++++++++++++++++++
include/linux/resctrl.h                   |  153 +-
include/linux/resctrl_types.h             |   98 +
24 files changed, 7244 insertions(+), 6875 deletions(-)
create mode 100644 fs/resctrl/Kconfig
create mode 100644 fs/resctrl/Makefile
create mode 100644 fs/resctrl/ctrlmondata.c
create mode 100644 fs/resctrl/internal.h
create mode 100644 fs/resctrl/monitor.c
create mode 100644 fs/resctrl/psuedo_lock.c
create mode 100644 fs/resctrl/rdtgroup.c
create mode 100644 include/linux/resctrl_types.h
[PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by James Morse 1 year, 10 months ago
Hello!

This is the final series that allows other architectures to implement resctrl.
The last patch just moves the code, and its a bit of a monster. I don't expect
that to get merged as part of this series - we should wait for it to make
less impact on other series. It's included here to show what gets moved, and
that structures/function-prototypes have the right visibility.

Otherwise this series renames functions and moves code around. With the
exception of invalid configurations for the configurable-events, there should
be no changes in behaviour caused by this series.

The driving pattern is to make things like struct rdtgroup private to resctrl.
Features like pseudo-lock aren't going to work on arm64, the ability to disable
it at compile time is added.

After this, I can start posting the MPAM driver to make use of resctrl on arm64.
(What's MPAM? See the cover letter of the first series. [1])

This series is based on Linus' commit 23956900041d and can be retrieved from:
https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git
mpam/move_to_fs/v1

Sorry for the mid-merge window base, I'm away for a few weeks - this should
rebase trivially onto rc1.

As ever - bugs welcome,
Thanks,

James

[1] https://lore.kernel.org/lkml/20201030161120.227225-1-james.morse@arm.com/

James Morse (31):
  x86/resctrl: Fix allocation of cleanest CLOSID on platforms with no
    monitors
  x86/resctrl: Add a helper to avoid reaching into the arch code
    resource list
  x86/resctrl: Move ctrlval string parsing policy away from the arch
    code
  x86/resctrl: Add helper for setting CPU default properties
  x86/resctrl: Remove rdtgroup from update_cpu_closid_rmid()
  x86/resctrl: Export resctrl fs's init function
  x86/resctrl: Wrap resctrl_arch_find_domain() around rdt_find_domain()
  x86/resctrl: Move resctrl types to a separate header
  x86/resctrl: Add a resctrl helper to reset all the resources
  x86/resctrl: Move monitor init work to a resctrl init call
  x86/resctrl: Move monitor exit work to a restrl exit call
  x86/resctrl: Move max_{name,data}_width into resctrl code
  x86/resctrl: Stop using the for_each_*_rdt_resource() walkers
  x86/resctrl: Export the is_mbm_*_enabled() helpers to asm/resctrl.h
  x86/resctrl: Add resctrl_arch_is_evt_configurable() to abstract BMEC
  x86/resctrl: Change mon_event_config_{read,write}() to be arch helpers
  x86/resctrl: Move mbm_cfg_mask to struct rdt_resource
  x86/resctrl: Allow resctrl_arch_mon_event_config_write() to return an
    error
  x86/resctrl: Add resctrl_arch_ prefix to pseudo lock functions
  x86/resctrl: Allow an architecture to disable pseudo lock
  x86/resctrl: Make prefetch_disable_bits belong to the arch code
  x86/resctrl: Make resctrl_arch_pseudo_lock_fn() take a plr
  x86/resctrl: Move thread_throttle_mode_init() to be managed by resctrl
  x86/resctrl: Move get_config_index() to a header
  x86/resctrl: Claim get_domain_from_cpu() for resctrl
  x86/resctrl: Describe resctrl's bitmap size assumptions
  x86/resctrl: Rename resctrl_sched_in() to begin resctrl_arch_
  x86/resctrl: Drop __init/__exit on assorted symbols
  fs/resctrl: Add boiler plate for external resctrl code
  x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl
  x86/resctrl: Move the resctrl filesystem code to /fs/resctrl

 MAINTAINERS                               |    2 +
 arch/Kconfig                              |    8 +
 arch/x86/Kconfig                          |    5 +-
 arch/x86/include/asm/resctrl.h            |   45 +-
 arch/x86/kernel/cpu/resctrl/Makefile      |    5 +-
 arch/x86/kernel/cpu/resctrl/core.c        |  119 +-
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  506 +--
 arch/x86/kernel/cpu/resctrl/internal.h    |  433 +--
 arch/x86/kernel/cpu/resctrl/monitor.c     |  813 +----
 arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 1126 +-----
 arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 3929 +-------------------
 arch/x86/kernel/process_32.c              |    2 +-
 arch/x86/kernel/process_64.c              |    2 +-
 fs/Kconfig                                |    1 +
 fs/Makefile                               |    1 +
 fs/resctrl/Kconfig                        |   23 +
 fs/resctrl/Makefile                       |    3 +
 fs/resctrl/ctrlmondata.c                  |  527 +++
 fs/resctrl/internal.h                     |  340 ++
 fs/resctrl/monitor.c                      |  843 +++++
 fs/resctrl/psuedo_lock.c                  | 1122 ++++++
 fs/resctrl/rdtgroup.c                     | 4013 +++++++++++++++++++++
 include/linux/resctrl.h                   |  153 +-
 include/linux/resctrl_types.h             |   98 +
 24 files changed, 7244 insertions(+), 6875 deletions(-)
 create mode 100644 fs/resctrl/Kconfig
 create mode 100644 fs/resctrl/Makefile
 create mode 100644 fs/resctrl/ctrlmondata.c
 create mode 100644 fs/resctrl/internal.h
 create mode 100644 fs/resctrl/monitor.c
 create mode 100644 fs/resctrl/psuedo_lock.c
 create mode 100644 fs/resctrl/rdtgroup.c
 create mode 100644 include/linux/resctrl_types.h

-- 
2.39.2
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Reinette Chatre 1 year, 10 months ago
Hi James and x86 Maintainers,

Please consider the file movements as captured in the diffstat below:

On 3/21/2024 9:50 AM, James Morse wrote:
>  MAINTAINERS                               |    2 +
>  arch/Kconfig                              |    8 +
>  arch/x86/Kconfig                          |    5 +-
>  arch/x86/include/asm/resctrl.h            |   45 +-
>  arch/x86/kernel/cpu/resctrl/Makefile      |    5 +-
>  arch/x86/kernel/cpu/resctrl/core.c        |  119 +-
>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  506 +--
>  arch/x86/kernel/cpu/resctrl/internal.h    |  433 +--
>  arch/x86/kernel/cpu/resctrl/monitor.c     |  813 +----
>  arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 1126 +-----
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 3929 +-------------------
>  arch/x86/kernel/process_32.c              |    2 +-
>  arch/x86/kernel/process_64.c              |    2 +-
>  fs/Kconfig                                |    1 +
>  fs/Makefile                               |    1 +
>  fs/resctrl/Kconfig                        |   23 +
>  fs/resctrl/Makefile                       |    3 +
>  fs/resctrl/ctrlmondata.c                  |  527 +++
>  fs/resctrl/internal.h                     |  340 ++
>  fs/resctrl/monitor.c                      |  843 +++++
>  fs/resctrl/psuedo_lock.c                  | 1122 ++++++

(sidenote: James, please note typo in psuedo_lock.c)

>  fs/resctrl/rdtgroup.c                     | 4013 +++++++++++++++++++++
>  include/linux/resctrl.h                   |  153 +-
>  include/linux/resctrl_types.h             |   98 +
>  24 files changed, 7244 insertions(+), 6875 deletions(-)
>  create mode 100644 fs/resctrl/Kconfig
>  create mode 100644 fs/resctrl/Makefile
>  create mode 100644 fs/resctrl/ctrlmondata.c
>  create mode 100644 fs/resctrl/internal.h
>  create mode 100644 fs/resctrl/monitor.c
>  create mode 100644 fs/resctrl/psuedo_lock.c
>  create mode 100644 fs/resctrl/rdtgroup.c
>  create mode 100644 include/linux/resctrl_types.h

I would like to check in on the sentiments regarding maintaining the resctrl
contributions after this work is merged. Considering that resctrl will 
be split between fs/resctrl and arch/x86, would it still be acceptable for
resctrl code to both areas (filesystem as well as arch) to flow via the tip tree with
help from x86 maintainers?

How will Arm patches flow?

James, are you planning a separate MAINTAINERS entry for the Arm specific code?
I would like to propose that you add yourself as a reviewer to the existing resctrl
MAINTAINERS entry to learn when any changes are submitted that may impact Arm. 

Reinette
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by James Morse 1 year, 7 months ago
Hi Reinette,

On 09/04/2024 04:13, Reinette Chatre wrote:
> Please consider the file movements as captured in the diffstat below:
> 
> On 3/21/2024 9:50 AM, James Morse wrote:
>>  MAINTAINERS                               |    2 +
>>  arch/Kconfig                              |    8 +
>>  arch/x86/Kconfig                          |    5 +-
>>  arch/x86/include/asm/resctrl.h            |   45 +-
>>  arch/x86/kernel/cpu/resctrl/Makefile      |    5 +-
>>  arch/x86/kernel/cpu/resctrl/core.c        |  119 +-
>>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  506 +--
>>  arch/x86/kernel/cpu/resctrl/internal.h    |  433 +--
>>  arch/x86/kernel/cpu/resctrl/monitor.c     |  813 +----
>>  arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 1126 +-----
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 3929 +-------------------
>>  arch/x86/kernel/process_32.c              |    2 +-
>>  arch/x86/kernel/process_64.c              |    2 +-
>>  fs/Kconfig                                |    1 +
>>  fs/Makefile                               |    1 +
>>  fs/resctrl/Kconfig                        |   23 +
>>  fs/resctrl/Makefile                       |    3 +
>>  fs/resctrl/ctrlmondata.c                  |  527 +++
>>  fs/resctrl/internal.h                     |  340 ++
>>  fs/resctrl/monitor.c                      |  843 +++++
>>  fs/resctrl/psuedo_lock.c                  | 1122 ++++++
> 
> (sidenote: James, please note typo in psuedo_lock.c)

Yeah - I still can't spell that...


>>  fs/resctrl/rdtgroup.c                     | 4013 +++++++++++++++++++++
>>  include/linux/resctrl.h                   |  153 +-
>>  include/linux/resctrl_types.h             |   98 +
>>  24 files changed, 7244 insertions(+), 6875 deletions(-)
>>  create mode 100644 fs/resctrl/Kconfig
>>  create mode 100644 fs/resctrl/Makefile
>>  create mode 100644 fs/resctrl/ctrlmondata.c
>>  create mode 100644 fs/resctrl/internal.h
>>  create mode 100644 fs/resctrl/monitor.c
>>  create mode 100644 fs/resctrl/psuedo_lock.c
>>  create mode 100644 fs/resctrl/rdtgroup.c
>>  create mode 100644 include/linux/resctrl_types.h

> I would like to check in on the sentiments regarding maintaining the resctrl
> contributions after this work is merged. Considering that resctrl will 
> be split between fs/resctrl and arch/x86, would it still be acceptable for
> resctrl code to both areas (filesystem as well as arch) to flow via the tip tree with
> help from x86 maintainers?

I think it makes sense for all that to be funnelled via tip.


> How will Arm patches flow?

No preference. If it makes the most sense for them to go via tip, then lets do that.

There will be the occasional dependency on arm64, but that should be manageable.
e.g. The first merge of the MPAM driver comes with some arm64 code for context-switch.
That will need co-ordinating with Will+Catalin.


> James, are you planning a separate MAINTAINERS entry for the Arm specific code?
> I would like to propose that you add yourself as a reviewer to the existing resctrl
> MAINTAINERS entry to learn when any changes are submitted that may impact Arm. 

I'll add a patch doing that to the end of the MPAM tree.


Thanks,

James
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Dave Martin 1 year, 9 months ago
Hi Reinette,

Since we seem to be getting to the end of the outstanding discussion on
this series, what's your preference for moving this forward?

James will be back towards the end of next week AIUI, so it should be
possible to apply most of the changes that have been discussed and post
a v2 for review fairly quickly after that.

I'm happy to apply my fixups branch and post an untested "v1.1" for
review if you think it would be useful to see the de-noised output of
the review so far, but I don't want to fork the discussion or create
unnecessary work.  This interim respin would be subject to what James
wants to pick up.

Alternatively, I can try to follow up on the string parsing discussion
from patch #3 with some real code if you'd prefer to get a clean
interface in place for that.

Cheers
---Dave


On Mon, Apr 08, 2024 at 08:13:19PM -0700, Reinette Chatre wrote:
> Hi James and x86 Maintainers,
> 
> Please consider the file movements as captured in the diffstat below:
> 
> On 3/21/2024 9:50 AM, James Morse wrote:
> >  MAINTAINERS                               |    2 +
> >  arch/Kconfig                              |    8 +
> >  arch/x86/Kconfig                          |    5 +-
> >  arch/x86/include/asm/resctrl.h            |   45 +-
> >  arch/x86/kernel/cpu/resctrl/Makefile      |    5 +-
> >  arch/x86/kernel/cpu/resctrl/core.c        |  119 +-
> >  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  506 +--
> >  arch/x86/kernel/cpu/resctrl/internal.h    |  433 +--
> >  arch/x86/kernel/cpu/resctrl/monitor.c     |  813 +----
> >  arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 1126 +-----
> >  arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 3929 +-------------------
> >  arch/x86/kernel/process_32.c              |    2 +-
> >  arch/x86/kernel/process_64.c              |    2 +-
> >  fs/Kconfig                                |    1 +
> >  fs/Makefile                               |    1 +
> >  fs/resctrl/Kconfig                        |   23 +
> >  fs/resctrl/Makefile                       |    3 +
> >  fs/resctrl/ctrlmondata.c                  |  527 +++
> >  fs/resctrl/internal.h                     |  340 ++
> >  fs/resctrl/monitor.c                      |  843 +++++
> >  fs/resctrl/psuedo_lock.c                  | 1122 ++++++
> 
> (sidenote: James, please note typo in psuedo_lock.c)
> 
> >  fs/resctrl/rdtgroup.c                     | 4013 +++++++++++++++++++++
> >  include/linux/resctrl.h                   |  153 +-
> >  include/linux/resctrl_types.h             |   98 +
> >  24 files changed, 7244 insertions(+), 6875 deletions(-)
> >  create mode 100644 fs/resctrl/Kconfig
> >  create mode 100644 fs/resctrl/Makefile
> >  create mode 100644 fs/resctrl/ctrlmondata.c
> >  create mode 100644 fs/resctrl/internal.h
> >  create mode 100644 fs/resctrl/monitor.c
> >  create mode 100644 fs/resctrl/psuedo_lock.c
> >  create mode 100644 fs/resctrl/rdtgroup.c
> >  create mode 100644 include/linux/resctrl_types.h
> 
> I would like to check in on the sentiments regarding maintaining the resctrl
> contributions after this work is merged. Considering that resctrl will 
> be split between fs/resctrl and arch/x86, would it still be acceptable for
> resctrl code to both areas (filesystem as well as arch) to flow via the tip tree with
> help from x86 maintainers?
> 
> How will Arm patches flow?
> 
> James, are you planning a separate MAINTAINERS entry for the Arm specific code?
> I would like to propose that you add yourself as a reviewer to the existing resctrl
> MAINTAINERS entry to learn when any changes are submitted that may impact Arm. 
> 
> Reinette
> 
>
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Reinette Chatre 1 year, 9 months ago
Hi Dave,

On 4/18/2024 8:32 AM, Dave Martin wrote:
> Since we seem to be getting to the end of the outstanding discussion on
> this series, what's your preference for moving this forward?
> 
> James will be back towards the end of next week AIUI, so it should be
> possible to apply most of the changes that have been discussed and post
> a v2 for review fairly quickly after that.
> 
> I'm happy to apply my fixups branch and post an untested "v1.1" for
> review if you think it would be useful to see the de-noised output of
> the review so far, but I don't want to fork the discussion or create
> unnecessary work.  This interim respin would be subject to what James
> wants to pick up.

Please post v2 with discussions complete and feedback addressed
(unless there are some discussions that need to see how other changes turn out
before they can complete but I do not think this applies to this series).

To create some expectations, when you submit a new series it will go to the back of
my review queue and at this time there are three other resctrl pieces of work
waiting for review (btw ... one of them, [1], is waiting for feedback from Arm). 
 
> Alternatively, I can try to follow up on the string parsing discussion
> from patch #3 with some real code if you'd prefer to get a clean
> interface in place for that.

We can keep discussing that, sure.

Reinette

[1] https://lore.kernel.org/lkml/cover.1711674410.git.babu.moger@amd.com/
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Moger, Babu 1 year, 9 months ago
Hi Reinette,

On 4/19/24 23:06, Reinette Chatre wrote:
> Hi Dave,
> 
> On 4/18/2024 8:32 AM, Dave Martin wrote:
>> Since we seem to be getting to the end of the outstanding discussion on
>> this series, what's your preference for moving this forward?
>>
>> James will be back towards the end of next week AIUI, so it should be
>> possible to apply most of the changes that have been discussed and post
>> a v2 for review fairly quickly after that.
>>
>> I'm happy to apply my fixups branch and post an untested "v1.1" for
>> review if you think it would be useful to see the de-noised output of
>> the review so far, but I don't want to fork the discussion or create
>> unnecessary work.  This interim respin would be subject to what James
>> wants to pick up.
> 
> Please post v2 with discussions complete and feedback addressed
> (unless there are some discussions that need to see how other changes turn out
> before they can complete but I do not think this applies to this series).
> 
> To create some expectations, when you submit a new series it will go to the back of
> my review queue and at this time there are three other resctrl pieces of work
> waiting for review (btw ... one of them, [1], is waiting for feedback from Arm). 
>  
>> Alternatively, I can try to follow up on the string parsing discussion
>> from patch #3 with some real code if you'd prefer to get a clean
>> interface in place for that.
> 
> We can keep discussing that, sure.
> 
> Reinette
> 
> [1] https://lore.kernel.org/lkml/cover.1711674410.git.babu.moger@amd.com/

Do you have any more feedback on this series. I have few feedbacks from
Peter. I was planning to work on v4 of this series.

-- 
Thanks
Babu Moger
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Reinette Chatre 1 year, 9 months ago
Hi Babu and Dave,

On 4/22/2024 6:51 AM, Moger, Babu wrote:
> On 4/19/24 23:06, Reinette Chatre wrote:
>>
>> [1] https://lore.kernel.org/lkml/cover.1711674410.git.babu.moger@amd.com/
> 
> Do you have any more feedback on this series. I have few feedbacks from
> Peter. I was planning to work on v4 of this series.
> 

Babu: It is difficult to start drilling into the implementation before there
is agreement on the interface. One reason you went through the effort of
the first few iterations was to accommodate Arm's use cases as we understand
it, but we need to hear from Arm if we are on the right track here.
I do hope that we will hear something in the next couple of weeks. 

Dave: Could you please check in if the interface introduced [1] is something
of interest to Arm? If it is not, we can proceed with the implementation without
trying to consider how Arm may use/need such an interface. If it is, could you
please let us know when we can expect feedback from Arm? 

Dave: fyi ... there is a similar scenario with [2] that aims to address some Arm
feedback ([3]). Do not be deceived by version number of [2], which is categorized
as a "new approach" aiming to accommodate feedback. It has a long history [4].

Reinette  

[2] https://lore.kernel.org/lkml/20240327200352.236835-1-tony.luck@intel.com/
[3] https://lore.kernel.org/lkml/88430722-67b3-4f7d-8db2-95ee52b6f0b0@arm.com/
[4] https://lore.kernel.org/lkml/20240312214247.91772-1-tony.luck@intel.com/
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Peter Newman 1 year, 9 months ago
Hi Dave,

On Mon, Apr 22, 2024 at 9:01 AM Reinette Chatre
<reinette.chatre@intel.com> wrote:
>
> Hi Babu and Dave,
>
> On 4/22/2024 6:51 AM, Moger, Babu wrote:
> > On 4/19/24 23:06, Reinette Chatre wrote:
> >>
> >> [1] https://lore.kernel.org/lkml/cover.1711674410.git.babu.moger@amd.com/
> >
> > Do you have any more feedback on this series. I have few feedbacks from
> > Peter. I was planning to work on v4 of this series.
> >
>
> Babu: It is difficult to start drilling into the implementation before there
> is agreement on the interface. One reason you went through the effort of
> the first few iterations was to accommodate Arm's use cases as we understand
> it, but we need to hear from Arm if we are on the right track here.
> I do hope that we will hear something in the next couple of weeks.
>
> Dave: Could you please check in if the interface introduced [1] is something
> of interest to Arm? If it is not, we can proceed with the implementation without
> trying to consider how Arm may use/need such an interface. If it is, could you
> please let us know when we can expect feedback from Arm?

Because MPAM implementations typically expose an MSC for each DRAM
channel, there is an alternate strategy we can use for the monitor
scalability problem:

When a single DRAM MSC does not provide enough monitors to track all
of the supported PARTID x PMG combinations simultaneously, the DRAM
MSCs collectively may provide a sufficient number of monitors.
Therefore, as long as the distribution of traffic among the DRAM
channels is uniform (or predictably non-uniform), it's possible to
estimate the total bandwidth with sufficient accuracy.

-Peter
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Dave Martin 1 year, 9 months ago
On Mon, Apr 22, 2024 at 11:39:00AM -0700, Peter Newman wrote:
> Hi Dave,
> 
> On Mon, Apr 22, 2024 at 9:01 AM Reinette Chatre
> <reinette.chatre@intel.com> wrote:
> >
> > Hi Babu and Dave,
> >
> > On 4/22/2024 6:51 AM, Moger, Babu wrote:
> > > On 4/19/24 23:06, Reinette Chatre wrote:
> > >>
> > >> [1] https://lore.kernel.org/lkml/cover.1711674410.git.babu.moger@amd.com/
> > >
> > > Do you have any more feedback on this series. I have few feedbacks from
> > > Peter. I was planning to work on v4 of this series.
> > >
> >
> > Babu: It is difficult to start drilling into the implementation before there
> > is agreement on the interface. One reason you went through the effort of
> > the first few iterations was to accommodate Arm's use cases as we understand
> > it, but we need to hear from Arm if we are on the right track here.
> > I do hope that we will hear something in the next couple of weeks.
> >
> > Dave: Could you please check in if the interface introduced [1] is something
> > of interest to Arm? If it is not, we can proceed with the implementation without
> > trying to consider how Arm may use/need such an interface. If it is, could you
> > please let us know when we can expect feedback from Arm?
> 
> Because MPAM implementations typically expose an MSC for each DRAM
> channel, there is an alternate strategy we can use for the monitor
> scalability problem:
> 
> When a single DRAM MSC does not provide enough monitors to track all
> of the supported PARTID x PMG combinations simultaneously, the DRAM
> MSCs collectively may provide a sufficient number of monitors.
> Therefore, as long as the distribution of traffic among the DRAM
> channels is uniform (or predictably non-uniform), it's possible to
> estimate the total bandwidth with sufficient accuracy.
> 
> -Peter
> 

So you're suggesting that if (say) DRAM traffic is striped symmetrically
across N channels, and each has counters, then a counter matching
PARTID:PMG on just one channel should given an unbiased estimate of the
traffic from that group (with some sacrifice of precision, and assuming
the workload is non-pathological)?

I guess that could work, though this might work badly for some workloads
and might give a malicious workload a way to hide transactions from
monitoring if the placement of the counter is too fixed and/or
predictable.

Cheers
---Dave
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Moger, Babu 1 year, 9 months ago

On 4/22/24 11:01, Reinette Chatre wrote:
> Hi Babu and Dave,
> 
> On 4/22/2024 6:51 AM, Moger, Babu wrote:
>> On 4/19/24 23:06, Reinette Chatre wrote:
>>>
>>> [1] https://lore.kernel.org/lkml/cover.1711674410.git.babu.moger@amd.com/
>>
>> Do you have any more feedback on this series. I have few feedbacks from
>> Peter. I was planning to work on v4 of this series.
>>
> 
> Babu: It is difficult to start drilling into the implementation before there
> is agreement on the interface. One reason you went through the effort of
> the first few iterations was to accommodate Arm's use cases as we understand
> it, but we need to hear from Arm if we are on the right track here.
> I do hope that we will hear something in the next couple of weeks. 

Sure. Sounds good.
-- 
Thanks
Babu Moger
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Dave Martin 1 year, 10 months ago
On Mon, Apr 08, 2024 at 08:13:19PM -0700, Reinette Chatre wrote:
> Hi James and x86 Maintainers,
> 
> Please consider the file movements as captured in the diffstat below:
> 
> On 3/21/2024 9:50 AM, James Morse wrote:
> >  MAINTAINERS                               |    2 +
> >  arch/Kconfig                              |    8 +
> >  arch/x86/Kconfig                          |    5 +-
> >  arch/x86/include/asm/resctrl.h            |   45 +-
> >  arch/x86/kernel/cpu/resctrl/Makefile      |    5 +-
> >  arch/x86/kernel/cpu/resctrl/core.c        |  119 +-
> >  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  506 +--
> >  arch/x86/kernel/cpu/resctrl/internal.h    |  433 +--
> >  arch/x86/kernel/cpu/resctrl/monitor.c     |  813 +----
> >  arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 1126 +-----
> >  arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 3929 +-------------------
> >  arch/x86/kernel/process_32.c              |    2 +-
> >  arch/x86/kernel/process_64.c              |    2 +-
> >  fs/Kconfig                                |    1 +
> >  fs/Makefile                               |    1 +
> >  fs/resctrl/Kconfig                        |   23 +
> >  fs/resctrl/Makefile                       |    3 +
> >  fs/resctrl/ctrlmondata.c                  |  527 +++
> >  fs/resctrl/internal.h                     |  340 ++
> >  fs/resctrl/monitor.c                      |  843 +++++
> >  fs/resctrl/psuedo_lock.c                  | 1122 ++++++
> 
> (sidenote: James, please note typo in psuedo_lock.c)

Noted.

(So that's what the Psuedo Lock key on the keyboard does...)

> 
> >  fs/resctrl/rdtgroup.c                     | 4013 +++++++++++++++++++++
> >  include/linux/resctrl.h                   |  153 +-
> >  include/linux/resctrl_types.h             |   98 +
> >  24 files changed, 7244 insertions(+), 6875 deletions(-)
> >  create mode 100644 fs/resctrl/Kconfig
> >  create mode 100644 fs/resctrl/Makefile
> >  create mode 100644 fs/resctrl/ctrlmondata.c
> >  create mode 100644 fs/resctrl/internal.h
> >  create mode 100644 fs/resctrl/monitor.c
> >  create mode 100644 fs/resctrl/psuedo_lock.c
> >  create mode 100644 fs/resctrl/rdtgroup.c
> >  create mode 100644 include/linux/resctrl_types.h
> 
> I would like to check in on the sentiments regarding maintaining the resctrl
> contributions after this work is merged. Considering that resctrl will 
> be split between fs/resctrl and arch/x86, would it still be acceptable for
> resctrl code to both areas (filesystem as well as arch) to flow via the tip tree with
> help from x86 maintainers?
> 
> How will Arm patches flow?
> 
> James, are you planning a separate MAINTAINERS entry for the Arm specific code?
> I would like to propose that you add yourself as a reviewer to the existing resctrl
> MAINTAINERS entry to learn when any changes are submitted that may impact Arm. 
> 
> Reinette

I'll leave this for James to respond.

Cheers
---Dave
Re: [PATCH v1 00/31] x86/resctrl: Move the resctrl filesystem code to /fs/resctrl
Posted by Dave Martin 1 year, 10 months ago
On Thu, Mar 21, 2024 at 04:50:35PM +0000, James Morse wrote:
> Hello!
> 
> This is the final series that allows other architectures to implement resctrl.
> The last patch just moves the code, and its a bit of a monster. I don't expect
> that to get merged as part of this series - we should wait for it to make
> less impact on other series. It's included here to show what gets moved, and
> that structures/function-prototypes have the right visibility.
> 
> Otherwise this series renames functions and moves code around. With the
> exception of invalid configurations for the configurable-events, there should
> be no changes in behaviour caused by this series.
> 
> The driving pattern is to make things like struct rdtgroup private to resctrl.
> Features like pseudo-lock aren't going to work on arm64, the ability to disable
> it at compile time is added.
> 
> After this, I can start posting the MPAM driver to make use of resctrl on arm64.
> (What's MPAM? See the cover letter of the first series. [1])
> 
> This series is based on Linus' commit 23956900041d and can be retrieved from:
> https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git
> mpam/move_to_fs/v1
> 
> Sorry for the mid-merge window base, I'm away for a few weeks - this should
> rebase trivially onto rc1.
> 
> As ever - bugs welcome,
> Thanks,
> 
> James
> 
> [1] https://lore.kernel.org/lkml/20201030161120.227225-1-james.morse@arm.com/

[...]

Hi all,

I'm going to try to give a preliminary response to review feedback while
James is away, to help move this series forward.

Many thanks to those who have reviewed / commented so far!


Apologies for the slow-ish response from me: I'm still not that familiar
with this code, so I wanted to make sure I had broadly covered most of
the simpler comments before I started sending replies.

N.B.: where I say "Noted", I mean that I've stashed a fixup for James
to review when he gets back.  Most of these seem straightforward and
uncontroversial, but it's his code, so it will be up to him what
changes actually get incorporated in the respin.

Cheers
---Dave