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 72D59A0C47; Sun, 31 Oct 2021 14:17:46 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3DFEC4068E; Sun, 31 Oct 2021 14:17:46 +0100 (CET) Received: from mail-io1-f50.google.com (mail-io1-f50.google.com [209.85.166.50]) by mails.dpdk.org (Postfix) with ESMTP id D7FB440689 for ; Sun, 31 Oct 2021 14:17:44 +0100 (CET) Received: by mail-io1-f50.google.com with SMTP id 62so10995087iou.2 for ; Sun, 31 Oct 2021 06:17:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=8lowH8hM9YTXV2+weyyq/N/BHYurTUDE9AeMGMIOIfw=; b=NfbooYFKwubzP6Xa5um26Afp/GZkKNTsDueoy9jqLqkhZgijmMMDZv7TtUhITsl4NU PqGscGCQCsSjM6T+CuQcvlFlEWrfdHBAe+ebHkefQ6u9YcAwIx2iWTZzDwl7oZpxRufr AI0xAxqAvk0rCECqaYljtbtAWQqEY/lgvppucTSQI/bx/tC7xPz6JCpdfUDPBN8mlOXE R9Bwrk5PUYKy1o/kSGJN9eg1M6e07SWctmwPoBsVEZwrWW9rnSqAb9UMwzWqagSJjnjl ptgoX9uq+GBnscroFe1Ew2p04gQ3KlZmlGaJIIyf5th1E8oCcFsmUp9g17IZu0Fp8AGu NkdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=8lowH8hM9YTXV2+weyyq/N/BHYurTUDE9AeMGMIOIfw=; b=VQMM2YkMzKgi0qyUAXqjOwgCFfKPbcVZ1pLo36/+bB++EXZT4KtxfkCbffhKefPuIL J8/2fPxxwxRE2CnpBPMWrkL/8BY25PzxL+U3R+jWVH8srpvlia5NCtfQniBPM7FNXAE5 RD+UJOFSGxB29EDgS2DqZ8dCK6B9MpJieBoBLyEP1qo0tJ5nnIBy27DcR8pudB/kp7hV gjq+DKabKIfefy+wb51jOu1OZqMPRLdcZ5HFpcRjpsKWxlUl36+5tevedbzwco6BvkIq GChaXgiMCk75qCmuHlJJWIDo4FK44ZnAKH7w600PbkcZ/qnJ5YOG9T7dBZXfPWYN+RkS s6Tg== X-Gm-Message-State: AOAM530nNLosPNKyHLcrTewxPjhZLPl9ebGvanqp3Tl2wqUV+7DaMKuc 9GdbQte6mdJ8AoGWdUlZB9BbKAjyveAXAG9Dyp4= X-Google-Smtp-Source: ABdhPJxa5JgUWwaRaTtbh71+W5m7IRS4dhl6rmMssw1zjTnCsf30NMS3FKtl9EK0GyuEYw0Bel36s1v9aiMDtvF+j3o= X-Received: by 2002:a6b:14cb:: with SMTP id 194mr16242241iou.185.1635686264151; Sun, 31 Oct 2021 06:17:44 -0700 (PDT) MIME-Version: 1.0 References: <3e8c8bab-783d-d132-a836-51bd4d5533bb@ericsson.com> <20211026173148.19399-1-mattias.ronnblom@ericsson.com> <7494151c-26b2-4ccc-f011-73ffceb92a85@ericsson.com> In-Reply-To: From: Jerin Jacob Date: Sun, 31 Oct 2021 18:47:18 +0530 Message-ID: To: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= Cc: "Van Haaren, Harry" , "McDaniel, Timothy" , Pavan Nikhilesh , Hemant Agrawal , Liang Ma , "Richardson, Bruce" , Jerin Jacob , "Gujjar, Abhinandan S" , Erik Gabriel Carrillo , "Jayatheerthan, Jay" , dpdk-dev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance 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 Sender: "dev" On Sat, Oct 30, 2021 at 10:49 PM Mattias R=C3=B6nnblom wrote: > > On 2021-10-29 17:17, Jerin Jacob wrote: > > On Fri, Oct 29, 2021 at 8:33 PM Mattias R=C3=B6nnblom > > wrote: > >> On 2021-10-29 16:38, Jerin Jacob wrote: > >>> On Tue, Oct 26, 2021 at 11:02 PM Mattias R=C3=B6nnblom > >>> wrote: > >>>> Extend Eventdev API to allow for event devices which require various > >>>> forms of internal processing to happen, even when events are not > >>>> enqueued to or dequeued from a port. > >>>> > >>>> PATCH v1: > >>>> - Adapt to the move of fastpath function pointers out of > >>>> rte_eventdev struct > >>>> - Attempt to clarify how often the application is expected to > >>>> call rte_event_maintain() > >>>> - Add trace point > >>>> RFC v2: > >>>> - Change rte_event_maintain() return type to be consistent > >>>> with the documentation. > >>>> - Remove unused typedef from eventdev_pmd.h. > >>>> > >>>> Signed-off-by: Mattias R=C3=B6nnblom > >>>> Tested-by: Richard Eklycke > >>>> Tested-by: Liron Himi > >>>> --- > >>>> > >>>> +/** > >>>> + * Maintain an event device. > >>>> + * > >>>> + * This function is only relevant for event devices which has the > >>>> + * RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag set. Such devices require = the > >>>> + * application to call rte_event_maintain() on a port during period= s > >>>> + * which it is neither enqueuing nor dequeuing events from that > >>>> + * port. > >>> # We need to add "by the same core". Right? As other core such as > >>> service core can not call rte_event_maintain() > >> > >> Do you mean by the same lcore thread that "owns" (dequeues and enqueue= s > >> to) the port? Yes. I thought that was implicit, since eventdev port ar= e > >> not MT safe. I'll try to figure out some wording that makes that more = clear. > > OK. > > > >> > >>> # Also, Incase of Adapters enqueue() happens, right? If so, either > >>> above text is not correct. > >>> # @Erik Gabriel Carrillo @Jayatheerthan, Jay @Gujjar, Abhinandan S > >>> Please review 3/3 patch on adapter change. > >>> Let me know you folks are OK with change or not or need more time to = analyze. > >>> > >>> If it need only for the adapter subsystem then can we make it an > >>> internal API between DSW and adapters? > >> > >> No, it's needed for any producer-only eventdev ports, including any su= ch > >> ports used by the application. > > > > In that case, the code path in testeventdev, eventdev_pipeline, etc nee= ds > > to be updated. I am worried about the performance impact for the driver= s they > > don't have such limitations. > > > Applications that are using some other event device today, and don't > care about DSW or potential future event devices > requiringRTE_EVENT_DEV_CAP_REQUIRES_MAINT, won't be affected at all, > except the ops struct will be 8 bytes larger. Ack. That's not an issue. > > > A rte_event_maintain() call on a device which doesn't need maintenance > is just an inlined NULL compare on the ops struct field, which is > frequently used and should be in a cache close to the core. In my > benchmarks, I've been unable to measure any additional cost at all. > > > I reviewed the test and example applications last time I attempted to > upstream this patch set, and from what I remember there was nothing to > update. Things might have changed and I might misremember, so I'll have > a look again. OK. > > > What's important to keep in mind is that applications (DPDK tests, > examples, user applications etc.) that have producer-only ports or > otherwise potentially leave eventdev ports "unattended" don't work with > DSW today, unless the take the measures described in the DSW > documentation (which for example the eventdev adapters do not). So > rte_event_maintain() will not break anything that's not already broken. OK. > > > > Why not have an additional config option in port_config which says > > it is a producer-only port by an application and takes care of the driv= er. > > > > In the current adapters code, you are calling maintain() when enqueue > > returns zero. > > > rte_event_maintain() is called when no interaction with the event device > has been done, during that service function call. That's the overall > intention. > > In the RX adapter, zero flushed events can also mean that the RX adapter > had buffered events it wanted to flush, but the event device didn't > accept new events (i.e, back pressure). In that case, the > rte_event_maintain() call is redundant, but harmless (both because it's > very low overhead on DSW, and near-zero overhead on any other current > event device). Plus, if you are back-pressured by the pipeline, RX is > not the bottleneck so a tiny bit of extra overhead is not an issue. OK. Looks good to me. Since DSW has better performance than other SW drivers, I think, it is OK to take some limitations to the application. Please send the next version with the suggested documentation change. If there is no objection, I will merge it on Wednesday. (3rd Nov)