Received-SPF: pass (google.com: domain of linux-kernel+bounces-143028-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 12 Apr 2024 17:13:23 +0100
From: Dave Martin <Dave.Martin@arm.com>
To: David Hildenbrand <david@redhat.com>
Cc: James Morse <james.morse@arm.com>, x86@kernel.org,
	linux-kernel@vger.kernel.org, Fenghua Yu <fenghua.yu@intel.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	H Peter Anvin <hpa@zytor.com>, Babu Moger <Babu.Moger@amd.com>,
	shameerali.kolothum.thodi@huawei.com,
	D Scott Phillips OS <scott@os.amperecomputing.com>,
	carl@os.amperecomputing.com, lcherian@marvell.com,
	bobo.shaobowang@huawei.com, tan.shaopeng@fujitsu.com,
	baolin.wang@linux.alibaba.com, Jamie Iles <quic_jiles@quicinc.com>,
	Xin Hao <xhao@linux.alibaba.com>, peternewman@google.com,
	dfustini@baylibre.com, amitsinght@marvell.com,
	Rex Nie <rex.nie@jaguarmicro.com>
Subject: Re: [PATCH v1 03/31] x86/resctrl: Move ctrlval string parsing policy
 away from the arch code
Message-ID: <Zhldo59cSmSLwFGZ@e133380.arm.com>
References: <20240321165106.31602-1-james.morse@arm.com>
 <20240321165106.31602-4-james.morse@arm.com>
 <1776b49b-3c0e-41a7-bbc5-19310c428429@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1776b49b-3c0e-41a7-bbc5-19310c428429@redhat.com>

Hi,

On Tue, Apr 09, 2024 at 05:13:01PM +0200, David Hildenbrand wrote:

[...]

> > @@ -195,6 +204,14 @@ int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
> >   	return 0;
> >   }
> > +static ctrlval_parser_t *get_parser(struct rdt_resource *res)
> > +{
> > +	if (res->fflags & RFTYPE_RES_CACHE)
> > +		return &parse_cbm;
> > +	else
> > +		return &parse_bw;
> > +}
> 
> Besides what Reinette said, I'd have added here something that would fire in
> case someone adds something unexpected in the future, like
> 
> WARN_ON_ONCE(!(res->fflags & (RFTYPE_RES_CACHE|RFTYPE_RES_MB));
> 
> At the beginning of the function.
> 
> 
> Apart from that, nothing jumped at me.
> 
> -- 
> Cheers,
> 
> David / dhildenb
> 

Thanks for that -- I guess that would benefit from discussion; please
see my reply to Reinette on this patch.

Cheers
---Dave