From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4D8EBA0583; Fri, 20 Mar 2020 13:21:09 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 727F0F94; Fri, 20 Mar 2020 13:21:08 +0100 (CET) Received: from mail-io1-f65.google.com (mail-io1-f65.google.com [209.85.166.65]) by dpdk.org (Postfix) with ESMTP id 78B503B5 for ; Fri, 20 Mar 2020 13:21:07 +0100 (CET) Received: by mail-io1-f65.google.com with SMTP id n21so5668123ioo.10 for ; Fri, 20 Mar 2020 05:21:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=f8q0P1Z/P5OIbzuxt4SHxvLgXeZtLQUmRDJ/skP4ooA=; b=loCrtq9EWP+rkAguNZX/w3ELZNjWHciqAA68vuWT3sOrfE1nirFVMa3bgz2km2Dt2P NNwuRjyaWyO7iedFiZUlITupYEwkkpH2fsTWI3ioEPKgnI92CdwElj/4V8UydX/dUYEt 7nqBQw+pQ5WAm9u8JyEcDGTAcvWUYAKxG1dTMvn2zKyv+R8OAu2MzWvbpkm7VBRIho6I XNEWFzYvd1mFYcHz1JrnxlqAGGMNYw2gf6fcV5Ysyj28NssBR4Bh7UVr74vi04+hNdtj kBfIT++hBwXIq4s8KwZjBQTSJaYdEg0YcDVscN4RwZKAARny0zn4ufIsK4lmYnj5LRLF XGBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=f8q0P1Z/P5OIbzuxt4SHxvLgXeZtLQUmRDJ/skP4ooA=; b=DHd9TecGC++2pjltNWO9gwTRxKcS5hrECy6KL3YbPxOvS8KRpYqnFA85Lux7XZ8D3u K4vY+hgYPiB+YXHMVOnegdfczxD1k8eepfdSFY3YxSPryR56q2jrVPukCMZ7V3RJVrJN kFt+4oaOuCSUbAZ4feljPDA0kS4huEjqS89puu87Ek0qh5qoIjNMhmEwE33Fia9e7CIE Qu/Swx86MBMd9Zjmxj3R7bItUJLkfoKfD3lgrsBXonejvk0f8vl8pMdUwwPAzyxKqHpq Yn9sG1sNJXvfpCv52k63mhsJc1oSU5EAcRCh/KxHAPO0BJP9ze+T04lwXnioO2N0vdmw zgwA== X-Gm-Message-State: ANhLgQ0MNe4K0YJ6dqss7BqkVLeHhJojqZGJrwg69QN3z/8m/INLSNam 342tMpIJL9oDoLAF3kq4uGtLnHETq4eFELv2IuY= X-Google-Smtp-Source: ADFU+vsF5iRW5InBnvfJfLazJft3h7WHLyAkBPq11Z3ut9KE9nUfQ1sc3CJzXXEu02J4x1monGVVfs4dBp5tmYz/YOo= X-Received: by 2002:a5d:9b19:: with SMTP id y25mr6877689ion.94.1584706866730; Fri, 20 Mar 2020 05:21:06 -0700 (PDT) MIME-Version: 1.0 References: <1583999071-22872-1-git-send-email-phil.yang@arm.com> <1584407863-774-1-git-send-email-phil.yang@arm.com> <2682224.FA0FI3ke8A@xps> In-Reply-To: From: Jerin Jacob Date: Fri, 20 Mar 2020 17:50:50 +0530 Message-ID: To: Honnappa Nagarahalli Cc: "thomas@monjalon.net" , Phil Yang , "Van Haaren, Harry" , "Ananyev, Konstantin" , "stephen@networkplumber.org" , "maxime.coquelin@redhat.com" , "dev@dpdk.org" , "david.marchand@redhat.com" , "jerinj@marvell.com" , "hemant.agrawal@nxp.com" , Gavin Hu , Ruifeng Wang , Joyce Kong , nd Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v3 00/12] generic rte atomic APIs deprecate proposal X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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 Fri, Mar 20, 2020 at 10:31 AM Honnappa Nagarahalli wrote: > > > > > Subject: Re: [PATCH v3 00/12] generic rte atomic APIs deprecate proposal > > > > 18/03/2020 15:01, Van Haaren, Harry: > > > Hi Phil & Honnappa, > > > > > > From: Phil Yang > > > > > > > > DPDK provides generic rte_atomic APIs to do several atomic operations. > > > > These APIs are using the deprecated __sync built-ins and enforce > > > > full memory barriers on aarch64. However, full barriers are not > > > > necessary in many use cases. In order to address such use cases, C > > > > language offers > > > > C11 atomic APIs. The C11 atomic APIs provide finer memory barrier > > > > control by making use of the memory ordering parameter provided by the > > user. > > > > Various patches submitted in the past [2] and the patches in this > > > > series indicate significant performance gains on multiple aarch64 > > > > CPUs and no performance loss on x86. > > > > > > > > But the existing rte_atomic API implementations cannot be changed as > > > > the APIs do not take the memory ordering parameter. The only choice > > > > available is replacing the usage of the rte_atomic APIs with C11 > > > > atomic APIs. In order to make this change, the following steps are > > proposed: > > > > > > > > [1] deprecate rte_atomic APIs so that future patches do not use > > > > rte_atomic APIs (a script is added to flag the usages). > > > > [2] refactor the code that uses rte_atomic APIs to use c11 atomic APIs. > > > > > > On [1] above, I feel deprecating DPDKs atomic functions and failing > > > checkpatch is a bit sudden. Perhaps noting that in a future release > > > (20.11?) DPDK will move to a > > > C11 based atomics model is a more gradual step to achieving the goal, > > > and at that point add a checkpatch warning for additions of rte_atomic*? > > > > > > More on [2] in context below. > > > > > > The above is my point-of-view, of course I'd like more people from the > > > DPDK community to provide their input too. > > > > > > > > > > This patchset contains: > > > > 1) the checkpatch script changes to flag rte_atomic API usage in patches. > > > > 2) changes to programmer guide describing writing efficient code for > > aarch64. > > > > 3) changes to various libraries to make use of c11 atomic APIs. > > > > > > > > We are planning to replicate this idea across all the other > > > > libraries, drivers, examples, test applications. In the next phase, > > > > we will add changes to the mbuf, the EAL interrupts and the event timer > > adapter libraries. > > > > > > About ~6/12 patches of this C11 set are targeting the Service Cores > > > area of DPDK. I have some concerns over increased complexity of C11 > > implementation vs the (already complex) rte_atomic implementation today. > > > I see other patchsets enabling C11 across other DPDK components, so > > > maybe we should also discuss C11 enabling in a wider context that just > > service cores? > > > > > > I don't think it fair to expect all developers to be well versed in > > > C11 atomic semantics like understanding the complex interactions between > > the various C11 RELEASE, AQUIRE barriers requires. > > > > > > As maintainer of Service Cores I'm hesitant to accept the large-scale > > > refactor of atomic-implementation, as it could lead to racey bugs that > > > are likely extremely difficult to track down. (The recent race-on-exit has > > proven the difficulty in reproducing, and that's with an atomics model I'm > > quite familiar with). > > > > > > Let me be very clear: I don't wish to block a C11 atomic > > > implementation, but I'd like to discuss how we (DPDK community) can > > > best port-to and maintain a complex multi-threaded service library with > > best-in-class performance for the workload. > > > > > > To put some discussions/solutions on the table: > > > - Shared Maintainership of a component? > > > Split in functionality and C11 atomics implementation > > > Obviously there would be collaboration required in such a case. > > > - Maybe shared maintainership is too much? > > > A gentlemans/womans agreement of "helping out" with C11 atomics > > debug is enough? > > > > > > > > > Hope my concerns are understandable, and of course input/feedback > > > welcomed! -Harry > > > > Thanks for raising the issue Harry. > > > > I think we should have at least two official maintainers for C11 atomics in > > general. > Sure, I can volunteer. > > > C11 conversion is a progressive effort being done, and should be merged step > > by step. > Agree, the changes need to be understood, it is not a search and replace effort. The changes will come-in in stages unless others join the effort. > The concern I have is about the new patches that get added. I think we need to stop the new patches from using rte_atomic APIs, otherwise we might be making these changes forever. +1. We must define a time frame otherwise we might be making these changes forever. > > > If C11 maintainers fail to fix some issues on time, then we can hold the effort. > > Does it make sense? > I am fine with this approach. But, I think we need to have a deadline in mind to complete the work. > > > >