From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 50CAE20F for ; Tue, 10 Feb 2015 03:45:49 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 09 Feb 2015 18:40:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,547,1418112000"; d="scan'208";a="664028148" Received: from pgsmsx106.gar.corp.intel.com ([10.221.44.98]) by fmsmga001.fm.intel.com with ESMTP; 09 Feb 2015 18:45:38 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by PGSMSX106.gar.corp.intel.com (10.221.44.98) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 10 Feb 2015 10:45:37 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.62]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.209]) with mapi id 14.03.0195.001; Tue, 10 Feb 2015 10:45:37 +0800 From: "Liang, Cunming" To: Olivier MATZ , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v4 05/17] eal: new TLS definition and API declaration Thread-Index: AQHQPoxkgmtTQs0uZE+mdAtUqfOWdJzmsHSAgAGa2DD//8xwAIABIelw Date: Tue, 10 Feb 2015 02:45:37 +0000 Message-ID: References: <1422491072-5114-1-git-send-email-cunming.liang@intel.com> <1422842559-13617-1-git-send-email-cunming.liang@intel.com> <1422842559-13617-6-git-send-email-cunming.liang@intel.com> <54D7C045.4070701@6wind.com> <54D8EDA8.7030303@6wind.com> In-Reply-To: <54D8EDA8.7030303@6wind.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 05/17] eal: new TLS definition and API declaration X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Feb 2015 02:45:49 -0000 > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Tuesday, February 10, 2015 1:26 AM > To: Liang, Cunming; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v4 05/17] eal: new TLS definition and API > declaration >=20 > Hi, >=20 > On 02/09/2015 01:45 PM, Liang, Cunming wrote: > >>> +/** > >>> + * Dump the current pthread cpuset. > >>> + * This function is private to EAL. > >>> + * > >>> + * @param str > >>> + * The string buffer the cpuset will dump to. > >>> + * @param size > >>> + * The string buffer size. > >>> + */ > >>> +#define CPU_STR_LEN 256 > >>> +void > >>> +eal_thread_dump_affinity(char str[], unsigned size); > >> > >> Although it's equivalent for function arguments, I think "char *str" i= s > >> usually preferred over "char str[]". See for instance in snprintf() or > >> fgets(). > > [LCM] Accept. > >> > >> What is the purpose of CPU_STR_LEN? > > [LCM] For default quick reference for str[] definition used in dump_aff= inity() >=20 > So the API comment of the function is not placed at the right > place. >=20 > A comment "Default buffer size to use with eal_thread_dump_affinity()" > should be added above CPU_STR_LEN. Also, it could be renamed in > RTE_CPU_STR_LEN or RTE_CPU_AFFINITY_STR_LEN. [LCM] Got you. >=20 >=20 >=20 > >>> @@ -80,7 +81,9 @@ struct lcore_config { > >>> */ > >>> extern struct lcore_config lcore_config[RTE_MAX_LCORE]; > >>> > >>> -RTE_DECLARE_PER_LCORE(unsigned, _lcore_id); /**< Per core "core id".= */ > >>> +RTE_DECLARE_PER_LCORE(unsigned, _lcore_id); /**< Per thread "lcore = id". > >> */ > >>> +RTE_DECLARE_PER_LCORE(unsigned, _socket_id); /**< Per thread "socket > id". > >> */ > >>> +RTE_DECLARE_PER_LCORE(rte_cpuset_t, _cpuset); /**< Per thread > "cpuset". > >> */ > >>> > >>> /** > >>> * Return the ID of the execution unit we are running on. > >>> @@ -146,7 +149,7 @@ rte_lcore_index(int lcore_id) > >>> static inline unsigned > >>> rte_socket_id(void) > >>> { > >>> - return lcore_config[rte_lcore_id()].socket_id; > >>> + return RTE_PER_LCORE(_socket_id); > >>> } > >> > >> I don't see where the _socket_id variable is assigned. I think there > >> is probably an issue with the splitting of the patches. > > [LCM] The value initializes as SOCKET_ID_ANY when RTE_DEFINE_PER_LCORE(= ). > > And updated in eal_thread_set_affinity() for EAL thread and > rte_thread_set_affinity() for non-EAL thread. >=20 > This is done in a later patches: >=20 > "eal: set _lcore_id and _socket_id to (-1) by default" > "eal: apply affinity of EAL thread by assigned cpuset" >=20 > That's why I said there is probably an issue with the ordering > of the patches as these values are used here but initialized > later in the series. [LCM] Will reorder them in next version.