From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 00F4FA00E6 for ; Tue, 16 Apr 2019 23:22:11 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D5B6A1B561; Tue, 16 Apr 2019 23:22:10 +0200 (CEST) Received: from mail-pf1-f196.google.com (mail-pf1-f196.google.com [209.85.210.196]) by dpdk.org (Postfix) with ESMTP id 5440C1B55F for ; Tue, 16 Apr 2019 23:22:09 +0200 (CEST) Received: by mail-pf1-f196.google.com with SMTP id c207so11022946pfc.7 for ; Tue, 16 Apr 2019 14:22:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=J+/UWthaG/DUOgnr1Qx4WyKEebNwzYLtO6scoDZG4V0=; b=srqZGIa6rJffEW+PcVJLG2vvQIwe83+Y2geoLePu+b1N5mtriUAXlRbn+pZXXxvKap UbyamGs9ScXfmB07IcBe7vhVCCZqlGqJQ6o1LtghmizN+xEFrwpHBPVUporikPGipB/6 rYCHRWF85qIsL9jdqgxG8sKrSYP0i2firoyeu5TC+EifYL+GJlVestni1pTJGdOWxmFq M/RS/XhT8IsvBoRbLkTxk36L1mr9s5u4LNpMREeM6QvRBugCZ32etV4dYqDRYmcxZau9 W9Jw4WCdu56ruamlTK4lHLxk7QxQL1uNX39kOZfHGirjdn1/zd0TzzpfOcZ0vdo0rRZF Bwww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=J+/UWthaG/DUOgnr1Qx4WyKEebNwzYLtO6scoDZG4V0=; b=e303xgj7pxRB9sUYJ2hKnCcPamZrq50k1IzTVWnz3XTLqFiqJOcZ9GNOv4dIXUJAJO gju0Sh/A4ipNVliazHS8YNHTVd7F7aB9CDFz1d5cvSM0LdOwxjejt1EE0Wgz/kHyBPhq 0829xEb1ktjDdAh2kMlxLeJ6CsvwXEmb8T3b6o/ypMnAyatwb5XK6hAODKWINfNiU93+ wv3CD30Vr13eEOnQnoEs6haoWYIoDZnqxH46bnNgKmwgIHIz+7uAdpS41G41cKNAxS7w G5bVwIQ3SdUsvFXi6cIfqlpXxkDLoJ4lBNyD5LEUB4xYy7OBHJRh5I7gjC98Q2ePlPke Y5tA== X-Gm-Message-State: APjAAAWPDx1JmhJX14tTorSmjNMHkmVf/ioECqGqP26KZPwkEvF/wa4R ka24ENiYqi/k5MBw8HPVlzl4VQpLJFM= X-Google-Smtp-Source: APXvYqxVvW2l4Rbr2tDy82kPl11mB9Viq4xSRmYc2RGPDkhefAyK2ayYHV5dQtR6xgkyh8KIAw6UWA== X-Received: by 2002:a65:44c6:: with SMTP id g6mr27780485pgs.157.1555449728426; Tue, 16 Apr 2019 14:22:08 -0700 (PDT) Received: from xps13.lan (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id p128sm122774479pfp.30.2019.04.16.14.22.07 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 16 Apr 2019 14:22:08 -0700 (PDT) Date: Tue, 16 Apr 2019 14:22:05 -0700 From: Stephen Hemminger To: Honnappa Nagarahalli Cc: "Ananyev, Konstantin" , "paulmck@linux.ibm.com" , "Kovacevic, Marko" , "dev@dpdk.org" , "Gavin Hu (Arm Technology China)" , Dharmik Thakkar , Malvika Gupta , nd Message-ID: <20190416142205.5d683e8e@xps13.lan> In-Reply-To: References: <20181122033055.3431-1-honnappa.nagarahalli@arm.com> <20190412202039.46902-1-honnappa.nagarahalli@arm.com> <20190412202039.46902-2-honnappa.nagarahalli@arm.com> <20190412150650.3709358e@shemminger-XPS-13-9360> <20190412160629.670eacd1@shemminger-XPS-13-9360> <2601191342CEEE43887BDE71AB9772580148A97E53@irsmsx105.ger.corp.intel.com> <20190415083834.31b38ed3@shemminger-XPS-13-9360> <2601191342CEEE43887BDE71AB9772580148A98064@irsmsx105.ger.corp.intel.com> <20190415142631.4c250248@shemminger-XPS-13-9360> <20190416075415.76c9d64a@xps13.lan> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v5 1/3] rcu: add RCU library supporting QSBR mechanism 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" Message-ID: <20190416212205._c7opVi-DCvlbdCa3Uxd7oVY_EfYhbBOcgfiBlOu6_0@z> On Tue, 16 Apr 2019 16:56:32 +0000 Honnappa Nagarahalli wrote: > > > > > > > > > > > > On Fri, 12 Apr 2019 15:20:37 -0500 Honnappa Nagarahalli > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Add RCU library supporting quiescent state based > > > > > > > > > > > memory reclamation > > > > > > > > > > method. > > > > > > > > > > > This library helps identify the quiescent state of the > > > > > > > > > > > reader threads so that the writers can free the memory > > > > > > > > > > > associated with the lock less data structures. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Honnappa Nagarahalli > > > > > > > > > > > > > > > > > > > > > > Reviewed-by: Steve Capper > > > > > > > > > > > Reviewed-by: Gavin Hu > > > > > > > > > > > Reviewed-by: Ola Liljedahl > > > > > > > > > > > Acked-by: Konstantin Ananyev > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > After evaluating long term API/ABI issues, I think you > > > > > > > > > > need to get rid of almost all use of inline and visible > > > > > > > > > > structures. Yes it might be marginally slower, but you > > > > > > > > > > thank me > > > > the first time you have to fix something. > > > > > > > > > > > > > > > > > > > Agree, I was planning on another version to address this > > > > > > > > > (I am yet > > > > to take a look at your patch addressing the ABI). > > > > > > > > > The structure visibility definitely needs to be addressed. > > > > > > > > > For the inline functions, is the plan to convert all the > > > > > > > > > inline functions in DPDK? If yes, I think we need to > > > > > > > > > consider the performance > > > > > > > > difference. May be consider L3-fwd application, change all > > > > > > > > the > > > > inline functions in its path and run a test? > > > > > > > > > > > > > > > > Every function that is not in the direct datapath should not > > > > > > > > be > > > > inline. > > > > > > > > Exceptions or things like rx/tx burst, ring enqueue/dequeue, > > > > > > > > and packet alloc/free > > > I do not understand how DPDK can claim ABI compatibility if we have > > inline functions (unless we freeze any development in these inline functions > > forever). > > > > > > > > > > > > > > > > > Plus synchronization routines: spin/rwlock/barrier, etc. > > > > > > > I think rcu should be one of such exceptions - it is just > > > > > > > another synchronization mechanism after all (just a bit more > > sophisticated). > > > > > > > Konstantin > > > > > > > > > > > > If you look at the other userspace RCU, you wil see that the > > > > > > only inlines are the rcu_read_lock,rcu_read_unlock and > > > > rcu_reference/rcu_assign_pointer. > > > > > > > > > > > > The synchronization logic is all real functions. > > > > > > > > > > In fact, I think urcu provides both flavors: > > > > > https://github.com/urcu/userspace- > > > > rcu/blob/master/include/urcu/static/ > > > > > urcu-qsbr.h I still don't understand why we have to treat it > > > > > differently then let say spin-lock/ticket-lock or rwlock. > > > > > If we gone all the way to create our own version of rcu, we > > > > > probably want it to be as fast as possible (I know that main > > > > > speedup should come from the fact that readers don't have to wait > > > > > for writer to finish, but still...) > > > > > > > > > > Konstantin > > > > > > > > > > > > > Having locking functions inline is already a problem in current releases. > > > > The implementation can not be improved without breaking ABI (or > > > > doing special workarounds like lock v2) > > > I think ABI and inline function discussion needs to be taken up in a > > different thread. > > > > > > Currently, I am looking to hide the structure visibility. I looked at your > > patch [1], it is a different case than what I have in this patch. It is a pretty > > generic use case as well (similar situation exists in other libraries). I think a > > generic solution should be agreed upon. > > > > > > If we have to hide the structure content, the handle to QS variable > > returned to the application needs to be opaque. I suggest using 'void *' > > behind which any structure can be used. > > > > > > typedef void * rte_rcu_qsbr_t; > > > typedef void * rte_hash_t; > > > > > > But it requires typecasting. > > > > > > [1] http://patchwork.dpdk.org/cover/52609/ > > > > C allows structure to be defined without knowing what is in it therefore. > > > > typedef struct rte_rcu_qsbr rte_rcu_qsbr_t; > > > > is preferred (or do it without typedef) > > > > struct rte_rcu_qsbr; > > I see that rte_hash library uses the same approach (struct rte_hash in rte_hash.h, though it is marking as internal). But the ABI Laboratory tool [1] seems to be reporting incorrect numbers for this library even though the internal structure is changed. > > [1] https://abi-laboratory.pro/index.php?view=compat_report&l=dpdk&v1=19.02&v2=current&obj=66794&kind=abi The problem is rte_hash structure is exposed as part of ABI in rte_cuckoo_hash.h This was a mistake.