From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-f196.google.com (mail-pl1-f196.google.com [209.85.214.196]) by dpdk.org (Postfix) with ESMTP id 894E61B5E9 for ; Tue, 18 Dec 2018 07:31:57 +0100 (CET) Received: by mail-pl1-f196.google.com with SMTP id t13so7329597ply.13 for ; Mon, 17 Dec 2018 22:31:57 -0800 (PST) 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=pxZ9db67/ck0ieEtMeJ7pLl0Vw0RFjHEMdu7RDI0x6A=; b=XyCCNxohpXpAq1QgjbuOXszZWYupi1a0CV3zZaA5ZMVdh+192LyYixvyGt+OTms34c TtP24jo1oRuUg3+uNCWDslOsVw7nu6moZbXqnWZ/0LkLEFk4akwo0NaXb9yZmLJPKkyQ howlIFoPz+61EJ8f9ZcAfW7FYtYw4QcKbK5b2wxlz/djA5V3WkXE4scBByD9x26tb0JY P8hV3E4InH0sbK3E72WrrrSa5dhh5bnnS9dJ3xpwCAD3V5dokdPxmzxWR4lHkvmlps4O ZPbrt9KDh+HVJ/TKQc9zuQvedkKol39GIDzO2s6dcFVeNsMR/d2wfhuUzTwElBZyPvNL yfNw== 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=pxZ9db67/ck0ieEtMeJ7pLl0Vw0RFjHEMdu7RDI0x6A=; b=c+XDSAJAR+YCW6LqrRbspLfHhI9BJJqL4Xh/mk3IkslaPO/+jeaFwm/PiCd2QCwrUi Jvg3g1l1yMkHvCwhjMcQ6vfpemiPPofXa6US1VXIhb5qF9ScRMimsUWmuW+drEy2Zq8Y x1nDr+IhmnhXBbly40tm7I/jdP9LmfDxUWJTlSEk3kWCHw/syzX5o4DVasqJpiuKTMd7 1M7sDUuK8AEUe1Vbry8+qRm1i2bbG5EtEtR5aceTydcwCH49wG5tx59XyXbgZA16wPxc kxV5yuioUiRaNXApruGVlIRpvfXVmslCK07DP5tKkU02qu2WXhiFlK6WRR0l8kujqssj yhZw== X-Gm-Message-State: AA+aEWa8PokqnLDFnYa73iZKAUb403bgQxkjSdKJvsy2GYPHZ+UKRHNk SmIEgsZ/RGMzv5S/sLwclGjfddkxxW8= X-Google-Smtp-Source: AFSGD/X3VtoYOfBVNPZLjLVBzEBeyn1R381TjJGJ2ixzb0qucbKZKXvdd0OF0se1fBhMhAefVuCwVQ== X-Received: by 2002:a17:902:2a29:: with SMTP id i38mr15663617plb.253.1545114716390; Mon, 17 Dec 2018 22:31:56 -0800 (PST) Received: from xeon-e3 (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id h10sm17901412pgn.11.2018.12.17.22.31.55 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 17 Dec 2018 22:31:56 -0800 (PST) Date: Mon, 17 Dec 2018 22:31:48 -0800 From: Stephen Hemminger To: Honnappa Nagarahalli Cc: "Burakov, Anatoly" , "Ananyev, Konstantin" , "dev@dpdk.org" , nd , Dharmik Thakkar , Malvika Gupta , "Gavin Hu (Arm Technology China)" Message-ID: <20181217223148.4945fdcc@xeon-e3> In-Reply-To: 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> <20181207092936.17bf2887@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: Tue, 18 Dec 2018 06:31:57 -0000 On Tue, 18 Dec 2018 04:30:39 +0000 Honnappa Nagarahalli wrote: > > > > On 11-Dec-18 6:40 AM, Honnappa Nagarahalli wrote: > > >> > > >>>>> > > >>>>>>> + > > >>>>>>> +/* 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 == NULL || lcore_id >= > > >>>>>> 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 >= 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 > > >>>>>> index 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 see the > > >>>> limitations of the current approach now. I will change it to > > >>>> allocation > > >> based. > > >>>> This will support even non-EAL pthreads as well. > > >>>> > > >>>> Yes, with such approach non-lcore threads will be able to use it also. > > >>>> > > >>> 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 call rte_tqs_unregister_lcore before calling > > >> rte_event_dequeue_burst and rte_tqs_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 > > >> application 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 use > > >> a thread_id in the range of 0 - RTE_TQS_MAX_THREADS. I do not see > > >> that DPDK has a thread_id concept. For TQS, the thread IDs are global > > >> (i.e. not per 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 thread ID might be useful for other libraries. > > >>> > > >>> > >> > > >> > > >> Thread id is problematic since Glibc doesn't want to give it out. > > >> You have to roll your own function to do gettid(). > > >> It is not as easy as just that. Plus what about preemption? > > > > > > Agree. I looked into this further. The rte_gettid function uses a system call > > (BSD and Linux). I am not clear on the space of the ID returned (as well). I do > > not think it is guaranteed that it will be with in a narrow range that is required > > here. > > > > > > My suggestion would be to add a set of APIs that would allow for > > > allocation of thread IDs which are within a given range of 0 to > > > > > > > > > > System-provided thread-ID's would probably also be potentially non-unique in > > multiprocess scenario? > For linux, rte_gettid is implemented as: > int rte_sys_gettid(void) > { > return (int)syscall(SYS_gettid); > } > > Came across [1] which states, thread-IDs are unique across the system. > > For BSD, thr_self is used. [2] says it provides system wide unique thread IDs. > > [1] https://stackoverflow.com/questions/6372102/what-is-the-difference-between-pthread-self-and-gettid-which-one-should-i-u > [2] https://nxmnpg.lemoda.net/2/thr_self > > > > > -- > > Thanks, > > Anatoly Using thread id directly on Linux is battling against the glibc gods wishes. Bad things may come to those that disobey them :-) But really many libraries need to do the same thing, it is worth looking around. The bigger issue is pid and thread id recycling with the limited range allowed.