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 40F90439B5; Wed, 24 Jan 2024 12:51:08 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CD64640294; Wed, 24 Jan 2024 12:51:07 +0100 (CET) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 5541B4026F for ; Wed, 24 Jan 2024 12:51:06 +0100 (CET) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 008EB1E28B for ; Wed, 24 Jan 2024 12:51:06 +0100 (CET) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id E85DB1E300; Wed, 24 Jan 2024 12:51:05 +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.NA.cust.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 3F0F91E206; Wed, 24 Jan 2024 12:51:04 +0100 (CET) Message-ID: <26f7d4b7-d46f-461b-a891-7a32d14f4b21@lysator.liu.se> Date: Wed, 24 Jan 2024 12:51:03 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 04/11] eventdev: cleanup doxygen comments on info structure Content-Language: en-US To: Bruce Richardson Cc: dev@dpdk.org, jerinj@marvell.com, mattias.ronnblom@ericsson.com, abdullah.sevincer@intel.com, sachin.saxena@oss.nxp.com, hemant.agrawal@nxp.com, pbhagavatula@marvell.com, pravin.pathak@intel.com References: <20240118134557.73172-1-bruce.richardson@intel.com> <20240119174346.108905-1-bruce.richardson@intel.com> <20240119174346.108905-5-bruce.richardson@intel.com> <65d09512-f184-4ac4-a513-e5820754889e@lysator.liu.se> From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: 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-23 10:43, Bruce Richardson wrote: > On Tue, Jan 23, 2024 at 10:35:02AM +0100, Mattias Rönnblom wrote: >> On 2024-01-19 18:43, Bruce Richardson wrote: >>> Some small rewording changes to the doxygen comments on struct >>> rte_event_dev_info. >>> >>> Signed-off-by: Bruce Richardson >>> --- >>> lib/eventdev/rte_eventdev.h | 46 ++++++++++++++++++++----------------- >>> 1 file changed, 25 insertions(+), 21 deletions(-) >>> >>> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h >>> index 57a2791946..872f241df2 100644 >>> --- a/lib/eventdev/rte_eventdev.h >>> +++ b/lib/eventdev/rte_eventdev.h >>> @@ -482,54 +482,58 @@ struct rte_event_dev_info { >>> const char *driver_name; /**< Event driver name */ >>> struct rte_device *dev; /**< Device information */ >>> uint32_t min_dequeue_timeout_ns; >>> - /**< Minimum supported global dequeue timeout(ns) by this device */ >>> + /**< Minimum global dequeue timeout(ns) supported by this device */ >> >> Are we missing a bunch of "." here and in the other fields? >> >>> uint32_t max_dequeue_timeout_ns; >>> - /**< Maximum supported global dequeue timeout(ns) by this device */ >>> + /**< Maximum global dequeue timeout(ns) supported by this device */ >>> uint32_t dequeue_timeout_ns; >>> /**< Configured global dequeue timeout(ns) for this device */ >>> uint8_t max_event_queues; >>> - /**< Maximum event_queues supported by this device */ >>> + /**< Maximum event queues supported by this device */ >>> uint32_t max_event_queue_flows; >>> - /**< Maximum supported flows in an event queue by this device*/ >>> + /**< Maximum number of flows within an event queue supported by this device*/ >>> uint8_t max_event_queue_priority_levels; >>> /**< Maximum number of event queue priority levels by this device. >>> - * Valid when the device has RTE_EVENT_DEV_CAP_QUEUE_QOS capability >>> + * Valid when the device has RTE_EVENT_DEV_CAP_QUEUE_QOS capability. >>> + * The priority levels are evenly distributed between >>> + * @ref RTE_EVENT_DEV_PRIORITY_HIGHEST and @ref RTE_EVENT_DEV_PRIORITY_LOWEST. >> >> This is a change of the API, in the sense it's defining something previously >> left undefined? >> > > Well, undefined is pretty useless for app writers, no? > However, agreed that the range between HIGHEST and LOWEST is an assumption > on my part, chosen because it matches what happens to the event priorities > which are documented in event struct as "The implementation shall normalize > the requested priority to supported priority value" - which, while better > than nothing, does technically leave the details of how normalization > occurs up to the implementation. > >> If you need 6 different priority levels in an app, how do you go about >> making sure you find the correct (distinct) Eventdev levels on any event >> device supporting >= 6 levels? >> >> #define NUM_MY_LEVELS 6 >> >> #define MY_LEVEL_TO_EVENTDEV_LEVEL(my_level) (((my_level) * >> (RTE_EVENT_DEV_PRIORITY_HIGHEST-RTE_EVENT_DEV_PRIORTY_LOWEST) / >> NUM_MY_LEVELS) >> >> This way? One would worry a bit exactly what "evenly" means, in terms of >> rounding errors. If you have an event device with 255 priority levels of >> (say) 256 levels available in the API, which two levels are the same >> priority? > > Yes, round etc. will be an issue in cases of non-powers-of 2. > However, I think we do need to clarify this behaviour, so I'm open to > alternative suggestions as to how update this. > In retrospect, maybe it would have been better to just express the number of priority levels an event device supported, only allow [0, max_levels - 1] in the prio field, and leave it to the app to do the conversion/normalization. Maybe a new helper macro would at least suggest to the PMD driver implementer and the application designer how this normalization should work. Something like the above, but where NUM_MY_LEVELS is an input parameter. Would result in an integer division though, so shouldn't be used in the fast path.