Re: [PATCH v8 11/24] x86: refactor psr: set value: implement cos id picking flow.

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

On 17-03-09 07:10:33, Jan Beulich wrote:
> >>> On 15.02.17 at 09:49, <yi.y.sun@linux.intel.com> wrote:
> > @@ -397,6 +408,25 @@ static int l3_cat_compare_val(const uint64_t val[],
> >      return 0;
> >  }
> >  
> > +static bool l3_cat_fits_cos_max(const uint64_t val[],
> > +                                const struct feat_node *feat,
> > +                                unsigned int cos)
> > +{
> > +    uint64_t l3_def_cbm;
> > +
> > +    l3_def_cbm = (1ull << feat->info.l3_cat_info.cbm_len) - 1;
> 
> Seeing this pattern recur I wonder whether this also wouldn't be
> something that could be stored once in a generic manner, avoiding
> the need for this per-feature callback (cos_max should be common
> to all features too - not the values, but the abstract notion - so
> perhaps get_cos_max() isn't needed as a callback either).
> 
DYM to create a member in 'struct feat_node'? E.g:
uint64_t def_val;

> >  static int pick_avail_cos(const struct psr_socket_info *info,
> >                            const uint64_t *val, uint32_t array_len,
> >                            unsigned int old_cos,
> >                            enum psr_feat_type feat_type)
> >  {
> > +    unsigned int cos;
> > +    unsigned int cos_max = 0;
> > +    const struct feat_node *feat;
> > +    const unsigned int *ref = info->cos_ref;
> > +
> > +    /*
> > +     * cos_max is the one of the feature which is being set.
> > +     */
> 
> This is a single line comment.
> 
> > +    list_for_each_entry(feat, &info->feat_list, list)
> > +    {
> > +        if ( feat->feature != feat_type )
> > +            continue;
> > +
> > +        cos_max = feat->ops.get_cos_max(feat);
> > +        if ( cos_max > 0 )
> > +            break;
> > +    }
> 
> I think I've seen this a number of times by now - please make this a
> helper function, or store the result (which isn't going to change
> afaict).
> 
This behavior is changed in next version.

> Other than these comments given to earlier patches apply here too,
> as I think is obvious.
> 
> Jan

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