From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5F05E41BE7; Mon, 6 Feb 2023 09:48:45 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 09E8240FAE; Mon, 6 Feb 2023 09:48:45 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id A103640A7A for ; Mon, 6 Feb 2023 09:48:43 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675673323; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0AFYVC3RQv5pffSEl2gTF7CQe6xv+eAZrimlC/HF30U=; b=f66WmjPGGPSiTi6DRn28zEV33HvRYep+AH+I4dBD1yfCA/sReURkPcV3ROAKvygSfveKrJ xIFOtqW5Z6HOIGVPtHujhgR2mcXGiWFZWQ4hQv2Jnv9npmQ6T+Yuq1RRwFtiLE0QczJTFP Ov4FVAtjB5W4RmPteQZsOXMBf+dNFYE= Received: from mail-pl1-f198.google.com (mail-pl1-f198.google.com [209.85.214.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-56-HQjoeCZoOMKUkZ8dsVZFqQ-1; Mon, 06 Feb 2023 03:48:34 -0500 X-MC-Unique: HQjoeCZoOMKUkZ8dsVZFqQ-1 Received: by mail-pl1-f198.google.com with SMTP id z8-20020a170902834800b001990ad8de5bso1859681pln.10 for ; Mon, 06 Feb 2023 00:48:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=0AFYVC3RQv5pffSEl2gTF7CQe6xv+eAZrimlC/HF30U=; b=fl0G0wpABcpMHUXT9mystaoQPpEGK/Qe3u0meKCI+aqCkgnF/b29T7p42d+MWlKC10 NOtR/+cFpVTrlE194bfO8Hwv5MVMC33e3D+YBTvYNOyUBeO/pQ0y1Q6URu/SpaAEklIo Bkz0k9hMYWc2k/DA6oN+EYeOHkUM5WsYWDBDCHVmRhAE8W07f2iou0RRv66XLBfqDXkx IRSYpbUbu9CJy20zz2BjwClGQCzykiXfmeHs5QfScO6kepJG5zUxomX6fbw4aKuWCXvp RmBLP8q14prVXhYp2WDnB13DIf4+Ox87LlyuRNkCuUWPkgOlB7csxRGiWPHSFXhhfss7 g8zw== X-Gm-Message-State: AO0yUKXuk5rbAGeXG9lW4o5xEfwn8BcW5ar8D2eSZKCcU0v2V5QUoHuM a1UfE9RXWHYb97CjDN6snrWwG51V146U+ypA3BpmlXF2Vkf+hIIQLdEZyvK4PEcBRzpFZfX/qC9 m1lOrloA7YvZL6oEWvJE= X-Received: by 2002:a17:90a:9f91:b0:230:b57f:5229 with SMTP id o17-20020a17090a9f9100b00230b57f5229mr625665pjp.68.1675673313137; Mon, 06 Feb 2023 00:48:33 -0800 (PST) X-Google-Smtp-Source: AK7set8vuDDa0sVgatiserhLtBdGghyF34u7tdtffS1TQfoG6OWnZGhfwmA7jXNn5fzIroeoXpuYnxh/zeS2cFTUA5w= X-Received: by 2002:a17:90a:9f91:b0:230:b57f:5229 with SMTP id o17-20020a17090a9f9100b00230b57f5229mr625659pjp.68.1675673312750; Mon, 06 Feb 2023 00:48:32 -0800 (PST) MIME-Version: 1.0 References: <20221123102612.1688865-1-rjarry@redhat.com> <20230202134329.539625-1-rjarry@redhat.com> <20230202134329.539625-3-rjarry@redhat.com> In-Reply-To: <20230202134329.539625-3-rjarry@redhat.com> From: David Marchand Date: Mon, 6 Feb 2023 09:48:21 +0100 Message-ID: Subject: Re: [PATCH v8 2/5] eal: report applications lcore usage To: Robin Jarry , =?UTF-8?Q?Morten_Br=C3=B8rup?= , Kevin Laatz Cc: dev@dpdk.org, Chengwen Feng X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hello Robin, On Thu, Feb 2, 2023 at 2:44 PM Robin Jarry wrote: > > Allow applications to register a callback that will be invoked in > rte_lcore_dump() and when requesting lcore info in the telemetry API. > > The callback is expected to return the number of TSC cycles that have > passed since application start and the number of these cycles that were > spent doing busy work. > > Signed-off-by: Robin Jarry > Acked-by: Morten Br=C3=B8rup > Reviewed-by: Kevin Laatz > --- > > Notes: > v7 -> v8: no change > > doc/guides/rel_notes/release_23_03.rst | 7 ++++ > lib/eal/common/eal_common_lcore.c | 48 ++++++++++++++++++++++++-- > lib/eal/include/rte_lcore.h | 35 +++++++++++++++++++ > lib/eal/version.map | 1 + > 4 files changed, 89 insertions(+), 2 deletions(-) > > diff --git a/doc/guides/rel_notes/release_23_03.rst b/doc/guides/rel_note= s/release_23_03.rst > index 73f5d94e143d..f407dc3df7a8 100644 > --- a/doc/guides/rel_notes/release_23_03.rst > +++ b/doc/guides/rel_notes/release_23_03.rst > @@ -78,6 +78,13 @@ New Features > ``rte_event_dev_config::nb_single_link_event_port_queues`` parameter > required for eth_rx, eth_tx, crypto and timer eventdev adapters. > > +* **Added support for reporting lcore usage in applications.** > + > + * The ``/eal/lcore/list`` and ``/eal/lcore/info`` telemetry endpoints = have > + been added to provide information similar to ``rte_lcore_dump()``. > + * Applications can register a callback at startup via > + ``rte_lcore_register_usage_cb()`` to provide lcore usage information= . > + EAL updates come first in RN sections. > > Removed Items > ------------- > diff --git a/lib/eal/common/eal_common_lcore.c b/lib/eal/common/eal_commo= n_lcore.c > index f53fc17b4d04..bbb734098b42 100644 > --- a/lib/eal/common/eal_common_lcore.c > +++ b/lib/eal/common/eal_common_lcore.c > @@ -2,6 +2,7 @@ > * Copyright(c) 2010-2014 Intel Corporation > */ > > +#include > #include > #include > > @@ -437,20 +438,49 @@ lcore_role_str(enum rte_lcore_role_t role) > } > } > > +static rte_lcore_usage_cb lcore_usage_cb; > + > +void > +rte_lcore_register_usage_cb(rte_lcore_usage_cb cb) > +{ > + lcore_usage_cb =3D cb; > +} > + > static int > lcore_dump_cb(unsigned int lcore_id, void *arg) > { > struct rte_config *cfg =3D rte_eal_get_configuration(); > char cpuset[RTE_CPU_AFFINITY_STR_LEN]; > + struct rte_lcore_usage usage; > + rte_lcore_usage_cb usage_cb; > + char *usage_str =3D NULL; > FILE *f =3D arg; > int ret; > > + /* The callback may not set all the fields in the structure, so c= lear it here. */ > + memset(&usage, 0, sizeof(usage)); > + /* > + * Guard against concurrent modification of lcore_usage_cb. > + * rte_lcore_register_usage_cb() should only be called once at ap= plication init > + * but nothing prevents and application to reset the callback to = NULL. This is copy/paste a few times, only commenting here: "prevents an* application from* resetting* the callback to NULL" > + */ > + usage_cb =3D lcore_usage_cb; > + if (usage_cb !=3D NULL && usage_cb(lcore_id, &usage) =3D=3D 0) { > + if (asprintf(&usage_str, ", busy cycles %"PRIu64"/%"PRIu6= 4, > + usage.busy_cycles, usage.total_cycles) < = 0) { > + return -ENOMEM; > + } > + } > ret =3D eal_thread_dump_affinity(&lcore_config[lcore_id].cpuset, = cpuset, > sizeof(cpuset)); > - fprintf(f, "lcore %u, socket %u, role %s, cpuset %s%s\n", lcore_i= d, > + fprintf(f, "lcore %u, socket %u, role %s, cpuset %s%s%s\n", lcore= _id, > rte_lcore_to_socket_id(lcore_id), > lcore_role_str(cfg->lcore_role[lcore_id]), > - cpuset, ret =3D=3D 0 ? "" : "..."); > + cpuset, ret =3D=3D 0 ? "" : "...", > + usage_str ? usage_str : ""); usage_str !=3D NULL > + > + free(usage_str); > + > return 0; > } > > @@ -489,7 +519,9 @@ lcore_telemetry_info_cb(unsigned int lcore_id, void *= arg) > { > struct rte_config *cfg =3D rte_eal_get_configuration(); > struct lcore_telemetry_info *info =3D arg; > + struct rte_lcore_usage usage; > struct rte_tel_data *cpuset; > + rte_lcore_usage_cb usage_cb; > unsigned int cpu; > > if (info->lcore_id !=3D lcore_id) > @@ -508,6 +540,18 @@ lcore_telemetry_info_cb(unsigned int lcore_id, void = *arg) > rte_tel_data_add_array_int(cpuset, cpu); > } > rte_tel_data_add_dict_container(info->d, "cpuset", cpuset, 0); > + /* The callback may not set all the fields in the structure, so c= lear it here. */ > + memset(&usage, 0, sizeof(usage)); > + /* > + * Guard against concurrent modification of lcore_usage_cb. > + * rte_lcore_register_usage_cb() should only be called once at ap= plication init > + * but nothing prevents and application to reset the callback to = NULL. > + */ > + usage_cb =3D lcore_usage_cb; > + if (usage_cb !=3D NULL && usage_cb(lcore_id, &usage) =3D=3D 0) { > + rte_tel_data_add_dict_u64(info->d, "total_cycles", usage.= total_cycles); > + rte_tel_data_add_dict_u64(info->d, "busy_cycles", usage.b= usy_cycles); > + } > > return 0; > } > diff --git a/lib/eal/include/rte_lcore.h b/lib/eal/include/rte_lcore.h > index 9c7865052100..b1c8afb05d28 100644 > --- a/lib/eal/include/rte_lcore.h > +++ b/lib/eal/include/rte_lcore.h > @@ -328,6 +328,41 @@ typedef int (*rte_lcore_iterate_cb)(unsigned int lco= re_id, void *arg); > int > rte_lcore_iterate(rte_lcore_iterate_cb cb, void *arg); > > +/** > + * lcore usage statistics. > + */ > +struct rte_lcore_usage { > + /** The total amount of time since application start, in TSC cycl= es. */ > + uint64_t total_cycles; This code comment needs some clarification. What is this "total amount of time"? "application start" is ambiguous. EAL lcore threads are not created/started by the application itself, so the application has no idea of the time the lcore/threads were created. I would describe as: /** The total amount of time that the application has been running on this lcore, in TSC cycles. */ Is it acceptable to you? > + /** The amount of busy time since application start, in TSC cycle= s. */ > + uint64_t busy_cycles; And here: /** The amount of time the application was busy, handling some workload on this lcore, in TSC cycles. */ > +}; > + > +/** > + * Callback to allow applications to report lcore usage. > + * > + * @param [in] lcore_id > + * The lcore to consider. > + * @param [out] usage > + * Counters representing this lcore usage. This can never be NULL. > + * @return > + * - 0 if fields in usage were updated successfully. The fields that t= he > + * application does not support must not be modified. > + * - a negative value if the information is not available or if any er= ror occurred. > + */ > +typedef int (*rte_lcore_usage_cb)(unsigned int lcore_id, struct rte_lcor= e_usage *usage); > + > +/** Missing a experimental banner: * @warning * @b EXPERIMENTAL: this API may change without prior notice. > + * Register a callback from an application to be called in rte_lcore_dum= p() and > + * the /eal/lcore/info telemetry endpoint handler. Applications are expe= cted to > + * report lcore usage statistics via this callback. > + * > + * @param cb > + * The callback function. > + */ > +__rte_experimental > +void rte_lcore_register_usage_cb(rte_lcore_usage_cb cb); > + > /** > * List all lcores. > * > diff --git a/lib/eal/version.map b/lib/eal/version.map > index 6523102157e2..1f70caac7b9c 100644 > --- a/lib/eal/version.map > +++ b/lib/eal/version.map > @@ -442,6 +442,7 @@ EXPERIMENTAL { > > # added in 23.03 > rte_thread_set_name; > + rte_lcore_register_usage_cb; Alphabetical order. > }; > > INTERNAL { > -- > 2.39.1 > --=20 David Marchand