|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 10/15] tools: implement the new libxl get hw info interface
On Tue, Sep 05, 2017 at 05:32:32PM +0800, Yi Sun wrote:
> This patch implements the new libxl get hw info interface,
> 'libxl_psr_get_hw_info', which is suitable to all psr allocation
> features. It also implements corresponding list free function,
> 'libxl_psr_hw_info_list_free' and make 'libxl_psr_cat_get_info' to call
^makes ^call
> 'libxl_psr_get_hw_info' to avoid redundant codes in libxl_psr.c.
^code
>
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> ---
> v3:
> - remove casting.
> (suggested by Roger Pau Monné)
> - remove inline.
> (suggested by Roger Pau Monné)
> - change 'libxc__psr_hw_info_to_libxl_psr_hw_info' to
> 'libxl__xc_hw_info_to_libxl_hw_info'.
> (suggested by Roger Pau Monné)
> - remove '_hw' from parameter names.
> (suggested by Roger Pau Monné)
> - change some 'LOGE' to 'LOG'.
> (suggested by Roger Pau Monné)
> - check returned 'xc_type' and remove redundant 'lvl' check.
> (suggested by Roger Pau Monné)
> v2:
> - split this patch out from a big patch in v1.
> (suggested by Wei Liu)
> - change 'CAT_INFO'/'MBA_INFO' to 'CAT' and 'MBA. Also the libxl structure
> name 'cat_info'/'mba_info' is changed to 'cat'/'mba'.
> (suggested by Chao Peng)
> - call 'libxl_psr_hw_info_list_free' in 'libxl_psr_cat_get_info' to free
> allocated resources.
> (suggested by Chao Peng)
> ---
> tools/libxl/libxl_psr.c | 145
> ++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 108 insertions(+), 37 deletions(-)
>
> diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> index dd412cc..d534ec2 100644
> --- a/tools/libxl/libxl_psr.c
> +++ b/tools/libxl/libxl_psr.c
> @@ -382,60 +382,49 @@ static xc_psr_feat_type
> libxl__feat_type_to_libxc_feat_type(
> return xc_type;
> }
>
> +static int libxl__hw_info_to_libxl_cat_info(
> + libxl_psr_feat_type type, libxl_psr_hw_info *hw_info,
> + libxl_psr_cat_info *cat_info)
> +{
> + if (type != LIBXL_PSR_FEAT_TYPE_CAT)
> + return ERROR_INVAL;
Since this is an internal libxl function, is there any possible valid
scenario where this function is called with type !=
LIBXL_PSR_FEAT_TYPE_CAT?
If not this should be an assert instead, and the function could return
void.
> +
> + cat_info->id = hw_info->id;
> + cat_info->cos_max = hw_info->u.cat.cos_max;
> + cat_info->cbm_len = hw_info->u.cat.cbm_len;
> + cat_info->cdp_enabled = hw_info->u.cat.cdp_enabled;
> +
> + return 0;
> +}
> +
> int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
> unsigned int *nr, unsigned int lvl)
> {
> GC_INIT(ctx);
> int rc;
> - int i = 0, socketid, nr_sockets;
> - libxl_bitmap socketmap;
> + unsigned int i;
> + libxl_psr_hw_info *hw_info;
> libxl_psr_cat_info *ptr;
> - xc_psr_hw_info hw_info;
> - xc_psr_feat_type xc_type;
> -
> - libxl_bitmap_init(&socketmap);
>
> - rc = libxl__count_physical_sockets(gc, &nr_sockets);
> - if (rc) {
> - LOGE(ERROR, "failed to get system socket count");
> + rc = libxl_psr_get_hw_info(ctx, &hw_info, nr, LIBXL_PSR_FEAT_TYPE_CAT,
> lvl);
> + if (rc)
> goto out;
> - }
>
> - libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets);
> - rc = libxl_get_online_socketmap(ctx, &socketmap);
> - if (rc < 0) {
> - LOGE(ERROR, "failed to get available sockets");
> - goto out;
> - }
> -
> - xc_type = libxl__feat_type_to_libxc_feat_type(LIBXL_PSR_FEAT_TYPE_CAT,
> lvl);
> - if (xc_type == XC_PSR_FEAT_UNKNOWN) {
> - LOG(ERROR, "feature type or lvl is wrong");
> - rc = ERROR_FAIL;
> - goto out;
> - }
> + ptr = libxl__malloc(NOGC, *nr * sizeof(libxl_psr_cat_info));
>
> - ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_cat_info));
> -
> - libxl_for_each_set_bit(socketid, socketmap) {
> - ptr[i].id = socketid;
> - if (xc_psr_get_hw_info(ctx->xch, socketid, xc_type, &hw_info)) {
> + for (i = 0; i < *nr; i++) {
> + if (libxl__hw_info_to_libxl_cat_info(LIBXL_PSR_FEAT_TYPE_CAT,
> + &hw_info[i], &ptr[i])) {
Please use rc here:
rc = libxl__hw_info_to_libxl_cat_info(...);
if (rc) {
...
This has the bonus of not losing the error code returned by
libxl__hw_info_to_libxl_cat_info.
> + libxl_psr_hw_info_list_free(hw_info, *nr);
> rc = ERROR_FAIL;
> free(ptr);
> goto out;
> }
> -
> - ptr[i].cos_max = hw_info.u.xc_cat.cos_max;
> - ptr[i].cbm_len = hw_info.u.xc_cat.cbm_len;
> - ptr[i].cdp_enabled = hw_info.u.xc_cat.cdp_enabled;
> -
> - i++;
> }
>
> *info = ptr;
> - *nr = i;
> + libxl_psr_hw_info_list_free(hw_info, *nr);
> out:
> - libxl_bitmap_dispose(&socketmap);
> GC_FREE;
> return rc;
> }
> @@ -476,15 +465,97 @@ int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid,
> return ERROR_FAIL;
> }
>
> +static int libxl__xc_hw_info_to_libxl_hw_info(
> + libxl_psr_feat_type type, xc_psr_hw_info *xc_info,
> + libxl_psr_hw_info *xl_info)
> +{
> + switch (type) {
> + case LIBXL_PSR_FEAT_TYPE_CAT:
> + xl_info->u.cat.cos_max = xc_info->u.xc_cat.cos_max;
> + xl_info->u.cat.cbm_len = xc_info->u.xc_cat.cbm_len;
> + xl_info->u.cat.cdp_enabled = xc_info->u.xc_cat.cdp_enabled;
> + break;
> + case LIBXL_PSR_FEAT_TYPE_MBA:
> + xl_info->u.mba.cos_max = xc_info->u.xc_mba.cos_max;
> + xl_info->u.mba.thrtl_max = xc_info->u.xc_mba.thrtl_max;
> + xl_info->u.mba.linear = xc_info->u.xc_mba.linear;
> + break;
> + default:
> + return ERROR_INVAL;
> + }
> +
> + return 0;
> +}
> +
> int libxl_psr_get_hw_info(libxl_ctx *ctx, libxl_psr_hw_info **info,
> unsigned int *nr, libxl_psr_feat_type type,
> unsigned int lvl)
> {
> - return ERROR_FAIL;
> + GC_INIT(ctx);
> + int rc, nr_sockets;
> + unsigned int i = 0, socketid;
> + libxl_bitmap socketmap;
> + libxl_psr_hw_info *ptr;
> + xc_psr_feat_type xc_type;
> + xc_psr_hw_info hw_info;
> +
> + libxl_bitmap_init(&socketmap);
> +
> + xc_type = libxl__feat_type_to_libxc_feat_type(type, lvl);
> + if (xc_type == XC_PSR_FEAT_UNKNOWN) {
> + LOG(ERROR, "feature type or lvl is wrong");
> + rc = ERROR_FAIL;
> + goto out;
> + }
> +
> + rc = libxl__count_physical_sockets(gc, &nr_sockets);
> + if (rc) {
> + LOG(ERROR, "failed to get system socket count");
> + goto out;
> + }
> +
> + libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets);
> + rc = libxl_get_online_socketmap(ctx, &socketmap);
> + if (rc < 0) {
> + LOGE(ERROR, "failed to get available sockets");
> + goto out;
> + }
> +
> + ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_hw_info));
> +
> + libxl_for_each_set_bit(socketid, socketmap) {
> + ptr[i].id = socketid;
> + if (xc_psr_get_hw_info(ctx->xch, socketid, xc_type, &hw_info)) {
> + rc = ERROR_FAIL;
> + free(ptr);
> + goto out;
> + }
> +
> + if (libxl__xc_hw_info_to_libxl_hw_info(type, &hw_info, &ptr[i])) {
> + LOGE(ERROR, "Input type %d is wrong!\n", type);
> + rc = ERROR_FAIL;
> + free(ptr);
> + goto out;
> + }
> + i++;
> + }
> +
> + *info = ptr;
> + *nr = i;
> +out:
> + libxl_bitmap_dispose(&socketmap);
> + GC_FREE;
> + return rc;
> }
>
> void libxl_psr_hw_info_list_free(libxl_psr_hw_info *list, unsigned int nr)
> {
> + unsigned int i;
> +
> + for (i = 0; i < nr; i++)
> + libxl_psr_hw_info_dispose(&list[i]);
> + free(list);
Don't you also need a libxl_psr_cat_info_list_free? Or am I missing
something?
> }
>
> /*
> --
> 1.9.1
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |