Messages in this thread | | | Date | Thu, 28 Sep 2023 13:47:49 -0700 | Subject | Re: [PATCH v2 4/4] x86/resctrl: Rename arch_has_sparse_bitmaps | From | Reinette Chatre <> |
| |
Hi Maciej
Could you please move this patch to the beginning of this series? Having it in the end results in a lot of churn with significant changes to new code introduced in this series. All this can be avoided by providing a smaller patch at the beginning of the series.
On 9/22/2023 1:48 AM, Maciej Wieczor-Retman wrote: > Both AMD and Intel documentations use capacity bitmasks terminology > rather than capacity bitmaps. Also bitmask term is much more widely > used inside x86 resctrl code.
Since resctrl is intended to support many architectures and Arm coming soon (and they use bitmap) I do not think we should use the vendor's language as a motivation. For me there are three reasons supporting the rename: * arch_has_sparse_bitmaps is the *only* instance in resctrl that uses the bitmap term for capacity bitmasks, all other parts of resctrl refers to it as a bitmask * bitmask is the established term used in resctrl documentation (Documentation/arch/x86/resctrl.rst) * bitmask is the term already exposed to user space via resctrl ("cbm_mask")
Finally, why do the rename as part of this work? This can be motivated with something like: A later patch exposes the value of arch_has_sparse_bitmaps to user space via the existing term of a bitmask. Rename arch_has_sparse_bitmaps to arch_has_sparse_bitmasks to ensure consistent terminology throughout resctrl.
> > Unify the naming convention by renaming arch_has_sparse_bitmaps struct > member to arch_has_sparse_bitmasks. > > Suggested-by: Reinette Chatre <reinette.chatre@intel.com> > Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com> > ---
...
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h > index 8334eeacfec5..83c2cbf7136d 100644 > --- a/include/linux/resctrl.h > +++ b/include/linux/resctrl.h > @@ -57,7 +57,7 @@ struct resctrl_staged_config { > * @list: all instances of this resource > * @id: unique id for this instance > * @cpu_mask: which CPUs share this resource > - * @rmid_busy_llc: bitmap of which limbo RMIDs are above threshold > + * @rmid_busy_llc: bitmask of which limbo RMIDs are above threshold > * @mbm_total: saved state for MBM total bandwidth > * @mbm_local: saved state for MBM local bandwidth > * @mbm_over: worker to periodically read MBM h/w counters
Please drop this hunk. rmid_busy_llc is indeed a bitmap.
Reinette
| |