From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-eopbgr80085.outbound.protection.outlook.com [40.107.8.85]) by dpdk.org (Postfix) with ESMTP id 0CBF15F24 for ; Fri, 7 Dec 2018 08:27:18 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=nfo++2s/vwroRxal4NFVp1M7Xco3ZRArYMMx4DPPMjw=; b=oS4NoRxwRuYNG/q4h6XF8PEDqCQSBPTcba4Nxs4h4GApqfA46qLLNAQsVzBGfdFkyTLF7BnUQ8wtRNShefW3GFNsBywyfnFs6fzteBCRG611Elgxuv6OqGJtvu0Mn3GaYMVWtuRCAPhe+94LFJo8KeHmegywUjRHcaKpoTW9lck= Received: from AM6PR08MB3672.eurprd08.prod.outlook.com (20.177.115.29) by AM6PR08MB4087.eurprd08.prod.outlook.com (20.179.1.225) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1404.22; Fri, 7 Dec 2018 07:27:17 +0000 Received: from AM6PR08MB3672.eurprd08.prod.outlook.com ([fe80::78ab:2bf4:5476:6c3e]) by AM6PR08MB3672.eurprd08.prod.outlook.com ([fe80::78ab:2bf4:5476:6c3e%2]) with mapi id 15.20.1404.021; Fri, 7 Dec 2018 07:27:16 +0000 From: Honnappa Nagarahalli To: "Ananyev, Konstantin" , "dev@dpdk.org" CC: nd , Dharmik Thakkar , Malvika Gupta , "Gavin Hu (Arm Technology China)" , Honnappa Nagarahalli , nd Thread-Topic: [dpdk-dev] [RFC 2/3] tqs: add thread quiescent state library Thread-Index: AQHUghQLRVNcuR2lRU2RH6S2NG2yyqVeB0ZggAUcBqCAAiq/IIANopHQ Date: Fri, 7 Dec 2018 07:27:16 +0000 Message-ID: References: <20181122033055.3431-1-honnappa.nagarahalli@arm.com> <20181122033055.3431-3-honnappa.nagarahalli@arm.com> <2601191342CEEE43887BDE71AB977258010CEBBBA9@IRSMSX106.ger.corp.intel.com> <2601191342CEEE43887BDE71AB977258010CEBD5D4@IRSMSX106.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258010CEBD5D4@IRSMSX106.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Honnappa.Nagarahalli@arm.com; x-originating-ip: [217.140.103.75] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM6PR08MB4087; 6:ErkdpnNpZO+6GXruL5Mw+vmdO4ebRrFvNUwzqK1G67ZwOBwj4C/zIaNM3Fv1nUdIPt0GVyMm4Yh2/FD+E8s06OrzXbZzEwFWws8HsNMFh7FbsWDrKyojL3OP0YzYFtHVkMtz8rxumEss6kYOAC2TyTJXualSe1kHjoKI6Euop8X03G+i/hIxX6tJUsgxO/2RyEmPI469fRqnlPU7gqFOgu7GmjS1WfqR8bustbXWHLkKeHQ1A9XTUqhtW6yRdRXiyjDa7iaiIqQuhEjZKqch2B/c8fN1GiHb9die3Swi0Ue6hyMHzKBv1hG7IM82diE0q3ZiNJCVXHv1xPO98/reZSF2NS4PSiFKazRR0/tvWZGcX34Py8xpPK2xXNoymTEByJgXP3bh3PnpyNomndYd0KbHvf2O+flmt3RWZxIwNsBLMaBSNBzrYXQ6Y7vXeEoSNqZ9qx5rIbTFiz87Kp6hTw==; 5:vVrKjBctxoglF+O/4T5RutelMa1zER88tjBWifE0/zVBnIUCeysfvbwtmzPxjDmd1QNOeI2FEy0bIBbxnBLkV9pM+Dlxph0IEJd7C96QMdblGhdNqRg7ipb66HOJ2fI6wHrkpdzRgSyGQtdk3VeYuCSCh+afXzK7EPs0B5jVTP4=; 7:XW9sq7cWwH4EdjDQym1u8Jowve3LZadPMpYJVVqvpzJkx4nwg+QHx8byWSMTLx1fiVR91T+rCa4SbZMsAyxwuqJlGmHysL+a/x/1BURZOnH9HXmOn98BzZrFOX5NCz4FYBmjoV+fCuBmfC57wNfuWg== x-ms-exchange-antispam-srfa-diagnostics: SOS;SOR; x-ms-office365-filtering-correlation-id: 85ddb5a5-96a9-4628-4816-08d65c156a79 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390098)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:AM6PR08MB4087; x-ms-traffictypediagnostic: AM6PR08MB4087: nodisclaimer: True x-microsoft-antispam-prvs: x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(10201501046)(93006095)(93001095)(3002001)(3231455)(999002)(944501520)(52105112)(6055026)(148016)(149066)(150057)(6041310)(20161123562045)(20161123558120)(20161123564045)(20161123560045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991095); SRVR:AM6PR08MB4087; BCL:0; PCL:0; RULEID:; SRVR:AM6PR08MB4087; x-forefront-prvs: 0879599414 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(396003)(136003)(376002)(346002)(39860400002)(199004)(189003)(43544003)(256004)(6436002)(14444005)(74316002)(305945005)(7736002)(81166006)(81156014)(8936002)(33656002)(71190400001)(66066001)(71200400001)(97736004)(229853002)(8676002)(4326008)(68736007)(316002)(5660300001)(6506007)(102836004)(26005)(186003)(106356001)(99286004)(2906002)(105586002)(7696005)(76176011)(93886005)(72206003)(478600001)(86362001)(486006)(9686003)(6116002)(3846002)(14454004)(110136005)(55016002)(53936002)(54906003)(476003)(446003)(11346002)(6246003)(25786009)(2501003); DIR:OUT; SFP:1101; SCL:1; SRVR:AM6PR08MB4087; H:AM6PR08MB3672.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: sOUE0MQ3pCGCWeB+9DyWvYY0nCvL0v4Ei02bZHXv42Dgx16WnhX9BitS/OT1i/YWh015fSREr1MQ/lcb6zxkdED1nQ1uHpqJU8RdzkncFahHCt3/z/+N6yK24nzr1KOVgf94pLW/WTILWhG4bGngNwBQhWGnWHkk905T7TtVo0sP0iW0z/BIny68BJ6oCeuKeUdqsJb/yYoN0slv0mmNzF/YoglthXaJre3NPSgQt9iUMl+rfDLtVcdBQB0WIrv5gV3o4N6m5S4hYcZQ/h/PO8/S5Wh7lr+Vcuq7Mi+YKxtiyiY3GFwVo5QtelqC5Edl8yf2JW9gPUfyr91MUj6o1xtT27E99e1h4J0yyq4OGy0= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 85ddb5a5-96a9-4628-4816-08d65c156a79 X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Dec 2018 07:27:16.9185 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR08MB4087 Subject: Re: [dpdk-dev] [RFC 2/3] tqs: add thread quiescent state library 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: , X-List-Received-Date: Fri, 07 Dec 2018 07:27:19 -0000 > > > > > > + > > > > +/* Add a reader thread, running on an lcore, to the list of > > > > +threads > > > > + * reporting their quiescent state on a TQS variable. > > > > + */ > > > > +int __rte_experimental > > > > +rte_tqs_register_lcore(struct rte_tqs *v, unsigned int lcore_id) { > > > > + TQS_RETURN_IF_TRUE((v =3D=3D NULL || lcore_id >=3D > > > RTE_TQS_MAX_LCORE), > > > > + -EINVAL); > > > > > > It is not very good practice to make function return different > > > values and behave in a different way in debug/non-debug mode. > > > I'd say that for slow-path (functions in .c) it is always good to > > > check input parameters. > > > For fast-path (functions in .h) we sometimes skip such checking, but > > > debug mode can probably use RTE_ASSERT() or so. > > Makes sense, I will change this in the next version. > > > > > > > > > > > lcore_id >=3D RTE_TQS_MAX_LCORE > > > > > > Is this limitation really necessary? > > I added this limitation because currently DPDK application cannot take > > a mask more than 64bit wide. Otherwise, this should be as big as > RTE_MAX_LCORE. > > I see that in the case of '-lcores' option, the number of lcores can > > be more than the number of PEs. In this case, we still need a MAX limit= (but > can be bigger than 64). > > > > > First it means that only lcores can use that API (at least data-path > > > part), second even today many machines have more than 64 cores. > > > I think you can easily avoid such limitation, if instead of > > > requiring lcore_id as input parameter, you'll just make it return ind= ex of > next available entry in w[]. > > > Then tqs_update() can take that index as input parameter. > > I had thought about a similar approach based on IDs. I was concerned > > that ID will be one more thing to manage for the application. But, I se= e the > limitations of the current approach now. I will change it to allocation b= ased. > This will support even non-EAL pthreads as well. >=20 > Yes, with such approach non-lcore threads will be able to use it also. >=20 I realized that rte_tqs_register_lcore/ rte_tqs_unregister_lcore need to be= efficient as they can be called from the worker's packet processing loop (= rte_event_dequeue_burst allows blocking. So, the worker thread needs to cal= l rte_tqs_unregister_lcore before calling rte_event_dequeue_burst and rte_t= qs_register_lcore before starting packet processing). Allocating the thread= ID in these functions will make them more complex. I suggest that we change the argument 'lcore_id' to 'thread_id'. The applic= ation could use 'lcore_id' as 'thread_id' if threads are mapped to physical= cores 1:1. If the threads are not mapped 1:1 to physical cores, the threads need to us= e a thread_id in the range of 0 - RTE_TQS_MAX_THREADS. I do not see that DP= DK has a thread_id concept. For TQS, the thread IDs are global (i.e. not pe= r TQS variable). I could provide APIs to do the thread ID allocation, but I= think the thread ID allocation should not be part of this library. Such th= read ID might be useful for other libraries. >=20 > > > > > > > > > + > > > > + while (lcore_mask) { > > > > + l =3D __builtin_ctz(lcore_mask); > > > > + if (v->w[l].cnt !=3D t) > > > > + break; > > > > > > As I understand, that makes control-path function progress dependent > > > on simultaneous invocation of data-path functions. > > I agree that the control-path function progress (for ex: how long to > > wait for freeing the memory) depends on invocation of the data-path > > functions. The separation of 'start', 'check' and the option not to blo= ck in > 'check' provide the flexibility for control-path to do some other work if= it > chooses to. > > > > > In some cases that might cause control-path to hang. > > > Let say if data-path function wouldn't be called, or user invokes > > > control-path and data-path functions from the same thread. > > I agree with the case of data-path function not getting called. I > > would consider that as programming error. I can document that warning i= n > the rte_tqs_check API. >=20 > Sure, it can be documented. > Though that means, that each data-path thread would have to do explicit > update() call for every tqs it might use. > I just think that it would complicate things and might limit usage of the= library > quite significantly. Each data path thread has to report its quiescent state. Hence, each data-p= ath thread has to call update() (similar to how rte_timer_manage() has to b= e called periodically on the worker thread).=20 Do you have any particular use case in mind where this fails? >=20 > > > > In the case of same thread calling both control-path and data-path > > functions, it would depend on the sequence of the calls. The following > sequence should not cause any hangs: > > Worker thread > > 1) 'deletes' an entry from a lock-free data structure > > 2) rte_tqs_start > > 3) rte_tqs_update > > 4) rte_tqs_check (wait =3D=3D 1 or wait =3D=3D 0) > > 5) 'free' the entry deleted in 1) >=20 > That an interesting idea, and that should help, I think. > Probably worth to have {2,3,4} sequence as a new high level function. >=20 Yes, this is a good idea. Such a function would be applicable only in the w= orker thread. I would prefer to leave it to the application to take care.