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 E250A42490; Thu, 26 Jan 2023 12:22:25 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8548340A79; Thu, 26 Jan 2023 12:22:25 +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 0662E40697 for ; Thu, 26 Jan 2023 12:22:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674732143; 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=T4nvCCj/TD2UoHgP/QVM0fl6QpUcqNw1kZCD6bK9yks=; b=VmlHafxIomOFNf/b0MEPPAoE9IW1238y3u64BVG5ACzmmRmCPiWn6pha49NikjG32xg5Id Nje7pXrtIeqesfhVi5N2vFg6j6kIrtiqjHCpO1hb426BhtmZfSCvQmZZD0h/miexFEvj/Z Y5GHek+lL9JctWFRMj5tIM0OTkIrWWY= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-158-5kl1q8VPP0evmzF9hDGMOg-1; Thu, 26 Jan 2023 06:22:21 -0500 X-MC-Unique: 5kl1q8VPP0evmzF9hDGMOg-1 Received: by mail-ej1-f71.google.com with SMTP id ds1-20020a170907724100b008775bfcef62so1071801ejc.9 for ; Thu, 26 Jan 2023 03:22:21 -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=T4nvCCj/TD2UoHgP/QVM0fl6QpUcqNw1kZCD6bK9yks=; b=jhqIFSs182aeIYAhM9dG1C4VsYpcNI9dqg1kB5SRLRH+os6x+D2e6tA/PR+OeyaZqk su2Dnrpj2DnN8jUE/L44FjgmC9Hq/m85nGivwxEj4P1kc2VAHvhYZNIiKTGpGNDjg3qR /nogFi9ZnJG+BOplu8HWmbDpdhlykb1cK1hMQPOQfuW6l4k0BMIoZDKiDhfEv+1nVLsM OsH92h1XF7BCCx9Rr29SXbF30lJAhL0LfCBMVlks6POI0VVEzGmX5o3cgRmNmIHWq0/M AYPUvKXVNV3XaR1nj+9ejKnRVK72n6Wq/CUjViku2B4lGat2gbpNBoVICqIjz3zfF7Nl IXIg== X-Gm-Message-State: AFqh2kp4WQm2quMUdRD6/AIu7MEJcqB6AP7fKW/oQvQTqNxjNNeUU0sn XLxiowUF0dAFUevOv/ASJLQKxnTXgqTDW8ErF7iwtqrtI7mgiFZjcugbnZ1RbyTS/sABv0kozb6 zdc7Hof6iIUQSmHmjHZo= X-Received: by 2002:a17:906:c206:b0:78d:b819:e0e7 with SMTP id d6-20020a170906c20600b0078db819e0e7mr4166717ejz.83.1674732140775; Thu, 26 Jan 2023 03:22:20 -0800 (PST) X-Google-Smtp-Source: AMrXdXuLzCmE0IuXleTai1ptzWUynUk01w7u93/UNKGP4gKNPWMqljCjekvWbNMt5aGJs6kMJ9nDx8CSMdcvNwvYW+w= X-Received: by 2002:a17:906:c206:b0:78d:b819:e0e7 with SMTP id d6-20020a170906c20600b0078db819e0e7mr4166709ejz.83.1674732140528; Thu, 26 Jan 2023 03:22:20 -0800 (PST) MIME-Version: 1.0 References: <20221123102612.1688865-1-rjarry@redhat.com> <20230119150656.418404-1-rjarry@redhat.com> <20230119150656.418404-3-rjarry@redhat.com> In-Reply-To: <20230119150656.418404-3-rjarry@redhat.com> From: David Marchand Date: Thu, 26 Jan 2023 12:22:07 +0100 Message-ID: Subject: Re: [PATCH v6 2/5] eal: allow applications to report their cpu usage To: Robin Jarry Cc: dev@dpdk.org, Tyler Retzlaff , Kevin Laatz , =?UTF-8?Q?Morten_Br=C3=B8rup?= 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 On Thu, Jan 19, 2023 at 4:08 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 > --- > > Notes: > v5 -> v6: Added/rephrased some inline comments. > > lib/eal/common/eal_common_lcore.c | 45 ++++++++++++++++++++++++++++--- > lib/eal/include/rte_lcore.h | 35 ++++++++++++++++++++++++ > lib/eal/version.map | 1 + > 3 files changed, 78 insertions(+), 3 deletions(-) > > diff --git a/lib/eal/common/eal_common_lcore.c b/lib/eal/common/eal_commo= n_lcore.c > index 16548977dce8..80513cfe3725 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 > > @@ -422,11 +423,21 @@ rte_lcore_iterate(rte_lcore_iterate_cb cb, void *ar= g) > return ret; > } > > +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]; > + char cpuset[RTE_CPU_AFFINITY_STR_LEN], usage_str[256]; This is a debug/non performance sensitive helper. Please remove this "big enough for now" buffer and use a dynamic allocation= . > + struct rte_lcore_usage usage; > + rte_lcore_usage_cb usage_cb; > const char *role; > FILE *f =3D arg; > int ret; > @@ -446,11 +457,25 @@ lcore_dump_cb(unsigned int lcore_id, void *arg) > break; > } > > + /* The callback may not set all the fields in the structure, so c= lear it here. */ > + memset(&usage, 0, sizeof(usage)); > + usage_str[0] =3D '\0'; > + /* > + * 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) { > + snprintf(usage_str, sizeof(usage_str), ", busy cycles %"P= RIu64"/%"PRIu64, > + usage.busy_cycles, usage.total_cycles); > + } > 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), role, cpuset, > - ret =3D=3D 0 ? "" : "..."); > + ret =3D=3D 0 ? "" : "...", usage_str); > + > return 0; > } > > @@ -489,7 +514,9 @@ lcore_telemetry_info_cb(unsigned int lcore_id, void *= arg) > { > struct lcore_telemetry_info *info =3D arg; > struct rte_config *cfg =3D rte_eal_get_configuration(); > + struct rte_lcore_usage usage; > struct rte_tel_data *cpuset; > + rte_lcore_usage_cb usage_cb; > const char *role; > unsigned int cpu; Reverse xmas tree please. > > @@ -522,6 +549,18 @@ lcore_telemetry_info_cb(unsigned int lcore_id, void = *arg) > if (CPU_ISSET(cpu, &lcore_config[lcore_id].cpuset)) > 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 6938c3fd7b81..52468e7120dd 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); > > +/** > + * CPU usage statistics. Let's be consistent and use lcore. > + */ > +struct rte_lcore_usage { > + uint64_t total_cycles; > + /**< The total amount of time since application start, in TSC cyc= les. */ > + uint64_t busy_cycles; > + /**< The amount of busy time since application start, in TSC cycl= es. */ This is confusing to have the comments after. Please put those comments before the associated fields (using /** ). > +}; > + > +/** > + * Callback to allow applications to report CPU usage. lcore* > + * > + * @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); > + > +/** > + * 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 CPU usage statistics via this callback. lcore* > + * > + * @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 7ad12a7dc985..30fd216a12ea 100644 > --- a/lib/eal/version.map > +++ b/lib/eal/version.map > @@ -440,6 +440,7 @@ EXPERIMENTAL { > rte_thread_detach; > rte_thread_equal; > rte_thread_join; > + rte_lcore_register_usage_cb; Please start a new block for 23.03 symbols. --=20 David Marchand