[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v8 11/24] x86: refactor psr: set value: implement cos id picking flow.
On 17-03-09 07:10:33, Jan Beulich wrote: > >>> On 15.02.17 at 09:49, <yi.y.sun@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |