[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 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). > 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). 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 |