Hi Dave,
On 2/13/25 9:37 AM, Dave Martin wrote:
Hi Reinette,
On Wed, Feb 12, 2025 at 03:33:31PM -0800, Reinette Chatre wrote:
Hi Dave,
On 2/12/25 9:46 AM, Dave Martin wrote:
Hi there,
On Wed, Jan 22, 2025 at 02:20:08PM -0600, Babu Moger wrote:
This series adds the support for Assignable Bandwidth Monitoring Counters
(ABMC). It is also called QoS RMID Pinning feature
Series is written such that it is easier to support other assignable
features supported from different vendors.
The feature details are documented in the APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable Bandwidth
Monitoring (ABMC). The documentation is available at
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
The patches are based on top of commit
d361b84d51bfe (tip/master) Merge branch into tip/master: 'x86/tdx'
[...]
b. Check how many ABMC counters are available.
# cat /sys/fs/resctrl/info/L3_MON/num_mbm_cntrs
32
Is this file needed?
With MPAM, it is more difficult to promise that the same number of
counters will be available everywhere.
Rather than lie, or report a "safe" value here that may waste some
counters, can we just allow the number of counters to be be discovered
per domain via available_mbm_cntrs?
This sounds reasonable to me. I think us having trouble with the
user documentation of this file so late in development should also have been
a sign to rethink its value.
For a user to discover the number of counters supported via available_mbm_cntrs
would require the file's contents to be captured right after mount. Since we've
had scenarios where new userspace needs to discover an up-and-running system's
configuration this may not be possible. I thus wonder instead of removing
num_mbm_cntrs, it could be modified to return the per-domain supported counters
instead of a single value?
Is it actually useful to be able to discover the number of counters
that exist? A counter that exists but is not available cannot be used,
so perhaps it is not useful to know about it in the first place.
An alternative perspective of what "available" means is "how many counters
could I possibly get to do this new monitoring task". A user may be willing
to re-assign counters if the new monitoring task is important. Knowing
how many counters are already free and available for assignment would be
easy from available_mbm_cntrs but to get an idea of how many counters
could be re-assigned to help out with the new task would require
some intricate parsing of mbm_assign_control.
But if we keep this file but make it report the number of counters for
each domain (similarly to mbm_available_cntrs), then I think the MPAM
driver should be able to work with that.
num_closids and num_rmids are already problematic for MPAM, so it would
be good to avoid any more parameters of this sort from being reported
to userspace unless there is a clear understanding of why they are
needed.
Yes. Appreciate your help in identifying what could be problematic for MPAM.
For clarity: this is a background issue, mostly orthogonal to this
series.
If this series is merged as-is, with a global per-resource
num_mbm_cntrs property, then this not really worse than the current
situation -- it's just a bit annoying from the MPAM perspective.
In a nutshell, the num_closids / num_rmids parameters seem to expose
RDT-specific hardware semantics to userspace, implying a specific
allocation model for control group and monitoring group identifiers.
The guarantees that userspace is entitled to asssume when resctrl
reports particular values do not seem to be well described and are hard
to map onto the nearest-equivalent MPAM implementation. A combination
of control and monitoring groups that can be created on x86 may not be
creatable on MPAM, even when the number of supportable control and
monitoring partitions is the same.
I understand. This interface was created almost a decade ago. It would have been
wonderful if the user interface could have been created with a clear vision
of all the use cases it would end up needing to support. I am trying to be
very careful with this new user interface as I try to consider all the things I
learned while working on resctrl. All help get this new interface right is
greatly appreciated.
Since your specifically mention issues that MPAM has with num_rmids, please
note that we have been trying (see [1], but maybe start reading thread at [2])
to find ways to make this work with MPAM but no word from MPAM side.
I see that you were not cc'd on the discussion so this is not a criticism of
you personally but I would like to highlight that we do try to make things
work well for MPAM but so far this work seems ignored, yet critisized
for not being done. I expect the more use cases are thrown at an interface
as it is developed the better it would get and I would gladly work with MPAM
folks to improve things.
Even with the ABMC series, we may still be constrained on what we can
report for num_rmids: we can't know in advance whether or not the user
is going to use mbm_cntr_assign mode -- if not, we can't promise to
create more monitoring groups than the number of counters in the
hardware.
It is the architecture that decides which modes are supported and
which is default.
It seems natural for the counts reported by "available_mbm_cntrs" to
change dynamically when the ABMC assignment mode is changed, but I
think userspace are likely to expect the global "num_rmids" parameters
to be fixed for the lifetime of the resctrl mount (and possibly fixed
for all time on a given hardware platform -- at least, modulo CDP).
I think it might be possible to tighten up the docmentation of
num_closids in particular in a way that doesn't conflict with x86 and
may make it easier for MPAM to fit in with, but that feels like a
separate conversation.
None of this should be considered a blocker for this series, either way.
Reporting number of counters per monitoring domain is a more natural
fit for MPAM, as below:
c. Check how many ABMC counters are available in each domain.
# cat /sys/fs/resctrl/info/L3_MON/available_mbm_cntrs
0=30;1=30
For MPAM, this seems supportable. Each monitoring domain will have
some counters, and a well-defined number of them will be available for
allocation at any one time.
[...]
e. This series adds a new interface file /sys/fs/resctrl/info/L3_MON/mbm_assign_control
[...]
Flags can be one of the following:
t MBM total event is enabled.
l MBM local event is enabled.
tl Both total and local MBM events are enabled.
_ None of the MBM events are enabled
Examples:
[...]
I think that this basically works for MPAM.
The local/total distinction doesn't map in a consistent way onto MPAM,
but this problem is not specific to ABMC. It feels sensible for ABMC
to be built around the same concepts that resctrl already has elsewhere
in the interface. MPAM will do its best to fit (as already).
Regarding Peter's use case of assiging multiple counters to a
monitoring group [1], I feel that it's probably good enough to make
sure that the ABMC interface can be extended in future in a backwards
compatible way so as to support this, without trying to support it
immediately.
[1] https://lore.kernel.org/lkml/CALPaoCjY-3f2tWvBjuaQPfoPhxveWxxCxHqQMn4BEaeBXBa0bA@xxxxxxxxxxxxxx/
I do not think that resctrl's current support of the mbm_total_bytes and
mbm_local_bytes should be considered as the "only" two available "slots"
into which all possible events should be forced into. "mon_features" exists
to guide user space to which events are supported and as I see it new events
can be listed here to inform user space of their availability, with their
associated event files available in the resource groups.
That's fair. I wasn't currently sure how (or if) the set of countable
events was expected to grow / evolve via this route.
Either way, I think this confirms that there is at least one viable way
to enable more counters for a single control group, on top of this
series.
(If there is more than one way, that seems fine?)
For example, if we added new generic "letters" -- say, "0" to "9",
combined with new counter files in resctrlfs, that feels like a
possible approach. ABMC (as in this series) should just reject such
such assignments, and the new counter files wouldn't exist.
Availability of this feature could also be reported as a distinct mode
in mbm_assign_mode, say "mbm_cntr_generic", or whatever.
A _sketch_ of this follows. This is NOT a proposal -- the key
question is whether we are confident that we can extend the interface
in this way in the future without breaking anything.
If "yes", then the ABMC interface (as proposed by this series) works as
a foundation to build on.
--8<--
[artists's impression]
# cat /sys/fs/resctrl/info/L3_MON/mbm_assign_mode
mbm_cntr_generic
[mbm_cntr_assign]
default
# echo mbm_cntr_generic >/sys/fs/resctrl/info/L3_MON/mbm_assign_mode
# echo '//0=01;1=23' >/sys/fs/resctrl/info/L3_MON/mbm_assign_control
# echo t >/sys/fs/resctrl/info/L3_MON/mbm_counter0_bytes_type
# echo l >/sys/fs/resctrl/info/L3_MON/mbm_counter1_bytes_type
# echo t >/sys/fs/resctrl/info/L3_MON/mbm_counter2_bytes_type
# echo l >/sys/fs/resctrl/info/L3_MON/mbm_counter3_bytes_type
...
# cat /sys/fs/resctrl/mon_data/mon_L3_00/mbm_counter1_bytes
etc.
It is not clear to me what additional features such an interface enables. It
also looks like user space will need to track and manage counter IDs?
My idea was that for these generic counters, new files could be exposed
to configure what they actually count (the ..._type files shown above;
or possibly via the ..._config files that already exist).
The "IDs" were inteded as abstract; the number only relates the
assignments in mbm_assign_control to the files created elsewhere. This
wouldn't be related to IDs assigned by the hardware.
I see. Yes, this sounds related to and a generalization of the AMD
configurable event feature.
If there are multiple resctrl users then using numeric IDs might be
problematic; though if we go eventually in the direction of making
resctrlfs multi-mountable then each mount could have its own namespace.
I am not aware of "multi-mountable" direction.
Allowing counters to be named and configured with a mkdir()-style
interface might be possible too; that might make it easier for users to
coexist within a single resctrl mount (if we think that's important
enough).
It sounds to me as though the issue starts with your statement
"The local/total distinction doesn't map in a consistent way onto MPAM". To
address this I expect that an MPAM system will not support nor list
mbm_total_bytes and/or mbm_local_bytes in its mon_features file (*)? Instead,
it would list the events that are appropriate to the system? Trying to match
with what Peter said [1] in the message you refer to, this may be possible:
# cat /sys/fs/resctrl/info/L3_MON/mon_features
mbm_local_read_bytes
mbm_local_write_bytes
mbm_local_bytes
(*) I am including mbm_local_bytes since it could be an event that can be software
defined as a sum of mbm_local_read_bytes and mbm_local_write_bytes when they are both
counted.
I see the support for MPAM events distinct from the support of assignable counters.
Once the MPAM events are sorted, I think that they can be assigned with existing interface.
Please help me understand if you see it differently.
Doing so would need to come up with alphabetical letters for these events,
which seems to be needed for your proposal also? If we use possible flags of:
mbm_local_read_bytes a
mbm_local_write_bytes b
Then mbm_assign_control can be used as:
# echo '//0=ab;1=b' >/sys/fs/resctrl/info/L3_MON/mbm_assign_control
# cat /sys/fs/resctrl/mon_data/mon_L3_00/mbm_local_read_bytes
<value>
# cat /sys/fs/resctrl/mon_data/mon_L3_00/mbm_local_bytes
<sum of mbm_local_read_bytes and mbm_local_write_bytes>
One issue would be when resctrl needs to support more than 26 events (no more flags available),
assuming that upper case would be used for "shared" counters (unless this interface is defined
differently and only few uppercase letters used for it). Would this be too low of a limit?
Reinette
[1] https://lore.kernel.org/lkml/CALPaoCjY-3f2tWvBjuaQPfoPhxveWxxCxHqQMn4BEaeBXBa0bA@xxxxxxxxxxxxxx/
That approach would also work, where an MPAM system has events are not
a reasonable approximation of the generic "total" or "local".
For now we would probably stick with "total" and "local" anyway though,
because the MPAM architecture doesn't natively allow the mapping onto
the memory system topology to be discovered, and the information in
ACPI / device tree is insufficient to tell us everything we'd need to
know. But I guess what counts as "local" in particular will be quite
hardware and topology dependent even on x86, so perhaps we shouldn't
worry about having the behaviour match exactly (?)
Regarding the code letters, my idea was that the event type might be
configured by a separate file, instead of in mbm_assign_control
directly, in which case running out of letters wouldn't be a problem.
This work started with individual files for counters but the issue was
raised that this will require a large number of filesystem calls when, for
example, a user wants to move a group of counters associated with the events
of one set of monitoring groups to another set of monitoring groups. This
is for the use case where there are a significant number of monitor groups
for which there are not sufficient counters. With mbm_assign_control this
can be done in a single write and such a monitoring transition can thus
be accomplished more efficiently.
Alternatively, if we want to be able to expand beyond single letters,
could we reserve one or more characters for extension purposes?
If braces are forbidden by the syntax today, could we add support for
something like the following later on, without breaking anything?
# echo '//0={foo}{bar};1={bar}' >/sys/fs/resctrl/info/L3_MON/mbm_assign_control
Thank you for the suggestion. I think we may need something like this.
Babu, what do you think?
