|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 14/15] tools: implement new generic set value interface and MBA set value command
On Tue, Sep 05, 2017 at 05:32:36PM +0800, Yi Sun wrote:
> This patch implements new generic set value interfaces in libxc and libxl.
> These interfaces are suitable for all allocation features. It also adds a
> new MBA set value command in xl.
>
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> ---
> v3:
> - add 'const' for 'opts[]' in 'main_psr_mba_set'.
> (suggested by Roger Pau Monné)
> - replace 'libxl_psr_cbm_type' to 'libxl_psr_type' for newly defined
> interfaces.
> (suggested by Roger Pau Monné)
> ---
> tools/libxc/include/xenctrl.h | 6 ++---
> tools/libxc/xc_psr.c | 9 ++++---
> tools/libxl/libxl_psr.c | 56
> +++++++++++++++++++++----------------------
> tools/xl/xl.h | 1 +
> tools/xl/xl_cmdtable.c | 6 +++++
> tools/xl/xl_psr.c | 55 ++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 99 insertions(+), 34 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index eef06be..21dac2f 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2499,9 +2499,9 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t
> rmid, uint32_t cpu,
> uint64_t *tsc);
> int xc_psr_cmt_enabled(xc_interface *xch);
>
> -int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
> - xc_psr_type type, uint32_t target,
> - uint64_t data);
> +int xc_psr_set_domain_data(xc_interface *xch, uint32_t domid,
> + xc_psr_type type, uint32_t target,
> + uint64_t data);
> int xc_psr_get_domain_data(xc_interface *xch, uint32_t domid,
> xc_psr_type type, uint32_t target,
> uint64_t *data);
> diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
> index 2f0eed9..e53b5f5 100644
> --- a/tools/libxc/xc_psr.c
> +++ b/tools/libxc/xc_psr.c
> @@ -248,9 +248,9 @@ int xc_psr_cmt_enabled(xc_interface *xch)
>
> return 0;
> }
> -int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
> - xc_psr_type type, uint32_t target,
> - uint64_t data)
> +int xc_psr_set_domain_data(xc_interface *xch, uint32_t domid,
> + xc_psr_type type, uint32_t target,
> + uint64_t data)
> {
> DECLARE_DOMCTL;
> uint32_t cmd;
> @@ -269,6 +269,9 @@ int xc_psr_cat_set_domain_data(xc_interface *xch,
> uint32_t domid,
> case XC_PSR_CAT_L2_CBM:
> cmd = XEN_DOMCTL_PSR_ALLOC_SET_L2_CBM;
> break;
> + case XC_PSR_MBA_THRTL:
> + cmd = XEN_DOMCTL_PSR_ALLOC_SET_MBA_THRTL;
> + break;
> default:
> errno = EINVAL;
> return -1;
> diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> index 78d5bc5..d3c3d42 100644
> --- a/tools/libxl/libxl_psr.c
> +++ b/tools/libxl/libxl_psr.c
> @@ -328,33 +328,7 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> libxl_psr_cbm_type type, libxl_bitmap *target_map,
> uint64_t cbm)
> {
> - GC_INIT(ctx);
> - int rc;
> - int socketid, nr_sockets;
> -
> - rc = libxl__count_physical_sockets(gc, &nr_sockets);
> - if (rc) {
> - LOGED(ERROR, domid, "failed to get system socket count");
> - goto out;
> - }
> -
> - libxl_for_each_set_bit(socketid, *target_map) {
> - xc_psr_type xc_type;
> -
> - if (socketid >= nr_sockets)
> - break;
> -
> - xc_type = libxl__psr_type_to_libxc_psr_type(type);
> - if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type,
> - socketid, cbm)) {
> - libxl__psr_alloc_log_err_msg(gc, errno, type);
> - rc = ERROR_FAIL;
> - }
> - }
> -
> -out:
> - GC_FREE;
> - return rc;
> + return libxl_psr_set_val(ctx, domid, type, target_map, cbm);
> }
>
> int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
> @@ -458,7 +432,33 @@ int libxl_psr_set_val(libxl_ctx *ctx, uint32_t domid,
> libxl_psr_type type, libxl_bitmap *target_map,
> uint64_t val)
> {
> - return ERROR_FAIL;
> + GC_INIT(ctx);
> + int rc;
> + int socketid, nr_sockets;
You could fit them all in a single line.
> + rc = libxl__count_physical_sockets(gc, &nr_sockets);
> + if (rc) {
> + LOG(ERROR, "failed to get system socket count");
> + goto out;
> + }
> +
> + libxl_for_each_set_bit(socketid, *target_map) {
> + xc_psr_type xc_type;
> +
> + if (socketid >= nr_sockets)
> + break;
> +
> + xc_type = libxl__psr_type_to_libxc_psr_type(type);
> + if (xc_psr_set_domain_data(ctx->xch, domid, xc_type,
> + socketid, val)) {
> + libxl__psr_alloc_log_err_msg(gc, errno, type);
> + rc = ERROR_FAIL;
> + }
> + }
> +
> +out:
> + GC_FREE;
> + return rc;
> }
>
> int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid,
> diff --git a/tools/xl/xl.h b/tools/xl/xl.h
> index 3389df9..3f99b6b 100644
> --- a/tools/xl/xl.h
> +++ b/tools/xl/xl.h
> @@ -204,6 +204,7 @@ int main_psr_cmt_detach(int argc, char **argv);
> int main_psr_cmt_show(int argc, char **argv);
> int main_psr_cat_cbm_set(int argc, char **argv);
> int main_psr_cat_show(int argc, char **argv);
> +int main_psr_mba_set(int argc, char **argv);
> int main_psr_mba_show(int argc, char **argv);
> #endif
> int main_qemu_monitor_command(int argc, char **argv);
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index cdc2349..9d45d3b 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -560,6 +560,12 @@ struct cmd_spec cmd_table[] = {
> "[options] <Domain>",
> "-l <level> Specify the cache level to process, otherwise L3
> cache is processed\n"
> },
> + { "psr-mba-set",
> + &main_psr_mba_set, 0, 1,
> + "Set throttling value (THRTL) for a domain",
> + "[options] <Domain> <THRTL>",
> + "-s <socket> Specify the socket to process, otherwise all
> sockets are processed\n"
> + },
> { "psr-mba-show",
> &main_psr_mba_show, 0, 1,
> "Show Memory Bandwidth Allocation information",
> diff --git a/tools/xl/xl_psr.c b/tools/xl/xl_psr.c
> index 46b7788..a648b1a 100644
> --- a/tools/xl/xl_psr.c
> +++ b/tools/xl/xl_psr.c
> @@ -552,6 +552,61 @@ int main_psr_mba_show(int argc, char **argv)
> return psr_val_show(domid, LIBXL_PSR_FEAT_TYPE_MBA, 0);
> }
>
> +int main_psr_mba_set(int argc, char **argv)
> +{
> + uint32_t domid;
> + libxl_psr_type type;
> + uint64_t thrtl;
> + int ret, opt = 0;
> + libxl_bitmap target_map;
> + char *value;
> + libxl_string_list socket_list;
> + unsigned long start, end;
> + unsigned int i, j, len;
> +
> + static const struct option opts[] = {
> + {"socket", 1, 0, 's'},
> + COMMON_LONG_OPTS
> + };
> +
> + libxl_socket_bitmap_alloc(ctx, &target_map, 0);
> + libxl_bitmap_set_none(&target_map);
> +
> + SWITCH_FOREACH_OPT(opt, "s:", opts, "psr-mba-set", 0) {
> + case 's':
> + trim(isspace, optarg, &value);
> + split_string_into_string_list(value, ",", &socket_list);
> + len = libxl_string_list_length(&socket_list);
> + for (i = 0; i < len; i++) {
> + parse_range(socket_list[i], &start, &end);
Indentation.
> + for (j = start; j <= end; j++)
> + libxl_bitmap_set(&target_map, j);
> + }
> +
> + libxl_string_list_dispose(&socket_list);
> + free(value);
> + break;
> + }
> +
> + type = LIBXL_PSR_CBM_TYPE_MBA_THRTL;
> +
> + if (libxl_bitmap_is_empty(&target_map))
> + libxl_bitmap_set_any(&target_map);
> +
> + if (argc != optind + 2) {
> + help("psr-mba-set");
> + return 2;
> + }
Can you do this check at the beginning of the function? Also why
return 2 instead of EXIT_FAILURE?
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |