RE: [PATCH v18 06/17] x86/resctrl: Introduce snc_nodes_per_l3_cache
From: Luck, Tony
Date: Thu May 23 2024 - 19:19:04 EST
> > So (in arch/x86/kernel/cpu/resctrl/monitor.c)
> >
> > static int snc_nodes_per_l3_cache = 1;
> >
> > Set and use only in this (arch specific) file.
>
> Since this series initializes this value in
> arch/x86/kernel/cpu/resctrl/core.c it is not clear to
> me from just this one line how you envision the changes.
v18 did the initialization in core.c. But since SNC is all about monitor
features it looks more logical to do this here:
resctrl_late_init()
get_rdt_resources()
get_rdt_mon_resources()
rdt_get_mon_l3_config() <<<< Do SNC enumeration here
> Just to be clear ... when I refer to arch specific and
> filesystem code I am considering how this series integrates with [1]
> since that is the direction resctrl is headed in.
> Being "arch specific" thus does not require that it be moved into
> monitor.c - it could be added to arch/x86/kernel/cpu/resctrl/internal.h
> where it can remain after the fs/arch split.
The logical place to convert from logical RMID to physical RMID looks
to be in __rmid_read(). Just need to pass in the domain pointer (from
both resctrl_arch_reset_rmid() and resctrl_arch_rmid_read().
>
> It will be very helpful if you view your series while taking
> [1] into account. For example, when looking at [1] you will see that
> mon_event_count() and __mon_event_count() are resctrl filesystem
> functions. When you consider that it should be clear that adding
> an arch specific get_node_rmid() between these functions will make
> the arch/fs split more difficult.
I'll try to keep that in mind as I rework my series. In v18 my "sum" code
went into __mon_event_count(). I'll see if I can push that down another
level.
-Tony