[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v9 07/25] x86: refactor psr: L3 CAT: implement get hw info flow.
>>> On 16.03.17 at 12:07, <yi.y.sun@xxxxxxxxxxxxxxx> wrote: > +static enum psr_feat_type psr_cbm_type_to_feat_type(enum cbm_type type) > +{ > + enum psr_feat_type feat_type; > + > + switch ( type ) > + { > + case PSR_CBM_TYPE_L3: > + feat_type = PSR_SOCKET_L3_CAT; > + break; > + default: > + feat_type = PSR_SOCKET_UNKNOWN; > + break; Is this actually reachable, if there are no bugs in the code? If not, you will want to add ASSERT_UNREACHABLE(). > +int psr_get_info(unsigned int socket, enum cbm_type type, > + uint32_t data[], unsigned int array_len) > +{ > + const struct psr_socket_info *info = get_socket_info(socket); > + const struct feat_node *feat; > + enum psr_feat_type feat_type; > + > + if ( IS_ERR(info) ) > + return PTR_ERR(info); > + > + if ( !data ) > + return -EINVAL; > + > + feat_type = psr_cbm_type_to_feat_type(type); > + feat = info->features[feat_type]; You can't blindly use the return value here as array index, as (at least in theory, see above) the function may return PSR_SOCKET_UNKNOWN. IOW you need to check against ARRAY_SIZE(info->features) first. > +/* Used by psr_get_info() */ > +#define PSR_INFO_IDX_CBM_LEN 0 > +#define PSR_INFO_IDX_COS_MAX 1 > +#define PSR_INFO_IDX_FLAG 2 > +#define PSR_INFO_CAT_SIZE 3 So I need some explanation on the naming here: Are the first three CAT-independent, but the last one is CAT-dependent? It doesn't look so (or else it would be odd coincidence for the last one to be one higher than the biggest of the _IDX ones). And if they're all in either of the two categories, their names should reflect that (i.e. either all have _CAT in their names, or none does). > + > + > struct psr_cmt_l3 { No double blank lines please (and just in case: comments of this kind apply to the entire series, i.e. you shouldn't expect them to be repeated in other patches). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |