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 9FAD5A057B; Tue, 14 Apr 2020 15:18:49 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 740F91C22E; Tue, 14 Apr 2020 15:18:49 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 4DF2B1C222 for ; Tue, 14 Apr 2020 15:18:47 +0200 (CEST) IronPort-SDR: y5CqiEkjDcAvAUI6DhHQQ0A+g6leKaAUFH6YVv67EaRpsOoZLS6dlKAL94QQSWVgQsD77Z0mjF wXlUdeF+NoHg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Apr 2020 06:18:46 -0700 IronPort-SDR: L/QZc6EafAkAevMeAad6kqSz0a2rawYVK9fAg4DwfwZrNT/+OYmt1qeqW6oIP2Vs5nZgFHDsKd 5mMqnbscs83Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,382,1580803200"; d="scan'208";a="399947307" Received: from orsmsx102.amr.corp.intel.com ([10.22.225.129]) by orsmga004.jf.intel.com with ESMTP; 14 Apr 2020 06:18:46 -0700 Received: from orsmsx123.amr.corp.intel.com (10.22.240.116) by ORSMSX102.amr.corp.intel.com (10.22.225.129) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 14 Apr 2020 06:18:46 -0700 Received: from ORSEDG001.ED.cps.intel.com (10.7.248.4) by ORSMSX123.amr.corp.intel.com (10.22.240.116) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 14 Apr 2020 06:18:45 -0700 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.108) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 14 Apr 2020 06:18:45 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OjlldTMR2boOwnyza/mcI9rlV9KQ1IpsPU9lVEUsle68Z9PYpSpcTE83+CT4NA6m40157g/WZOk2mCoPe5BhPZYobgUFUUSvEYLaGuGZiPqGH2YCHfuwD66jyJfC/fzLFU0WerJ1dxjw8q0//7Xi33ChoLkSALqigmbvyeAH9wKh9uH6VkUWQwXU9l0cOBXWCJJ5qYvnGlwRW1vu5UfIAbfTRbtShwViZYjyBXZO2POMVH8SydhSYU0Ia1Z6lOA9U9M/KWNha5IMgCH3zAbEX3WMPoQ5HKRQ/5bZj7IGiS8uGaYbERDnlTsE4wvQPq6qM9qeVBd5i35ZV4Yh/wi2Wg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=z6MN2wqzF66SpXhE43McdM5Kihj9XmTnJo7b17QvAbI=; b=J5Wcsy8vIq5sfpV5KUwA9KcIwgi/GSYOG5F9uWH93clSIJMqdvWlq5hSb4NML54TP71LGWzdCtPcdf3i49HA/B0emRNYsL+2XcE1inSCeD7hz8yI5iJ1jGNsFbCur04v7mibfMNPf+5JOwRyIEoG97E+7s7HafuZVGz2O3FdpmjOSk3lUUsvZFqri6sGlh1AUdZtYdaV0p6+Ld3XsgP+k1Maj/Ri/+Kz6HHan4RzrXdftcS4v+Smz4om1hmtji37XjcQVC5E7kTquKzA/92NmSC9uJ2xzgkx6fP9fLQTQYgkV7kN40W9AoamIGmztp1uSiRbdWUfi/13bA/SYhB+Qg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=z6MN2wqzF66SpXhE43McdM5Kihj9XmTnJo7b17QvAbI=; b=osS2yZ5Wobgx45+5f482GnNRXXHCZbFplr3Jgap0AXET7jLX1mRJDe8uK3A0WtfcSOhF2TVzJ7QS5RXI/CaMKiH13IEcrFkky3sd7GdS0UAPm1N2nF0wI4ZpcwuqjAhAUW17EQzRgLjnV+IfvqrgPXcOp0IP8q5b3DAIZIoq9Z0= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by BYAPR11MB2870.namprd11.prod.outlook.com (2603:10b6:a02:cb::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2900.26; Tue, 14 Apr 2020 13:18:44 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f8cb:58cd:e958:fff4]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f8cb:58cd:e958:fff4%6]) with mapi id 15.20.2900.028; Tue, 14 Apr 2020 13:18:44 +0000 From: "Ananyev, Konstantin" To: Honnappa Nagarahalli , "dev@dpdk.org" CC: "david.marchand@redhat.com" , "jielong.zjl@antfin.com" , nd , nd Thread-Topic: [PATCH v3 3/9] ring: introduce RTS ring mode Thread-Index: AQHWCd9yutInr8fFKkq13RS+eoOjdahusX8AgAIjjGCAAjG4gIAFm0HA Date: Tue, 14 Apr 2020 13:18:44 +0000 Message-ID: References: <20200402220959.29885-1-konstantin.ananyev@intel.com> <20200403174235.23308-1-konstantin.ananyev@intel.com> <20200403174235.23308-4-konstantin.ananyev@intel.com> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=konstantin.ananyev@intel.com; x-originating-ip: [192.198.151.172] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 83922c81-25db-4fdc-fcd9-08d7e0765b8e x-ms-traffictypediagnostic: BYAPR11MB2870: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:5516; x-forefront-prvs: 0373D94D15 x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10019020)(396003)(136003)(346002)(376002)(366004)(39850400004)(478600001)(4326008)(2906002)(26005)(7696005)(186003)(9686003)(86362001)(6506007)(316002)(55016002)(66446008)(5660300002)(54906003)(66946007)(81156014)(71200400001)(52536014)(110136005)(66476007)(8676002)(66556008)(33656002)(8936002)(64756008)(76116006)(21314003); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: gzpY01GT4uNRX2EONAVuFYJBENmHcCQB3GzkS2tOc4tr9zoTSsqei8sS3DsWRVmwIQBl98LgQKnzcpOa81KUODwy7vSl9K1p9wRay2JsuUJtwaMnow1zl8Hlaok11DRYKzZF8TF0yvM9zv6SjpOnRg/R1CR8HwumJKv3sXTfP+Q6Twn2WvbZs5FyQl8lw62SwRs7w0g2xeE/0oW4SJ9dim6TXjFwWy2R1Wu1KWxagVC7PCrUXlWiDRdu3iYByLH4mf5KCpHKXxZUz12agjK1YGFeSxQlyMo8ycrANIb+ySs/fV8o6mjBOb4qPMhjFPO3MLtWi/O5vbaBtsPhj4Dc566tCNsntCn3RU54f4eU2+Sdo8fOcG/R13T+UR4llHe/dd7Aev2ubrcd8iKystIe54rTVsjxcq3G2ct9mZ3vK0IZvALKRG2fHPXcdzD3sdrp29gfNADENTSa6RikFWl8W7pOKN9PTlPV37TvLKDaZXdIh9m18uTycD6ODFkjmWe0 x-ms-exchange-antispam-messagedata: 3/hyECqWJqS6489VXoVv/2AkxvQqyaGDL6yRFNklPkiXfWA29iTeCDvQJLBeOUlBVG7zh2ztyVeG5VbKADdbg2IhGB7rQTBYXaUzPD1Z380yve1CJ4VFjbrPEArLYa77wUyBUNdQ30PYZI20daD8YA== x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 83922c81-25db-4fdc-fcd9-08d7e0765b8e X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Apr 2020 13:18:44.1315 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: QoOxJQlvlWc6hVT6USwmWmS1Q5bShSOXYqHQYe8GR6f9g9+0Puk5JbxgXTYREajF9NunlBMyzCR+BpqX0OZB/wfmzQ0CIBbp0OusXNtL6b8= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB2870 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v3 3/9] ring: introduce RTS ring mode 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" > > > > > > > > +#ifdef ALLOW_EXPERIMENTAL_API > > > > +#include > > > > +#endif > > > > + > > > > /** > > > > * Enqueue several objects on a ring. > > > > * > > > > @@ -484,8 +520,21 @@ static __rte_always_inline unsigned int > > > > rte_ring_enqueue_bulk(struct rte_ring *r, void * const *obj_table, > > > > unsigned int n, unsigned int *free_space) { > > > > - return __rte_ring_do_enqueue(r, obj_table, n, > > > > RTE_RING_QUEUE_FIXED, > > > > - r->prod.sync_type, free_space); > > > > + switch (r->prod.sync_type) { > > > > + case RTE_RING_SYNC_MT: > > > > + return rte_ring_mp_enqueue_bulk(r, obj_table, n, free_space); > > > > + case RTE_RING_SYNC_ST: > > > > + return rte_ring_sp_enqueue_bulk(r, obj_table, n, free_space); > > > Have you validated if these affect the performance for the existing A= PIs? > > > > I run ring_pmd_perf_autotest > > (AFAIK, that's the only one of our perf tests that calls > > rte_ring_enqueue/dequeue), and didn't see any real difference in perf > > numbers. > > > > > I am also wondering why should we support these new modes in the lega= cy > > APIs? > > > > Majority of DPDK users still do use legacy API, and I am not sure all o= f them > > will be happy to switch to _elem_ one manually. > > Plus I can't see how we can justify that after let say: > > rte_ring_init(ring, ..., RING_F_MP_HTS_ENQ | RING_F_MC_HTS_DEQ); return= s > > with success valid call to rte_ring_enqueue(ring,...) should fail. > Agree, I think the only way right now is through documentation. >=20 > > > > > I think users should move to use rte_ring_xxx_elem APIs. If users wan= t to > > use RTS/HTS it will be a good time for them to move to new APIs. > > > > If they use rte_ring_enqueue/dequeue all they have to do - just change = flags > > in ring_create/ring_init call. > > With what you suggest - they have to change every > > rte_ring_enqueue/dequeue to rte_ring_elem_enqueue/dequeue. > > That's much bigger code churn. > But these are just 1 to 1 mapped. I would think, there are not a whole l= ot of them in the application code, may be ~10 lines? I suppose it depends a lot on particular user app. My preference not to force users to do extra changes in their code. If we can add new functionality while keeping existing API, why not to do i= t? Less disturbance for users seems a good thing to me. > I think the bigger factor for the user here is the algorithm changes in r= te_ring library. Bigger effort for the users would be testing rather than > code changes in the applications. > > > > > They anyway have to test their code for RTS/HTS, might as well make t= he > > change to new APIs and test both. > > > It will be less code to maintain for the community as well. > > > > That's true, right now there is a lot of duplication between _elem_ and= legacy > > code. > > Actually the only real diff between them - actual copying of the objec= ts. > > But I thought we are going to deal with that, just by changing one day= all > > legacy API to wrappers around _elem_ calls, i.e something like: > > > > static __rte_always_inline unsigned int > > rte_ring_enqueue_bulk(struct rte_ring *r, void * const *obj_table, > > unsigned int n, unsigned int *free_space) { > > return rte_ring_enqueue_elem_bulk(r, obj_table, sizeof(uintptr_t), n, > > free_space); } > > > > That way users will switch to new API automatically, without any extra = effort > > for them, and we will be able to remove legacy code. > > Do you have some other thoughts here how to deal with this legacy/elem > > conversion? > Yes, that is what I was thinking, but had not considered any addition of = new APIs. > But, I am wondering if we should look at deprecation? You mean to deprecate existing legacy API? rte_ring_enqueue/dequeue_bulk, etc? I don't think we need to deprecate it at all. As long as we'll have _elem_ functions called underneath there would be on= e implementation anyway, and we can leave them forever, so users wouldn't need to change their exist= ing code at all.=20 > If we decide to deprecate, it would be good to avoid making the users of = RTS/HTS do > the work twice (once to make the switch to RTS/HTS and then another to _e= lem_ APIs). >=20 > One thing we can do is to implement the wrappers you mentioned above for = RTS/HTS now. That's a very good point. It will require some re-org to allow rte_ring.h to include rte_ring_elem.h= , but I think it is doable, will try to make these changes in v4.=20 > I also it is worth considering to switch to these > wrappers 20.05 so that come 20.11, we have a code base that has gone thro= ugh couple of releases' testing. You mean wrappers for existing legacy API (MP/MC, SP/SC modes)? It is probably too late to make such changes in 20.05, probably early 20.08= is a good time for that. =20 >=20 > >=20 > > > > + > > > > +#endif /* _RTE_RING_RTS_ELEM_H_ */ > > > > diff --git a/lib/librte_ring/rte_ring_rts_generic.h > > > > b/lib/librte_ring/rte_ring_rts_generic.h > > > > new file mode 100644 > > > > index 000000000..f88460d47 > > > > --- /dev/null > > > > +++ b/lib/librte_ring/rte_ring_rts_generic.h > > > I do not know the benefit to providing the generic version. Do you kn= ow > > why this was done in the legacy APIs? > > > > I think at first we had generic API only, then later C11 was added. > > As I remember, C11 one on IA was measured as a bit slower then generic, > > so it was decided to keep both. > > > > > If there is no performance difference between generic and C11 version= s, > > should we just skip the generic version? > > > The oldest compiler in CI are GCC 4.8.5 and Clang 3.4.2 and C11 built= -ins > > are supported earlier than these compiler versions. > > > I feel the code is growing exponentially in rte_ring library and we s= hould try > > to cut non-value-ass code/APIs aggressively. > > > > I'll check is there perf difference for RTS and HTS between generic and= C11 > > versions on IA. > > Meanwhile please have a proper look at C11 implementation, I am not tha= t > > familiar with C11 atomics yet. > ok >=20 > > If there would be no problems with it and no noticeable diff in perform= ance - > > I am ok to have for RTS/HTS modes C11 version only. >From what I see on my box, there is no much difference in terms of performance between *generic* and *c11_mem* for RTS/HTS. ring_stress_autotest for majority of cases shows ~1% diff, in some cases c11 numbers are even a bit better. So will keep c11 version only in v4.