[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <49e78ef5-b500-4135-a32d-003d3d62f239@intel.com>
Date: Wed, 23 Oct 2024 15:00:51 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: James Morse <james.morse@....com>, <x86@...nel.org>,
<linux-kernel@...r.kernel.org>
CC: Fenghua Yu <fenghua.yu@...el.com>, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, H Peter Anvin
<hpa@...or.com>, Babu Moger <Babu.Moger@....com>,
<shameerali.kolothum.thodi@...wei.com>, D Scott Phillips OS
<scott@...amperecomputing.com>, <carl@...amperecomputing.com>,
<lcherian@...vell.com>, <bobo.shaobowang@...wei.com>,
<tan.shaopeng@...itsu.com>, <baolin.wang@...ux.alibaba.com>, Jamie Iles
<quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
<peternewman@...gle.com>, <dfustini@...libre.com>, <amitsinght@...vell.com>,
David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
"Dave Martin" <dave.martin@....com>, Shaopeng Tan
<tan.shaopeng@...fujitsu.com>
Subject: Re: [PATCH v5 18/40] x86/resctrl: Export the is_mbm_*_enabled()
helpers to asm/resctrl.h
Hi James,
On 10/4/24 11:03 AM, James Morse wrote:
> The architecture specific parts of resctrl have helpers to hide accesses
> to the rdt_mon_features bitmap.
hmmmm ... no ... this patch creates those helpers.
>
> Once the filesystem parts of resctrl are moved, these can no longer live
> in internal.h. Once these are exposed to the wider kernel, they should
> have a 'resctrl_arch_' prefix, to fit the rest of the arch<->fs interface.
>
> Move and rename the helpers that touch rdt_mon_features directly.
> is_mbm_event() and is_mbm_enabled() are only called from rdtgroup.c,
> so can be moved into that file.
There seems to be a contradiction here ... earlier patch moved the
event IDs to common header so this makes these events shared between
resctrl and all archs. rdt_mon_features bitmap positions are
the common event IDs. Why should rdt_mon_features thus be considered arch
specific if bits that can be set are not?
The patch may be ok if MPAM wants to do something different here but
motivating it as "this is arch specific and needs to be hidden by helpers"
is a stretch since there is nothing arch specific about it.
Reinette
Powered by blists - more mailing lists