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 B16A7A0548; Thu, 4 Nov 2021 13:34:03 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 789C3411A4; Thu, 4 Nov 2021 13:34:03 +0100 (CET) Received: from mail-il1-f180.google.com (mail-il1-f180.google.com [209.85.166.180]) by mails.dpdk.org (Postfix) with ESMTP id 4126B41144 for ; Thu, 4 Nov 2021 13:34:02 +0100 (CET) Received: by mail-il1-f180.google.com with SMTP id s14so5966649ilv.10 for ; Thu, 04 Nov 2021 05:34:02 -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=ARmfyN30seOBWY15Qwd57JEOZ8yEmbG5T/uJGdWIjB0=; b=bHVSosrsWvHpJfJ6DSlGfh8stDt1w3ZiwPrXSG+UelsZlgyg+81eqMXXMf++PGc/0c Ta0lZfdV6KNEFlYaGwYt7Z5JL7zpgdiIB7fTOP7XXj3jMSGdsP4h15By73xL9VqO48Hb P+VSFeYUZ9aqDZcdkEiTpmC/nEmuOtFwP1pOQj953eUllknv8Qm/F05KL7090vwdL+VU VMPzWI9XO1EM6BIc/TZ8hjlBKzfqa/S2unEXi0PowEzNlxCamXxP7zwZ1ryckS6DYBgi iujnrzlFSMCdjvEHCQ0Q3P4PoyykAJUwq6zRDH3t2y6L1lTvh6Y7D5zic0S1rtmPRfei SmxA== 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=ARmfyN30seOBWY15Qwd57JEOZ8yEmbG5T/uJGdWIjB0=; b=3Nn7oEIPCVBj+Gu7es99HKmavHP44F2gL1Da+JHbF30TYHs2xnItIL6R95pmPJ1Bvp 500zVl+KlmRrEDLEOLDuLoMCL+jOK7sd0H28oLJzrbnuufExLRvnHv/6wNokvwxWYGKp 7tEEwBU64pMSBjY4XuWYoAOEekP/jooZozl4L3IPOt3kBN3JdcIeDdsxAAu79FAfbEqG WmyEo1NoxUESWkzdQ3VpVQDiSm5DzwNQRSl2rSSur11ucHXEDUEAGnTd3WdRHbF4+y+7 Kvma4PRlp/AAxVLCI6/7WpzvRODNfXUy7PAPQ600shQ5JBovmkps0v/dTn8psSsiU2Di YGyg== X-Gm-Message-State: AOAM530uIrj6HBbWJCTss0l/VlccrKsrwn9iBefGF0kbtv/1y7qAwTOy DwfAeu8a1ISNrPS4Ax2Z7g+I5mQD7oaqNPwV7w4= X-Google-Smtp-Source: ABdhPJwyCVFexlQ0d26onbUSk1DKvetDpck2wuRWglc1tsK2kDaajeveAt/pwUt7ZLgf88uA1NSC2imCcNPJc+xrNHI= X-Received: by 2002:a05:6e02:1aac:: with SMTP id l12mr7234045ilv.295.1636029241557; Thu, 04 Nov 2021 05:34:01 -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: Thu, 4 Nov 2021 18:03:35 +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 Sun, Oct 31, 2021 at 6:47 PM Jerin Jacob wrote: > > 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 vario= us > > >>>> 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 requir= e the > > >>>> + * application to call rte_event_maintain() on a port during peri= ods > > >>>> + * 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 enque= ues > > >> to) the port? Yes. I thought that was implicit, since eventdev port = are > > >> not MT safe. I'll try to figure out some wording that makes that mor= e 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 t= o 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 = such > > >> ports used by the application. > > > > > > In that case, the code path in testeventdev, eventdev_pipeline, etc n= eeds > > > to be updated. I am worried about the performance impact for the driv= ers 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 dr= iver. > > > > > > In the current adapters code, you are calling maintain() when enqueue > > > returns zero. > > > > > > rte_event_maintain() is called when no interaction with the event devic= e > > 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 adapte= r > > 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) Applied v3 version to dpdk-next-net-eventdev/for-main. Thanks