Re: [Xen-devel] [PATCH v11 4/9] x86: detect and initialize Platform QoS Monitoring feature

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.06.14 at 08:38, <dongxiao.xu@xxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: Friday, June 20, 2014 11:04 PM
>> To: Xu, Dongxiao
>> Cc: andrew.cooper3@xxxxxxxxxx; Ian.Campbell@xxxxxxxxxx;
>> George.Dunlap@xxxxxxxxxxxxx; Ian.Jackson@xxxxxxxxxxxxx;
>> stefano.stabellini@xxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxx;
>> konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx; keir@xxxxxxx 
>> Subject: Re: [PATCH v11 4/9] x86: detect and initialize Platform QoS 
> Monitoring
>> feature
>> 
>> >>> On 20.06.14 at 16:31, <dongxiao.xu@xxxxxxxxx> wrote:
>> > Detect platform QoS feature status and enumerate the resource types,
>> > one of which is to monitor the L3 cache occupancy.
>> >
>> > Also introduce a Xen grub command line parameter to control the
>> > QoS feature status.
>> 
>> "grub"?
> 
> Sorry, what's your point here? Not quite understand it...

I'm asking what the mentioning of "grub" here means. Xen does have
command line parameters, but they can equally well be used when
booting without GrUB, e.g. directly from UEFI. IOW saying "grub" in
a statement like this is at best confusing, unless talk is of an option
that's only affecting booting via GrUB (e.g. something affecting the
handling of multiboot, albeit even in that case nothing dictates that
only GrUB can implement this protocol).

>> > +void __init init_platform_qos(void)
>> > +{
>> > +    if ( !opt_pqos )
>> > +        return;
>> > +
>> > +    if ( opt_pqos_monitor && opt_rmid_max )
>> 
>> Kind of pointless to split the two if()-s.
> 
> The split is reserved for the following QoS features, such as CQE, etc.

Since that isn't part of this series, I'd suggest avoiding such odd
looking constructs (unless you already have a follow-up series in the
works, and a reasonably certain it'll make 4.5). Future patches can
always split the one if() into multiple.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel