From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 957F6255 for ; Mon, 16 Jul 2018 18:29:30 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Jul 2018 09:29:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,361,1526367600"; d="scan'208";a="72780432" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.237.220.102]) ([10.237.220.102]) by fmsmga001.fm.intel.com with ESMTP; 16 Jul 2018 09:28:59 -0700 To: Olivier Matz , Honnappa Nagarahalli Cc: dev@dpdk.org, nd@arm.com, John McNamara References: <1531195247-22612-1-git-send-email-honnappa.nagarahalli@arm.com> <20180716070018.4dvpx73vy5o2nplt@platinum> From: "Burakov, Anatoly" Message-ID: <2a1b1848-2f59-40ec-5066-560d82694e4c@intel.com> Date: Mon, 16 Jul 2018 17:28:58 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180716070018.4dvpx73vy5o2nplt@platinum> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3] rte_ring: clarify preemptible nature of ring algorithm 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: Mon, 16 Jul 2018 16:29:31 -0000 On 16-Jul-18 8:00 AM, Olivier Matz wrote: > Hi, > > On Mon, Jul 09, 2018 at 11:00:47PM -0500, Honnappa Nagarahalli wrote: >> rte_ring implementation is not preemptible only under certain >> circumstances. This clarification is helpful for data plane and >> control plane communication using rte_ring. >> >> Signed-off-by: Honnappa Nagarahalli >> Reviewed-by: Gavin Hu >> Reviewed-by: Ola Liljedahl >> --- >> v3: >> * Corrected known issues for rte_ring >> * Referred to known issues in rte_ring.h (Burakov, Oliver) >> >> v2: >> * Fixed checkpatch warnings >> >> doc/guides/prog_guide/env_abstraction_layer.rst | 27 ++++++++++++++----------- >> lib/librte_ring/rte_ring.h | 5 +++-- >> 2 files changed, 18 insertions(+), 14 deletions(-) >> >> diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst b/doc/guides/prog_guide/env_abstraction_layer.rst >> index a22640d..f47a4be 100644 >> --- a/doc/guides/prog_guide/env_abstraction_layer.rst >> +++ b/doc/guides/prog_guide/env_abstraction_layer.rst >> @@ -435,23 +435,26 @@ Known Issues >> >> The "non-preemptive" constraint means: >> >> - - a pthread doing multi-producers enqueues on a given ring must not >> - be preempted by another pthread doing a multi-producer enqueue on >> - the same ring. >> - - a pthread doing multi-consumers dequeues on a given ring must not >> - be preempted by another pthread doing a multi-consumer dequeue on >> - the same ring. >> + A preempted pthread can block other pthreads (operating on the same ring) >> + from completing their operations, only if those pthreads are performing >> + the same ring operation (enqueue/dequeue) as the preempted pthread. >> + In other words, a preempted consumer pthread will not block any producer >> + pthreads and vice versa. >> >> - Bypassing this constraint may cause the 2nd pthread to spin until the 1st one is scheduled again. >> - Moreover, if the 1st pthread is preempted by a context that has an higher priority, it may even cause a dead lock. >> + Bypassing this constraint may cause other pthreads to spin until the preempted pthread is scheduled again. >> + Moreover, if the pthread is preempted by a context that has a higher priority, it may even cause a dead lock. > > Your reword makes this section more generic, but I'm not sure it is > really clearer. IMO it can stay as it was. > > John, Anatoly, do you have any opinion? I think that part can stay as it was. As much as avoiding overly specific wording is useful, in this case the original specificity is warranted because thinking about this stuff makes enough heads hurt as it is :) > > >> - This does not mean it cannot be used, simply, there is a need to narrow down the situation when it is used by multi-pthread on the same core. >> + This means, use cases involving preemptible pthreads should consider using rte_ring carefully. >> >> - 1. It CAN be used for any single-producer or single-consumer situation. >> + 1. It CAN be used for preemptible single-producer and single-consumer use case. >> >> - 2. It MAY be used by multi-producer/consumer pthread whose scheduling policy are all SCHED_OTHER(cfs). User SHOULD be aware of the performance penalty before using it. >> + 2. It CAN be used for non-preemptible multi-producer and preemptible single-consumer use case. >> >> - 3. It MUST not be used by multi-producer/consumer pthreads, whose scheduling policies are SCHED_FIFO or SCHED_RR. >> + 3. It CAN be used for preemptible single-producer and non-preemptible multi-consumer use case. >> + >> + 4. It MAY be used by preemptible multi-producer and/or preemptible multi-consumer pthreads whose scheduling policy are all SCHED_OTHER(cfs), SCHED_IDLE or SCHED_BATCH. User SHOULD be aware of the performance penalty before using it. >> + >> + 5. It MUST not be used by multi-producer/consumer pthreads, whose scheduling policies are SCHED_FIFO or SCHED_RR. >> >> + rte_timer >> >> diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h >> index 1245822..e680101 100644 >> --- a/lib/librte_ring/rte_ring.h >> +++ b/lib/librte_ring/rte_ring.h >> @@ -26,8 +26,9 @@ >> * - Bulk dequeue. >> * - Bulk enqueue. >> * >> - * Note: the ring implementation is not preemptable. A lcore must not >> - * be interrupted by another task that uses the same ring. >> + * Note: the ring implementation is not preemptible. Refer to Programmer's >> + * guide/Environment Abstraction Layer/Multiple pthread/Known Issues/rte_ring >> + * for more information. > > > The other parts looks good to me, thank you. > > -- Thanks, Anatoly