RE: [PATCH v20 09/18] x86/resctrl: Add a new field to struct rmid_read for summation of domains

Luck, Tony
2024-06-21T16:07:03Z

> I do not think there is a need to introduce new syntax. It will be easiest
> to just have all sentences end with a period. The benefit of this is that it
> encourages useful full sentence descriptions. For example, below is a _draft_ of
> such a description. Please note that I wrote it quickly and hope it will be improved
> (and corrected!). The goal of it being here is to give ideas on how this kerneldoc
> can be written to be useful and consistent.
>
> /**
> * struct rmid_read - Data passed across smp_call*() to read event count

Should this end with a period too? In the resctrl code a few cases use ".",
most don't. So no period matches resctrl style. But the example in
Documentation/doc-guide/kernel-doc.rst does end with a period.

> * @rgrp: Resource group for which the counter is being read. If it is a parent
> * resource group then its event count is summed with the count from all
> * its child resource groups.
> * @r: Resource describing the properties of the event being read.
> * @d: Domain that the counter should be read from. If NULL then sum all
> * domains in @r sharing L3 @ci.id
> * @evtid: Which monitor event to read.
> * @first: Initialize MBM counter when true.
> * @ci: Cacheinfo for L3. Only set when @d is NULL. Used when summing domains.
> * @err: Error encountered when reading counter.
> * @val: Returned value of event counter. If @rgrp is a parent resource group,
> * @val contains the sum of event counts from its child resource groups.
> * If @d is NULL, @val contains the sum of all domains in @r sharing @ci.id,
> * (summed across child resource groups if @rgrp is a parent resource group).
> * @arch_mon_ctx: Hardware monitor allocated for this read request (MPAM only).
> */

This all looks good to me. Since you have supplied 99% of the content for this
patch in the series I should assign authorship to you (which requires your
Signed-off-by tag). Is that OK? Should I split into two parts? First to add the
kerneldoc (by you). Second to add the new field (by me).

-Tony