[PATCH v19 11/20] x86/resctrl: Allocate a new bit in union mon_data_bits

Tony Luck posted 20 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH v19 11/20] x86/resctrl: Allocate a new bit in union mon_data_bits
Posted by Tony Luck 1 year, 6 months ago
When Sub-NUMA Cluster (SNC) mode is enabled the legacy monitor reporting
files must report the sum of the data from all of the SNC nodes that
share the L3 cache that is referenced by the monitor file.

Resctrl squeezes all the attributes of these files into 32-bits so they
can be stored in the "priv" field of struct kernfs_node.

Arbitrarily choose the "evtid" field to sacrifice one bit to make
space for a new bit. This structure is purely internal to resctrl,
no ABI issues with modifying it. Subsequent changes may rearrange the
allocation of bits between each of the fields as needed.

The stolen bit is given to a new "sum" field that indicates that reading
this file must sum across SNC nodes. This bit also indicates that the
domid field is the l3_cache_id (instead of a domain id) to find which
domains must be summed.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/resctrl/internal.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index d8156d22cbdc..7957fc38b71c 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -133,14 +133,20 @@ struct mon_evt {
  *                     as kernfs private data
  * @rid:               Resource id associated with the event file
  * @evtid:             Event id associated with the event file
- * @domid:             The domain to which the event file belongs
+ * @sum:               Set when event must be summed across multiple
+ *                     domains.
+ * @domid:             When @sum is zero this is the domain to which
+ *                     the event file belongs. When @sum is one this
+ *                     is the id of the L3 cache that all domains to be
+ *                     summed share.
  * @u:                 Name of the bit fields struct
  */
 union mon_data_bits {
 	void *priv;
 	struct {
 		unsigned int rid		: 10;
-		enum resctrl_event_id evtid	: 8;
+		enum resctrl_event_id evtid	: 7;
+		unsigned int sum		: 1;
 		unsigned int domid		: 14;
 	} u;
 };
-- 
2.45.0
Re: [PATCH v19 11/20] x86/resctrl: Allocate a new bit in union mon_data_bits
Posted by Reinette Chatre 1 year, 6 months ago
Hi Tony,

Regarding shortlog: isn't the purpose of this change to _avoid_
allocating a new bit?

On 5/28/24 3:19 PM, Tony Luck wrote:
> When Sub-NUMA Cluster (SNC) mode is enabled the legacy monitor reporting
> files must report the sum of the data from all of the SNC nodes that
> share the L3 cache that is referenced by the monitor file.
> 
> Resctrl squeezes all the attributes of these files into 32-bits so they
> can be stored in the "priv" field of struct kernfs_node.
> 
> Arbitrarily choose the "evtid" field to sacrifice one bit to make
> space for a new bit. This structure is purely internal to resctrl,

Missing explanation why this is ok for this field to sacrifice a bit.

> no ABI issues with modifying it. Subsequent changes may rearrange the
> allocation of bits between each of the fields as needed.
> 
> The stolen bit is given to a new "sum" field that indicates that reading

stolen? Is that necessary? It can just be "Give the bit ..." (also
note imperative tone)

> this file must sum across SNC nodes. This bit also indicates that the
> domid field is the l3_cache_id (instead of a domain id) to find which
> domains must be summed.

l3_cache_id looks like a variable that does not exist anywhere in this series
or in existing resctrl.

Reinette