Re: [PATCH v1 30/31] x86/resctrl: Move the filesystem bits to headers visible to fs/resctrl

[Dave Martin]

On Mon, Apr 08, 2024 at 08:42:00PM -0700, Reinette Chatre wrote:
> Hi James,
> 
> On 3/21/2024 9:51 AM, James Morse wrote:
> ..
> > diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> > index 4788bd95dac6..fe0b10b589c0 100644
> > --- a/include/linux/resctrl_types.h
> > +++ b/include/linux/resctrl_types.h
> > @@ -7,6 +7,36 @@
> >  #ifndef __LINUX_RESCTRL_TYPES_H
> >  #define __LINUX_RESCTRL_TYPES_H
> >  
> > +#define CQM_LIMBOCHECK_INTERVAL	1000
> > +
> > +#define MBM_CNTR_WIDTH_BASE		24
> > +#define MBM_OVERFLOW_INTERVAL		1000
> > +#define MAX_MBA_BW			100u
> > +#define MBA_IS_LINEAR			0x4
> > +
> > +/* rdtgroup.flags */
> > +#define	RDT_DELETED		1
> > +
> > +/* rftype.flags */
> > +#define RFTYPE_FLAGS_CPUS_LIST	1
> > +
> > +/*
> > + * Define the file type flags for base and info directories.
> > + */
> > +#define RFTYPE_INFO			BIT(0)
> > +#define RFTYPE_BASE			BIT(1)
> > +#define RFTYPE_CTRL			BIT(4)
> > +#define RFTYPE_MON			BIT(5)
> > +#define RFTYPE_TOP			BIT(6)
> > +#define RFTYPE_RES_CACHE		BIT(8)
> > +#define RFTYPE_RES_MB			BIT(9)
> > +#define RFTYPE_DEBUG			BIT(10)
> > +#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
> > +#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
> > +#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
> > +#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
> > +#define RFTYPE_MON_BASE			(RFTYPE_BASE | RFTYPE_MON)
> > +
> >  /* Reads to Local DRAM Memory */
> >  #define READS_TO_LOCAL_MEM		BIT(0)
> >  
> 
> Not all these new seem to belong in this file. Could you please confirm?
> 
> For example:
> Earlier in series it was mentioned that struct rdtgroup is private to the
> fs so having RDT_DELETED is unexpected as it implies access to struct rdtgroup.
> 
> CQM_LIMBOCHECK_INTERVAL seems private to the fs code, so too
> RFTYPE_FLAGS_CPUS_LIST.
> 
> Reinette
> 

I'll flag this for James to review.

These have to be moved out of the x86 private headers, but you're right
that some of them seem logically private to the resctrl core.

I guess some of these could move to fs/resctrl/internal.h?

OTOH, might it be preferable to keep all the flag definitions for a
given member together for ease of maintenance, even if some are for
resctrl internal use only?

Cheers
---Dave