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 4CAD0439CE; Fri, 26 Jan 2024 09:28:08 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C86A40DF5; Fri, 26 Jan 2024 09:28:08 +0100 (CET) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 6B7C840289 for ; Fri, 26 Jan 2024 09:28:06 +0100 (CET) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 238FE26B4C for ; Fri, 26 Jan 2024 09:28:06 +0100 (CET) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 1795426B4B; Fri, 26 Jan 2024 09:28:06 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.4 required=5.0 tests=ALL_TRUSTED,AWL, T_SCC_BODY_TEXT_LINE autolearn=disabled version=4.0.0 X-Spam-Score: -1.4 Received: from [192.168.1.59] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 7696A26AE5; Fri, 26 Jan 2024 09:28:04 +0100 (CET) Message-ID: Date: Fri, 26 Jan 2024 09:28:04 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC] service: extend service function call statistics Content-Language: en-US To: =?UTF-8?Q?Morten_Br=C3=B8rup?= , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , dev@dpdk.org Cc: harry.van.haaren@intel.com, Stefan Sundkvist References: <20240125191529.388570-1-mattias.ronnblom@ericsson.com> <98CBD80474FA8B44BF855DF32C47DC35E9F1A4@smartserver.smartshare.dk> From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F1A4@smartserver.smartshare.dk> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP 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 2024-01-26 00:19, Morten Brørup wrote: >> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com] >> Sent: Thursday, 25 January 2024 20.15 >> >> Add two new per-service counters. >> >> RTE_SERVICE_ATTR_IDLE_CALL_COUNT tracks the number of service function >> invocations where no work was performed. >> >> RTE_SERVICE_ATTR_ERROR_CALL_COUNT tracks the number invocations >> resulting in an error. >> >> The semantics of RTE_SERVICE_ATTR_CALL_COUNT remains the same (i.e., >> counting all invocations, regardless of return value). >> >> The new statistics may be useful for both debugging and profiling >> (e.g., calculate the average per-call processing latency for non-idle >> service calls). >> >> Service core tests are extended to cover the new counters, and >> coverage for RTE_SERVICE_ATTR_CALL_COUNT is improved. > > OK to all of the above. Good stuff. > >> >> The documentation for the CYCLES attributes are updated to reflect >> their actual semantics. > > If this is intended behavior, then updating the documentation seems appropriate - I would even go so far as considering it a bug fix. > > However, quite a few cycles may be consumed by a service before it can conclude that it had no work to do. Shouldn't that be considered time spent by the service? I.e. should the code be fixed instead of the documentation? > Generally, polling for new work in the service is cheap, in my experience. But there's nothing in principle that prevents the situation your describe from occurring. You could add an "IDLE_CYCLES" counter in case that would ever be a real-world problem. That wouldn't be a fix, but rather just returning to the old, subtly broken, (pre-22.11?) semantics. Have a look at 809bd24 to see the rationale for the change. There's an example in 4689c57. The cause of this ambiguity is due to the fact that the platform/OS (i.e., DPDK) doesn't know which service to "wake up" (which in turn is the result of the platform not being able to tracking input sources, like NIC RX queues or timer wheels) and thus must ask every service to check if it has something to do. > Alternatively, keep the behavior (for backwards compatibility) and fix the documentation, as this patch does, and add an IDLE_CYCLES counter for time spent in idle calls. > > PS: We're not using DPDK service cores in our applications, so I'm not familiar with the details. We are using something somewhat similar (but homegrown), also for profiling and power management purposes, and my feedback is based on my experience with our own variant of service cores. > When are you making the switch to service cores? :) > Either way: > > Acked-by: Morten Brørup > Thanks for the review.