Re: [PATCH v8 08/16] x86: implement set value flow for MBA

[Yi Sun <yi.y.sun@linux.intel.com>]

On 17-10-16 06:49:49, Jan Beulich wrote:
> >>> On 16.10.17 at 05:04, <yi.y.sun@linux.intel.com> wrote:
> > This patch implements set value flow for MBA including its callback
> > function and domctl interface.
> > 
> > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> > v8:
> >     - restore some unnecessary changes in 'cat_check_cbm'.
> >       (suggested by Jan Beulich)
> 
> This reads as if you did exactly the opposite thing of what you
> actually did.
> 
> > +static bool mba_sanitize_thrtl(const struct feat_node *feat, uint32_t *thrtl)
> > +{
> > +    if ( *thrtl > feat->mba.thrtl_max )
> > +        return false;
> 
> Wouldn't it be better to do this check after ...
> 
> > +    /*
> > +     * Per SDM (chapter "Memory Bandwidth Allocation Configuration"):
> > +     * 1. Linear mode: In the linear mode the input precision is defined
> > +     *    as 100-(MBA_MAX). For instance, if the MBA_MAX value is 90, the
> > +     *    input precision is 10%. Values not an even multiple of the
> > +     *    precision (e.g., 12%) will be rounded down (e.g., to 10% delay
> > +     *    applied).
> > +     * 2. Non-linear mode: Input delay values are powers-of-two from zero
> > +     *    to the MBA_MAX value from CPUID. In this case any values not a
> > +     *    power of two will be rounded down the next nearest power of two.
> > +     */
> > +    if ( feat->mba.linear )
> > +        *thrtl -= *thrtl % (100 - feat->mba.thrtl_max);
> > +    else
> > +    {
> > +        /* Not power of 2. */
> > +        if ( *thrtl & (*thrtl - 1) )
> > +            *thrtl = 1 << (fls(*thrtl) - 1);
> > +    }
> 
> ... these adjustments? That way someone specifying e.g. a linear
> value of 95% would get 90% just like for 85% (s)he would get
> 80%.
> 
> > +    return true;
> 
> If so, the return statement could simply become
> 
>     return *thrtl <= feat->mba.thrtl_max;
> 
> My R-b also applies if you decide to make this change.
> 
Thank you for the suggestion! It is not worthy to send whole patch set out.
I will just update this patch.

> Jan

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