Message-ID: <6fb89a6b-031c-451e-80f7-58c277eda265@intel.com>
Date: Wed, 24 Jan 2024 14:23:40 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: Haifeng Xu <haifeng.xu@...pee.com>
CC: <fenghua.yu@...el.com>, <babu.moger@....com>, <peternewman@...gle.com>,
	<x86@...nel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] x86/resctrl: Display the number of available CLOSIDs

Hi Haifeng,

Thank you for sharing your requirements as well as submitting
changes to support them.

I would like to start with a high level overview that applies
to all three patches you submitted. This relates to customs,
formatting, and style that will help your contributions get
into the kernel.

In your next submission, please do submit your patches together
as a series with a cover letter. This means that your series
starts with a cover letter (think of it as "patch #0") and the
patches are sent in reply to that cover letter. This is described
in more detail in "Documentation/process/5.Posting.rst".

Regarding the patches themselves. Please read and follow
Documentation/process/coding-style.rst and
Documentation/process/maintainer-tip.rst regarding customs.
The former is a general document that applies to the whole kernel
while the latter contains more specific customs related to the
area you are are contributing to here.

As a final comment, please ensure that your patches
pass a "scripts/checkpatch.pl --strict" check. There are more
details about this in "Documentation/process/5.Posting.rst".

While the documents mentioned above should get you started there
is a lot of other valuable information in "Documentation/process".
Consider the index in that directory to help you navigate through
the available topics.

On 1/23/2024 1:20 AM, Haifeng Xu wrote:
> We can know the number of CLOSIDs for each rdt resource, for example:
> 
> cat /sys/fs/resctrl/info/L3/num_closids
> cat /sys/fs/resctrl/info/MB/num_closids
> ...
> 
> The number of available CLOSIDs is the minimal value of them. When users
> try to create new control groups, to avoid running out of CLOSIDs, they
> have to traverse /sys/fs/resctrl/ and count the number of directories.
> 
> To make things more easier, add a RFTYPE_TOP_INFO file 'free_closids'
> that tells users how many free closids are left.

I do not see this as a change that benefits the kernel or user space.
It sounds to me as though user space is planning some behavior based
on what it knows about the current kernel internals and requesting
more information to peek into these internals to make it easier
to do so. The kernel can always choose to do things different
internally, but it is required to maintain a consistent interface to
user space. We should thus always take great care with new interfaces.

>From what I can tell user space intends to use this "free_closids"
to mean "how many more control resource groups can be created". This
is not a contract that I think we should enter into. There has been
discussions aiming to disconnect the number of resource groups
from the number of closids (effectively letting resource groups
with the same resource allocations share a closid). This is something
that the kernel may still do at some point but sharing "free_closids"
knowing that user space intends to use it as a "number of resource groups
remaining" counter would make future enhancements like this difficult.

Could you please provide more detail in why this is required? User
space should not need to keep track to know how many groups can be
created, creating a new group will fail with ENOSPC if no more
groups can be created.

Reinette

Powered by blists - more mailing lists