Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> > > > > > > > > > > > Subject: [dpdk-dev] [RFC 0/6] New sync modes for ring > > > > > > > > > > Upfront note - that RFC is not a complete patch. > > > > > It introduces an ABI breakage, plus it doesn't update ring_elem > > > > > code properly, > > > > As per the current rules, these changes (in the current form) will > > > > be accepted only for 20.11 release. How do we address this for > > > > immediate > > > requirements like RCU defer APIs? > > > > > > I think I found a way to introduce these new modes without API/ABI > > breakage. > > > Working on v1 right now. Plan to submit it by end of that week/start > > > of next one. > > ok > RCU defer APIs require the rte_ring_xxx_elem versions. I guess you are adding > those as well. Yes, I added it into V1, please have a look. Also I made 'legacy' peek API (enqueue/dequeue_start/finish) to call 'elem' peek API (enqueue/dequeue_elem_start/finish). About naming: thought about changing start/finish to reserve/commit, but decided to left as it is for now - in case you would like to go ahead with SG API and use reserve/commit there. Konstantin
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> > > > > > > > Subject: [dpdk-dev] [RFC 0/6] New sync modes for ring > > > > > > > > Upfront note - that RFC is not a complete patch. > > > > It introduces an ABI breakage, plus it doesn't update ring_elem > > > > code properly, > > > As per the current rules, these changes (in the current form) will > > > be accepted only for 20.11 release. How do we address this for > > > immediate > > requirements like RCU defer APIs? > > > > I think I found a way to introduce these new modes without API/ABI > breakage. > > Working on v1 right now. Plan to submit it by end of that week/start > > of next one. > ok RCU defer APIs require the rte_ring_xxx_elem versions. I guess you are adding those as well. >
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> >
> > > Subject: [dpdk-dev] [RFC 0/6] New sync modes for ring
> > >
> > > Upfront note - that RFC is not a complete patch.
> > > It introduces an ABI breakage, plus it doesn't update ring_elem code
> > > properly,
> > As per the current rules, these changes (in the current form) will be
> > accepted only for 20.11 release. How do we address this for immediate
> requirements like RCU defer APIs?
>
> I think I found a way to introduce these new modes without API/ABI breakage.
> Working on v1 right now. Plan to submit it by end of that week/start of next
> one.
ok
>
> > I suggest that we move forward with my RFC (taking into consideration your
> feedback) to make progress on RCU APIs.
> >
> > > etc.
> > > I plan to deal with all these things in later versions.
> > > Right now I seek an initial feedback about proposed ideas.
> > > Would also ask people to repeat performance tests (see below) on
> > > their platforms to confirm the impact.
> > >
> > > More and more customers use(/try to use) DPDK based apps within
> > > overcommitted systems (multiple acttive threads over same pysical cores):
> > > VM, container deployments, etc.
> > > One quite common problem they hit: Lock-Holder-Preemption with
> rte_ring.
> > > LHP is quite a common problem for spin-based sync primitives
> > > (spin-locks, etc.) on overcommitted systems.
> > > The situation gets much worse when some sort of fair-locking
> > > technique is used (ticket-lock, etc.).
> > > As now not only lock-owner but also lock-waiters scheduling order
> > > matters a lot.
> > > This is a well-known problem for kernel within VMs:
> > > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf
> > > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf
> > > The problem with rte_ring is that while head accusion is sort of
> > > un-fair locking, waiting on tail is very similar to ticket lock
> > > schema - tail has to be updated in particular order.
> > > That makes current rte_ring implementation to perform really pure on
> > > some overcommited scenarios.
> > > While it is probably not possible to completely resolve this problem
> > > in userspace only (without some kernel communication/intervention),
> > > removing fairness in tail update can mitigate it significantly.
> > > So this RFC proposes two new optional ring synchronization modes:
> > > 1) Head/Tail Sync (HTS) mode
> > > In that mode enqueue/dequeue operation is fully serialized:
> > > only one thread at a time is allowed to perform given op.
> > > As another enhancement provide ability to split enqueue/dequeue
> > > operation into two phases:
> > > - enqueue/dequeue start
> > > - enqueue/dequeue finish
> > > That allows user to inspect objects in the ring without removing
> > > them from it (aka MT safe peek).
> > IMO, this will not address the problem described above.
>
> It does, please see the results produced by ring_stress_*autotest below.
> Let say for test-case: 8thread@2core(--lcores='6,(10-13)@7,(20-23)@8' it
Had not looked at these tests. Please see the numbers below.
> shows:
> avg number of cycles per object for enqueue /dequeue:
> MP/MC: 280314.32
> HTS: 294.72
> RTS: 318.79
>
> Customer who tried it reported similar level of improvement.
Is this tested with the VM/Container setup described in the slides you referred
to?
> Actually if you have time - would be very interesting to see what numbers will
> be on ARM boxes.
> To reproduce, just:
> $cat ring_tests_u4
> ring_stress_autotest
> ring_stress_hts_autotest
> ring_stress_rts_autotest
>
> /app/test/dpdk-test --lcores='6,(10-13)@7,(20-23)@8' -n 4 < ring_tests_u4
> 2>&1 | tee res1
>
> Then look at the ' AGGREGATE' stats.
> Right now it is a bit too verbose, so probably the easiest thing to extract
> same
> numbers quickly:
> grep 'cycles/obj' res1 | grep 'cycles/obj' | cat -n | awk '{if ($(1)%9==0)
> print
> $(NF);}'
> 280314.32
> 1057833.55
> 294.72
> 480.10
> 318.79
> 461.52
>
> First 2 numbers will be for MP/MC, next 2 for HTS, last 2 for RTS.
12305.05
12027.09
3.59
7.37
4.41
7.98
>
> > For ex: when a producer updates the head and gets scheduled out, other
> > producers have to spin.
>
> Sure, as I wrote in original cover letter:
> " While it is probably not possible to completely resolve this problem in
> userspace only (without some kernel communication/intervention), removing
> fairness in tail update can mitigate it significantly."
> Results from the ring_stress_*_autotest confirm that.
>
> > The problem is probably worse as with non-HTS case moving of the head
> > and copying of the ring elements can happen in parallel between the
> producers (similarly for consumers).
>
> Yes as we serialize the ring, we remove possibility of simultaneous copy.
> That's why for 'normal' cases (one thread per core) original MP/MC is usually
> faster.
> Though on overcommitted scenarios current MP/MC performance degrades
> dramatically.
> The main probl
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
>
>
>
> > Subject: [dpdk-dev] [RFC 0/6] New sync modes for ring
> >
> > Upfront note - that RFC is not a complete patch.
> > It introduces an ABI breakage, plus it doesn't update ring_elem code
> > properly,
> As per the current rules, these changes (in the current form) will be
> accepted only for 20.11 release. How do we address this for immediate
> requirements like RCU defer APIs?
I think I found a way to introduce these new modes without API/ABI breakage.
Working on v1 right now. Plan to submit it by end of that week/start of next
one.
> I suggest that we move forward with my RFC (taking into consideration your
> feedback) to make progress on RCU APIs.
>
> > etc.
> > I plan to deal with all these things in later versions.
> > Right now I seek an initial feedback about proposed ideas.
> > Would also ask people to repeat performance tests (see below) on their
> > platforms to confirm the impact.
> >
> > More and more customers use(/try to use) DPDK based apps within
> > overcommitted systems (multiple acttive threads over same pysical cores):
> > VM, container deployments, etc.
> > One quite common problem they hit: Lock-Holder-Preemption with rte_ring.
> > LHP is quite a common problem for spin-based sync primitives (spin-locks,
> > etc.)
> > on overcommitted systems.
> > The situation gets much worse when some sort of fair-locking technique is
> > used (ticket-lock, etc.).
> > As now not only lock-owner but also lock-waiters scheduling order matters a
> > lot.
> > This is a well-known problem for kernel within VMs:
> > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf
> > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf
> > The problem with rte_ring is that while head accusion is sort of un-fair
> > locking,
> > waiting on tail is very similar to ticket lock schema - tail has to be
> > updated in
> > particular order.
> > That makes current rte_ring implementation to perform really pure on some
> > overcommited scenarios.
> > While it is probably not possible to completely resolve this problem in
> > userspace only (without some kernel communication/intervention), removing
> > fairness in tail update can mitigate it significantly.
> > So this RFC proposes two new optional ring synchronization modes:
> > 1) Head/Tail Sync (HTS) mode
> > In that mode enqueue/dequeue operation is fully serialized:
> > only one thread at a time is allowed to perform given op.
> > As another enhancement provide ability to split enqueue/dequeue
> > operation into two phases:
> > - enqueue/dequeue start
> > - enqueue/dequeue finish
> > That allows user to inspect objects in the ring without removing
> > them from it (aka MT safe peek).
> IMO, this will not address the problem described above.
It does, please see the results produced by ring_stress_*autotest below.
Let say for test-case: 8thread@2core(--lcores='6,(10-13)@7,(20-23)@8' it shows:
avg number of cycles per object for enqueue /dequeue:
MP/MC: 280314.32
HTS: 294.72
RTS: 318.79
Customer who tried it reported similar level of improvement.
Actually if you have time - would be very interesting to see what numbers will
be on ARM boxes.
To reproduce, just:
$cat ring_tests_u4
ring_stress_autotest
ring_stress_hts_autotest
ring_stress_rts_autotest
/app/test/dpdk-test --lcores='6,(10-13)@7,(20-23)@8' -n 4 < ring_tests_u4 2>&1
| tee res1
Then look at the ' AGGREGATE' stats.
Right now it is a bit too verbose, so probably the easiest thing to extract
same numbers quickly:
grep 'cycles/obj' res1 | grep 'cycles/obj' | cat -n | awk '{if ($(1)%9==0)
print $(NF);}'
280314.32
1057833.55
294.72
480.10
318.79
461.52
First 2 numbers will be for MP/MC, next 2 for HTS, last 2 for RTS.
> For ex: when a producer updates the head and gets scheduled out, other
> producers
> have to spin.
Sure, as I wrote in original cover letter:
" While it is probably not possible to completely resolve this problem in
userspace only (without some kernel communication/intervention),
removing fairness in tail update can mitigate it significantly."
Results from the ring_stress_*_autotest confirm that.
> The problem is probably worse as with non-HTS case moving of the head and
> copying of the ring elements can happen in
> parallel between the producers (similarly for consumers).
Yes as we serialize the ring, we remove possibility of simultaneous copy.
That's why for 'normal' cases (one thread per core) original MP/MC is usually
faster.
Though on overcommitted scenarios current MP/MC performance degrades
dramatically.
The main problem with current MP/MC implementation is in that tail update
have to be done in strict order (sort of fair locking scheme).
Which means that we have much-much worse LHP manifestation,
then when we use unfair schemes.
With serialized ring (HTS) we remove that ordering completely
(same idea as switch from fair to unfair locking for PV spin-locks).
> IMO, HTS should not be a
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> Subject: [dpdk-dev] [RFC 0/6] New sync modes for ring > > Upfront note - that RFC is not a complete patch. > It introduces an ABI breakage, plus it doesn't update ring_elem code properly, As per the current rules, these changes (in the current form) will be accepted only for 20.11 release. How do we address this for immediate requirements like RCU defer APIs? I suggest that we move forward with my RFC (taking into consideration your feedback) to make progress on RCU APIs. > etc. > I plan to deal with all these things in later versions. > Right now I seek an initial feedback about proposed ideas. > Would also ask people to repeat performance tests (see below) on their > platforms to confirm the impact. > > More and more customers use(/try to use) DPDK based apps within > overcommitted systems (multiple acttive threads over same pysical cores): > VM, container deployments, etc. > One quite common problem they hit: Lock-Holder-Preemption with rte_ring. > LHP is quite a common problem for spin-based sync primitives (spin-locks, > etc.) > on overcommitted systems. > The situation gets much worse when some sort of fair-locking technique is > used (ticket-lock, etc.). > As now not only lock-owner but also lock-waiters scheduling order matters a > lot. > This is a well-known problem for kernel within VMs: > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf > The problem with rte_ring is that while head accusion is sort of un-fair > locking, > waiting on tail is very similar to ticket lock schema - tail has to be > updated in > particular order. > That makes current rte_ring implementation to perform really pure on some > overcommited scenarios. > While it is probably not possible to completely resolve this problem in > userspace only (without some kernel communication/intervention), removing > fairness in tail update can mitigate it significantly. > So this RFC proposes two new optional ring synchronization modes: > 1) Head/Tail Sync (HTS) mode > In that mode enqueue/dequeue operation is fully serialized: > only one thread at a time is allowed to perform given op. > As another enhancement provide ability to split enqueue/dequeue > operation into two phases: > - enqueue/dequeue start > - enqueue/dequeue finish > That allows user to inspect objects in the ring without removing > them from it (aka MT safe peek). IMO, this will not address the problem described above. For ex: when a producer updates the head and gets scheduled out, other producers have to spin. The problem is probably worse as with non-HTS case moving of the head and copying of the ring elements can happen in parallel between the producers (similarly for consumers). IMO, HTS should not be a configurable flag. In RCU requirement, a MP enqueue and HTS dequeue are required. > 2) Relaxed Tail Sync (RTS) > The main difference from original MP/MC algorithm is that tail value is > increased not by every thread that finished enqueue/dequeue, but only by the > last one. > That allows threads to avoid spinning on ring tail value, leaving actual tail > value > change to the last thread in the update queue. This can be a configurable flag on the ring. I am not sure how this solves the problem you have stated above completely. Updating the count from all intermediate threads is still required to update the value of the head. But yes, it reduces the severity of the problem by not enforcing the order in which the tail is updated. I also think it introduces the problem on the other side of the ring because the tail is not updated soon enough (the other side has to wait longer for the elements to become available). It also introduces another configuration parameter (HTD_MAX_DEF) which they have to deal with. Users have to still implement the current hypervisor related solutions. IMO, we should run the benchmark for this on an over committed setup to understand the benefits. > > Test results on IA (see below) show significant improvements for average > enqueue/dequeue op times on overcommitted systems. > For 'classic' DPDK deployments (one thread per core) original MP/MC > algorithm still shows best numbers, though for 64-bit target RTS numbers are > not that far away. > Numbers were produced by ring_stress_*autotest (first patch in these series). > > X86_64 @ Intel(R) Xeon(R) Platinum 8160 CPU @ 2.10GHz > DEQ+ENQ average cycles/obj > > MP/MC HTS RTS > 1thread@1core(--lcores=6-7) 8.00 8.158.99 > 2thread@2core(--lcores=6-8) 19.14 19.61 20.35 > 4thread@4core(--lcores=6-10)29.43 29.79 31.82 > 8thread@8core(--lcores=6-14)110.59 192.81 119.50 > 16thread@16core(--lcores=6-22) 461.03 813.12 495.59 > 32thread/@32core(--lcores='6-22,55-70')
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> > > > > > I tested on an arm64 HW. The former section is without the > > patch(20.02) and later one with this patch. > > I agree with Konstantin that getting more platform tests will be good > > early so that we can focus on the approach > > to avoid back and forth latter. > > > > > > RTE>>ring_perf_autotest // without path > > > > ### Testing single element enq/deq ### > > legacy APIs: SP/SC: single: 289.78 > > legacy APIs: MP/MC: single: 516.20 > > > > ### Testing burst enq/deq ### > > legacy APIs: SP/SC: burst (size: 8): 312.88 > > legacy APIs: SP/SC: burst (size: 32): 426.72 > > legacy APIs: MP/MC: burst (size: 8): 510.95 > > legacy APIs: MP/MC: burst (size: 32): 702.01 > > > > ### Testing bulk enq/deq ### > > legacy APIs: SP/SC: bulk (size: 8): 306.74 > > legacy APIs: SP/SC: bulk (size: 32): 411.56 > > legacy APIs: MP/MC: bulk (size: 8): 501.32 > > legacy APIs: MP/MC: bulk (size: 32): 693.07 > > > > ### Testing empty bulk deq ### > > legacy APIs: SP/SC: bulk (size: 8): 7.00 > > legacy APIs: MP/MC: bulk (size: 8): 7.00 > > > > ### Testing using two physical cores ### > > legacy APIs: SP/SC: bulk (size: 8): 74.36 > > legacy APIs: MP/MC: bulk (size: 8): 110.18 > > legacy APIs: SP/SC: bulk (size: 32): 23.04 > > legacy APIs: MP/MC: bulk (size: 32): 32.29 > > > > ### Testing using all slave nodes ## > > Bulk enq/dequeue count on size 8 > > Core [8] count = 293741 > > Core [9] count = 293741 > > Total count (size: 8): 587482 > > > > Bulk enq/dequeue count on size 32 > > Core [8] count = 244909 > > Core [9] count = 244909 > > Total count (size: 32): 1077300 > > > > ### Testing single element enq/deq ### > > elem APIs: element size 16B: SP/SC: single: 255.37 > > elem APIs: element size 16B: MP/MC: single: 456.68 > > > > ### Testing burst enq/deq ### > > elem APIs: element size 16B: SP/SC: burst (size: 8): 291.99 > > elem APIs: element size 16B: SP/SC: burst (size: 32): 456.25 > > elem APIs: element size 16B: MP/MC: burst (size: 8): 497.77 > > elem APIs: element size 16B: MP/MC: burst (size: 32): 680.87 > > > > ### Testing bulk enq/deq ### > > elem APIs: element size 16B: SP/SC: bulk (size: 8): 284.40 > > elem APIs: element size 16B: SP/SC: bulk (size: 32): 453.17 > > elem APIs: element size 16B: MP/MC: bulk (size: 8): 485.77 > > elem APIs: element size 16B: MP/MC: bulk (size: 32): 675.08 > > > > ### Testing empty bulk deq ### > > elem APIs: element size 16B: SP/SC: bulk (size: 8): 8.00 > > elem APIs: element size 16B: MP/MC: bulk (size: 8): 7.00 > > > > ### Testing using two physical cores ### > > elem APIs: element size 16B: SP/SC: bulk (size: 8): 74.45 > > elem APIs: element size 16B: MP/MC: bulk (size: 8): 105.91 > > elem APIs: element size 16B: SP/SC: bulk (size: 32): 22.92 > > elem APIs: element size 16B: MP/MC: bulk (size: 32): 31.55 > > > > ### Testing using all slave nodes ### > > > > Bulk enq/dequeue count on size 8 > > Core [8] count = 308724 > > Core [9] count = 308723 > > Total count (size: 8): 617447 > > > > Bulk enq/dequeue count on size 32 > > Core [8] count = 214269 > > Core [9] count = 214269 > > Total count (size: 32): 1045985 > > > > RTE>>ring_perf_autotest // with patch > > > > ### Testing single element enq/deq ### > > legacy APIs: SP/SC: single: 289.78 > > legacy APIs: MP/MC: single: 475.76 > > > > ### Testing burst enq/deq ### > > legacy APIs: SP/SC: burst (size: 8): 323.91 > > legacy APIs: SP/SC: burst (size: 32): 424.60 > > legacy APIs: MP/MC: burst (size: 8): 523.00 > > legacy APIs: MP/MC: burst (size: 32): 717.09 > > > > ### Testing bulk enq/deq ### > > legacy APIs: SP/SC: bulk (size: 8): 317.74 > > legacy APIs: SP/SC: bulk (size: 32): 413.57 > > legacy APIs: MP/MC: bulk (size: 8): 512.89 > > legacy APIs: MP/MC: bulk (size: 32): 712.45 > > > > ### Testing empty bulk deq ### > > legacy APIs: SP/SC: bulk (size: 8): 7.00 > > legacy APIs: MP/MC: bulk (size: 8): 7.00 > > > > ### Testing using two physical cores ### > > legacy APIs: SP/SC: bulk (size: 8): 74.82 > > legacy APIs: MP/MC: bulk (size: 8): 96.45 > > legacy APIs: SP/SC: bulk (size: 32): 22.97 > > legacy APIs: MP/MC: bulk (size: 32): 32.52 > > > > ### Testing using all slave nodes ### > > > > Bulk enq/dequeue count on size 8 > > Core [8] count = 283928 > > Core [9] count = 283927 > > Total count (size: 8): 567855 > > > > Bulk enq/dequeue count on size 32 > > Core [8] count = 223916 > > Core [9] count = 223915 > > Total count (size: 32): 1015686 > > > > ### Testing single element enq/deq ### > > elem APIs: element size 16B: SP/SC: single: 267.65 > > elem APIs: element size 16B: MP/MC: single: 439.06 > > > > ### Testing burst enq/deq ### > > elem APIs: element size 16B: SP/SC: burst (size: 8): 302.44 > > elem APIs: element size 16B: SP/SC: burst (size: 32): 466.31 > > elem APIs: element size 16B: MP/MC: burst (size: 8): 502.51 > > elem APIs: element size 16B: MP/MC: burst (size: 32): 695.81 > > > > ### Testing bulk enq/deq ### > > elem APIs: element size 16B: SP/SC: bulk (size: 8): 295.15 > > elem APIs: element size 16B: SP/SC: b
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
On Tue, Feb 25, 2020 at 7:11 PM Ananyev, Konstantin wrote: We do have a run-time check in our current enqueue()/dequeue implementation. In fact we support both modes: we have generic rte_ring_enqueue(/dequeue)_bulk(/burst) where sync behaviour is determined at runtime by value of prod(/cons).single. Or user can call rte_ring_(mp/sp)_enqueue_* functions directly. This RFC follows exactly the same paradigm: rte_ring_enqueue(/dequeue)_bulk(/burst) kept generic and it's behaviour is determined at runtime, by value of prod(/cons).sync_type. Or user can call enqueue/dequeue with particular sync mode directly: rte_ring_(mp/sp/rts/hts)_enqueue_(bulk/burst)*. The only thing that changed: Format of prod/cons now could differ depending on mode selected at _init_. So you can't create a ring for let say SP mode and then in the middle of data-path change your mind and start using MP_RTS mode. For existing modes (SP/MP, SC/MC) format remains the same and user can still use them interchangeably, though of course that is an error prone practice. Makes sense. But I agree with the problem statement that in the virtualization use case, It may be possible to have N virtual cores runs on a physical core. IMO, The best solution would be keeping the ring API same and have a different flavor in "compile-time". Something like liburcu did for accommodating different flavors. i.e urcu-qsbr.h and urcu-bp.h will identical definition of API. The application can simply include ONE header file in a C file based on the flavor. I don't think it is a flexible enough approach. In one app user might need to have several rings with different sync modes. Or even user might need a ring with different sync modes for enqueue/dequeue. Ack. Yes, hiding rte_ring implementation inside .c would help a lot in terms of ABI maintenance and would make our future life easier. The question is what is the price for it in terms of performance, and are we ready to pay it. Not to mention that it would cause changes in many other libs/apps... So I think it should be a subject for a separate discussion. But, agree it would be good at least to measure the performance impact of such change. If I'll have some spare cycles, will give it a try. Meanwhile, can I ask Jerin and other guys to repeat tests from this RFC on their HW? Before continuing discussion would probably be good to know does the suggested patch work as expected across different platforms. I tested on an arm64 HW. The former section is without the patch(20.02) and later one with this patch. I agree with Konstantin that getting more platform tests will be good early so that we can focus on the approach to avoid back and forth latter. RTE>>ring_perf_autotest // without path ### Testing single element enq/deq ### legacy APIs: SP/SC: single: 289.78 legacy APIs: MP/MC: single: 516.20 ### Testing burst enq/deq ### legacy APIs: SP/SC: burst (size: 8): 312.88 legacy APIs: SP/SC: burst (size: 32): 426.72 legacy APIs: MP/MC: burst (size: 8): 510.95 legacy APIs: MP/MC: burst (size: 32): 702.01 ### Testing bulk enq/deq ### legacy APIs: SP/SC: bulk (size: 8): 306.74 legacy APIs: SP/SC: bulk (size: 32): 411.56 legacy APIs: MP/MC: bulk (size: 8): 501.32 legacy APIs: MP/MC: bulk (size: 32): 693.07 ### Testing empty bulk deq ### legacy APIs: SP/SC: bulk (size: 8): 7.00 legacy APIs: MP/MC: bulk (size: 8): 7.00 ### Testing using two physical cores ### legacy APIs: SP/SC: bulk (size: 8): 74.36 legacy APIs: MP/MC: bulk (size: 8): 110.18 legacy APIs: SP/SC: bulk (size: 32): 23.04 legacy APIs: MP/MC: bulk (size: 32): 32.29 ### Testing using all slave nodes ## Bulk enq/dequeue count on size 8 Core [8] count = 293741 Core [9] count = 293741 Total count (size: 8): 587482 Bulk enq/dequeue count on size 32 Core [8] count = 244909 Core [9] count = 244909 Total count (size: 32): 1077300 ### Testing single element enq/deq ### elem APIs: element size 16B: SP/SC: single: 255.37 elem APIs: element size 16B: MP/MC: single: 456.68 ### Testing burst enq/deq ### elem APIs: element size 16B: SP/SC: burst (size: 8): 291.99 elem APIs: element size 16B: SP/SC: burst (size: 32): 456.25 elem APIs: element size 16B: MP/MC: burst (size: 8): 497.77 elem APIs: element size 16B: MP/MC: burst (size: 32): 680.87 ### Testing bulk enq/deq ### elem APIs: element size 16B: SP/SC: bulk (size: 8): 284.40 elem APIs: element size 16B: SP/SC: bulk (size: 32): 453.17 elem APIs: element size 16B: MP/MC: bulk (size: 8): 485.77 elem APIs: element size 16B: MP/MC: bulk (size: 32): 675.08 ### Testing empty bulk deq ### elem APIs: element size 16B: SP/SC: bulk (size: 8): 8.00 elem APIs: element size 16B: MP/MC: bulk (size: 8): 7.00 ### Testing using two physical cores ### elem APIs: element size 16B: SP/SC: bulk (size: 8): 74.45 elem APIs: element size 16B: MP/MC: bulk (size: 8): 105.91 elem APIs: element size 16B: SP/SC: bulk (size: 32): 22.92 elem APIs: element size 16B: MP/MC: bulk (size: 32): 31.55 ### Testing usin
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
On Tue, Feb 25, 2020 at 7:11 PM Ananyev, Konstantin wrote: > We do have a run-time check in our current enqueue()/dequeue implementation. > In fact we support both modes: we have generic > rte_ring_enqueue(/dequeue)_bulk(/burst) > where sync behaviour is determined at runtime by value of prod(/cons).single. > Or user can call rte_ring_(mp/sp)_enqueue_* functions directly. > This RFC follows exactly the same paradigm: > rte_ring_enqueue(/dequeue)_bulk(/burst) kept generic and it's > behaviour is determined at runtime, by value of prod(/cons).sync_type. > Or user can call enqueue/dequeue with particular sync mode directly: > rte_ring_(mp/sp/rts/hts)_enqueue_(bulk/burst)*. > The only thing that changed: > Format of prod/cons now could differ depending on mode selected at _init_. > So you can't create a ring for let say SP mode and then in the middle of > data-path > change your mind and start using MP_RTS mode. > For existing modes (SP/MP, SC/MC) format remains the same and user can still > use them interchangeably, though of course that is an error prone practice. Makes sense. > > > > But I agree with the problem statement that in the virtualization use > > > case, It may be possible to have N virtual cores runs on a physical > > > core. > > > > > > IMO, The best solution would be keeping the ring API same and have a > > > different flavor in "compile-time". Something like > > > liburcu did for accommodating different flavors. > > > > > > i.e urcu-qsbr.h and urcu-bp.h will identical definition of API. The > > > application can simply include ONE header file in a C file based on > > > the flavor. > > I don't think it is a flexible enough approach. > In one app user might need to have several rings with different sync modes. > Or even user might need a ring with different sync modes for enqueue/dequeue. Ack. > Yes, hiding rte_ring implementation inside .c would help a lot > in terms of ABI maintenance and would make our future life easier. > The question is what is the price for it in terms of performance, > and are we ready to pay it. Not to mention that it would cause > changes in many other libs/apps... > So I think it should be a subject for a separate discussion. > But, agree it would be good at least to measure the performance > impact of such change. > If I'll have some spare cycles, will give it a try. > Meanwhile, can I ask Jerin and other guys to repeat tests from this RFC > on their HW? Before continuing discussion would probably be good to know > does the suggested patch work as expected across different platforms. I tested on an arm64 HW. The former section is without the patch(20.02) and later one with this patch. I agree with Konstantin that getting more platform tests will be good early so that we can focus on the approach to avoid back and forth latter. RTE>>ring_perf_autotest // without path ### Testing single element enq/deq ### legacy APIs: SP/SC: single: 289.78 legacy APIs: MP/MC: single: 516.20 ### Testing burst enq/deq ### legacy APIs: SP/SC: burst (size: 8): 312.88 legacy APIs: SP/SC: burst (size: 32): 426.72 legacy APIs: MP/MC: burst (size: 8): 510.95 legacy APIs: MP/MC: burst (size: 32): 702.01 ### Testing bulk enq/deq ### legacy APIs: SP/SC: bulk (size: 8): 306.74 legacy APIs: SP/SC: bulk (size: 32): 411.56 legacy APIs: MP/MC: bulk (size: 8): 501.32 legacy APIs: MP/MC: bulk (size: 32): 693.07 ### Testing empty bulk deq ### legacy APIs: SP/SC: bulk (size: 8): 7.00 legacy APIs: MP/MC: bulk (size: 8): 7.00 ### Testing using two physical cores ### legacy APIs: SP/SC: bulk (size: 8): 74.36 legacy APIs: MP/MC: bulk (size: 8): 110.18 legacy APIs: SP/SC: bulk (size: 32): 23.04 legacy APIs: MP/MC: bulk (size: 32): 32.29 ### Testing using all slave nodes ## Bulk enq/dequeue count on size 8 Core [8] count = 293741 Core [9] count = 293741 Total count (size: 8): 587482 Bulk enq/dequeue count on size 32 Core [8] count = 244909 Core [9] count = 244909 Total count (size: 32): 1077300 ### Testing single element enq/deq ### elem APIs: element size 16B: SP/SC: single: 255.37 elem APIs: element size 16B: MP/MC: single: 456.68 ### Testing burst enq/deq ### elem APIs: element size 16B: SP/SC: burst (size: 8): 291.99 elem APIs: element size 16B: SP/SC: burst (size: 32): 456.25 elem APIs: element size 16B: MP/MC: burst (size: 8): 497.77 elem APIs: element size 16B: MP/MC: burst (size: 32): 680.87 ### Testing bulk enq/deq ### elem APIs: element size 16B: SP/SC: bulk (size: 8): 284.40 elem APIs: element size 16B: SP/SC: bulk (size: 32): 453.17 elem APIs: element size 16B: MP/MC: bulk (size: 8): 485.77 elem APIs: element size 16B: MP/MC: bulk (size: 32): 675.08 ### Testing empty bulk deq ### elem APIs: element size 16B: SP/SC: bulk (size: 8): 8.00 elem APIs: element size 16B: MP/MC: bulk (size: 8): 7.00 ### Testing using two physical cores ### elem APIs: element size 16B: SP/SC: bulk (size: 8): 74.45 elem APIs: element size 16B: MP/MC: bulk (size: 8): 105.91 elem APIs: element
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ananyev, > Konstantin > > > > > > More and more customers use(/try to use) DPDK based apps within > > > > > overcommitted systems (multiple acttive threads over same > pysical cores): > > > > > VM, container deployments, etc. > > > > > That makes current rte_ring implementation to perform > > > > > really pure on some overcommited scenarios. > > > > > > > > Rather than reform rte_ring to fit this scenario, it would make > > > > more sense to me to introduce another primitive. > > I don't see much advantages it will bring us. > As a disadvantages, for developers and maintainers - code duplication, > for end users - extra code churn and removed ability to mix and match > different sync modes in one ring. > I strongly agree with Konstantin on this. Please consider this discussion at a higher abstraction level: As DPDK applications grow in number and popularity, people will deploy them in overcommitted systems. In this scenario, I consider it extremely unlikely that the user is able to dedicate N physical cores to N specific lcores of the M total lcores of the DPDK application. In the typical hypervisor scenario, the user will be able to assign a number of virtual CPUs to the virtual machine running the DPDK application, and these vCPUs will either be dedicated to the virtual machine or shared with other virtual machines. DPDK is currently designed for dedicated CPUs only. If a user runs it on shared vCPUs, the user is violating an important DPDK precondition, and DPDK's behavior is undefined! Adding the ability to run DPDK applications on shared vCPUs would be a great improvement. I prefer that support for this is as painless as possible for the DPDK application developer. Perhaps run-time detection during the EAL initialization could be a solution. The EAL would then configure relevant libraries to use the appropriate synchronization primitives. Med venlig hilsen / kind regards - Morten Brørup
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> > > > Upfront note - that RFC is not a complete patch. > > > > It introduces an ABI breakage, plus it doesn't update ring_elem code > > > > properly, etc. > > > > I plan to deal with all these things in later versions. > > > > Right now I seek an initial feedback about proposed ideas. > > > > Would also ask people to repeat performance tests (see below) on > > > > their platforms to confirm the impact. > > > > > > > > More and more customers use(/try to use) DPDK based apps within > > > > overcommitted systems (multiple acttive threads over same pysical > > > > cores): > > > > VM, container deployments, etc. > > > > One quite common problem they hit: Lock-Holder-Preemption with > > rte_ring. > > > > LHP is quite a common problem for spin-based sync primitives > > > > (spin-locks, etc.) on overcommitted systems. > > > > The situation gets much worse when some sort of fair-locking > > > > technique is used (ticket-lock, etc.). > > > > As now not only lock-owner but also lock-waiters scheduling order > > > > matters a lot. > > > > This is a well-known problem for kernel within VMs: > > > > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf > > > > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf > These slides seem to indicate that the problems are mitigated through the > Hypervisor configuration. Do we still need to address the issues? I am not really an expert here, but AFAIK current mitigations deal mostly with guest kernel: linux implements PV version of spinlocks (unfair and/or based on hypercall availability), hypervisor might make decision itself based on is guest in user/kernel mode, plus on some special cpu instructions. We do spin in user-space mode. Might be hypervisors became smarter these days, but so far, I heard about few different customers that hit such problem. As an example, NA DPDK summit presentation: https://dpdkna2019.sched.com/event/WYBG/dpdk-containers-challenges-solutions-wang-yong-zte page 16 (problem #4) describes same issue. > > > > > The problem with rte_ring is that while head accusion is sort of > > > > un-fair locking, waiting on tail is very similar to ticket lock > > > > schema - tail has to be updated in particular order. > > > > That makes current rte_ring implementation to perform really pure on > > > > some overcommited scenarios. > > > > > > Rather than reform rte_ring to fit this scenario, it would make more > > > sense to me to introduce another primitive. The current lockless ring > > > performs very well for the isolated thread model that DPDK was built > > > around. This looks like a case of customers violating the usage model > > > of the DPDK and then being surprised at the fallout. > > > > I agree with Stephen here. > > > > I think, adding more runtime check in the enqueue() and dequeue() will have > > a > > bad effect on the low-end cores too. > > But I agree with the problem statement that in the virtualization use case, > > It > > may be possible to have N virtual cores runs on a physical core. > It is hard to imagine that there are data plane applications deployed in such > environments. Wouldn't this affect the performance terribly? It wouldn't reach same performance as isolated threads, but for some tasks it might be enough. AFAIK, one quite common scenario - few isolated threads(/processes) doing actual IO and then spread packets over dozens(/hundreds) non-isolated consumers. > > > > > IMO, The best solution would be keeping the ring API same and have a > > different flavor in "compile-time". Something like liburcu did for > > accommodating different flavors. > > > > i.e urcu-qsbr.h and urcu-bp.h will identical definition of API. The > > application > > can simply include ONE header file in a C file based on the flavor. > > If need both at runtime. Need to have function pointer or so in the > > application > > and define the function in different c file by including the approaite > > flavor in C > > file. > > > > #include /* QSBR RCU flavor */ #include /* > > Bulletproof RCU flavor */
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> > > > Upfront note - that RFC is not a complete patch. > > > > It introduces an ABI breakage, plus it doesn't update ring_elem > > > > code properly, etc. > > > > I plan to deal with all these things in later versions. > > > > Right now I seek an initial feedback about proposed ideas. > > > > Would also ask people to repeat performance tests (see below) > > > > on their platforms to confirm the impact. > > > > > > > > More and more customers use(/try to use) DPDK based apps within > > > > overcommitted systems (multiple acttive threads over same pysical > > > > cores): > > > > VM, container deployments, etc. > > > > One quite common problem they hit: Lock-Holder-Preemption with rte_ring. > > > > LHP is quite a common problem for spin-based sync primitives > > > > (spin-locks, etc.) on overcommitted systems. > > > > The situation gets much worse when some sort of > > > > fair-locking technique is used (ticket-lock, etc.). > > > > As now not only lock-owner but also lock-waiters scheduling > > > > order matters a lot. > > > > This is a well-known problem for kernel within VMs: > > > > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf > > > > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf > > > > The problem with rte_ring is that while head accusion is sort of > > > > un-fair locking, waiting on tail is very similar to ticket lock schema - > > > > tail has to be updated in particular order. > > > > That makes current rte_ring implementation to perform > > > > really pure on some overcommited scenarios. > > > > > > Rather than reform rte_ring to fit this scenario, it would make > > > more sense to me to introduce another primitive. I don't see much advantages it will bring us. As a disadvantages, for developers and maintainers - code duplication, for end users - extra code churn and removed ability to mix and match different sync modes in one ring. > The current lockless > > > ring performs very well for the isolated thread model that DPDK > > > was built around. This looks like a case of customers violating > > > the usage model of the DPDK and then being surprised at the fallout. For customers using isolated thread model - nothing should change (both in terms of API and performance). Existing sync modes MP/MC,SP/SC kept untouched, set up in the same way (via flags and _init_), and MP/MC remains as default one. >From other side I don't see why we should ignore customers that want to use their DPDK apps in different deployment scenarios. > > > > I agree with Stephen here. > > > > I think, adding more runtime check in the enqueue() and dequeue() will > > have a bad effect on the low-end cores too. We do have a run-time check in our current enqueue()/dequeue implementation. In fact we support both modes: we have generic rte_ring_enqueue(/dequeue)_bulk(/burst) where sync behaviour is determined at runtime by value of prod(/cons).single. Or user can call rte_ring_(mp/sp)_enqueue_* functions directly. This RFC follows exactly the same paradigm: rte_ring_enqueue(/dequeue)_bulk(/burst) kept generic and it's behaviour is determined at runtime, by value of prod(/cons).sync_type. Or user can call enqueue/dequeue with particular sync mode directly: rte_ring_(mp/sp/rts/hts)_enqueue_(bulk/burst)*. The only thing that changed: Format of prod/cons now could differ depending on mode selected at _init_. So you can't create a ring for let say SP mode and then in the middle of data-path change your mind and start using MP_RTS mode. For existing modes (SP/MP, SC/MC) format remains the same and user can still use them interchangeably, though of course that is an error prone practice. > > But I agree with the problem statement that in the virtualization use > > case, It may be possible to have N virtual cores runs on a physical > > core. > > > > IMO, The best solution would be keeping the ring API same and have a > > different flavor in "compile-time". Something like > > liburcu did for accommodating different flavors. > > > > i.e urcu-qsbr.h and urcu-bp.h will identical definition of API. The > > application can simply include ONE header file in a C file based on > > the flavor. I don't think it is a flexible enough approach. In one app user might need to have several rings with different sync modes. Or even user might need a ring with different sync modes for enqueue/dequeue. > > If need both at runtime. Need to have function pointer or so in the > > application and define the function in different c file by including > > the approaite flavor in C file. Big issue with function pointers here would be DPDK MP model. AFAIK, rte_ring is quite popular mechanism for IPC between DPDK apps. To support such model, we'll need to split rte_ring data into 'shared' and 'private' and initialize private one for every process that is going to use it. That sounds like a massive change, and I am not sure the required effort will worth it. BTW, if user just calls API functions without
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> > -Original Message- > > From: dev On Behalf Of Stephen Hemminger > > Sent: Monday, February 24, 2020 1:35 PM > > To: Jerin Jacob > > Cc: Konstantin Ananyev ; dpdk-dev > > ; Olivier Matz > > Subject: Re: [dpdk-dev] [RFC 0/6] New sync modes for ring > > > > On Mon, 24 Feb 2020 23:29:57 +0530 > > Jerin Jacob wrote: > > > > > On Mon, Feb 24, 2020 at 10:29 PM Stephen Hemminger > > > wrote: > > > > > > > > On Mon, 24 Feb 2020 11:35:09 + > > > > Konstantin Ananyev wrote: > > > > > > > > > Upfront note - that RFC is not a complete patch. > > > > > It introduces an ABI breakage, plus it doesn't update ring_elem > > > > > code properly, etc. > > > > > I plan to deal with all these things in later versions. > > > > > Right now I seek an initial feedback about proposed ideas. > > > > > Would also ask people to repeat performance tests (see below) on > > > > > their platforms to confirm the impact. > > > > > > > > > > More and more customers use(/try to use) DPDK based apps within > > > > > overcommitted systems (multiple acttive threads over same pysical > > cores): > > > > > VM, container deployments, etc. > > > > > One quite common problem they hit: Lock-Holder-Preemption with > > rte_ring. > > > > > LHP is quite a common problem for spin-based sync primitives > > > > > (spin-locks, etc.) on overcommitted systems. > > > > > The situation gets much worse when some sort of fair-locking > > > > > technique is used (ticket-lock, etc.). > > > > > As now not only lock-owner but also lock-waiters scheduling order > > > > > matters a lot. > > > > > This is a well-known problem for kernel within VMs: > > > > > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf > > > > > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf > > > > > The problem with rte_ring is that while head accusion is sort of > > > > > un-fair locking, waiting on tail is very similar to ticket lock > > > > > schema - tail has to be updated in particular order. > > > > > That makes current rte_ring implementation to perform really pure > > > > > on some overcommited scenarios. > > > > > > > > Rather than reform rte_ring to fit this scenario, it would make more > > > > sense to me to introduce another primitive. The current lockless > > > > ring performs very well for the isolated thread model that DPDK was > > > > built around. This looks like a case of customers violating the > > > > usage model of the DPDK and then being surprised at the fallout. > > > > > > I agree with Stephen here. > > > > > > I think, adding more runtime check in the enqueue() and dequeue() will > > > have a bad effect on the low-end cores too. > > > But I agree with the problem statement that in the virtualization use > > > case, It may be possible to have N virtual cores runs on a physical > > > core. > > > > > > IMO, The best solution would be keeping the ring API same and have a > > > different flavor in "compile-time". Something like liburcu did for > > > accommodating different flavors. > > > > > > i.e urcu-qsbr.h and urcu-bp.h will identical definition of API. The > > > application can simply include ONE header file in a C file based on > > > the flavor. > > > If need both at runtime. Need to have function pointer or so in the > > > application and define the function in different c file by including > > > the approaite flavor in C file. > > > > This would also be a good time to consider the tradeoffs of the heavy use of > > inlining that is done in rte_ring vs the impact that has on API/ABI > > stability. > > > I was working on few requirements in rte_ring library for RCU defer APIs. RFC > is at https://patchwork.dpdk.org/cover/66020/. Yep, noticed your patch, seems we sort of collided here. As I understand you patch aims to provide functionality similar to my HTS one. Will try to look at yours one in next fee days, hopefully we can end-up with some common denominator. Konstantin
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> > On Mon, Feb 24, 2020 at 10:29 PM Stephen Hemminger > wrote: > > > > On Mon, 24 Feb 2020 11:35:09 + > > Konstantin Ananyev wrote: > > > > > Upfront note - that RFC is not a complete patch. > > > It introduces an ABI breakage, plus it doesn't update ring_elem code > > > properly, etc. > > > I plan to deal with all these things in later versions. > > > Right now I seek an initial feedback about proposed ideas. > > > Would also ask people to repeat performance tests (see below) on > > > their platforms to confirm the impact. > > > > > > More and more customers use(/try to use) DPDK based apps within > > > overcommitted systems (multiple acttive threads over same pysical cores): > > > VM, container deployments, etc. > > > One quite common problem they hit: Lock-Holder-Preemption with > rte_ring. > > > LHP is quite a common problem for spin-based sync primitives > > > (spin-locks, etc.) on overcommitted systems. > > > The situation gets much worse when some sort of fair-locking > > > technique is used (ticket-lock, etc.). > > > As now not only lock-owner but also lock-waiters scheduling order > > > matters a lot. > > > This is a well-known problem for kernel within VMs: > > > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf > > > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf These slides seem to indicate that the problems are mitigated through the Hypervisor configuration. Do we still need to address the issues? > > > The problem with rte_ring is that while head accusion is sort of > > > un-fair locking, waiting on tail is very similar to ticket lock > > > schema - tail has to be updated in particular order. > > > That makes current rte_ring implementation to perform really pure on > > > some overcommited scenarios. > > > > Rather than reform rte_ring to fit this scenario, it would make more > > sense to me to introduce another primitive. The current lockless ring > > performs very well for the isolated thread model that DPDK was built > > around. This looks like a case of customers violating the usage model > > of the DPDK and then being surprised at the fallout. > > I agree with Stephen here. > > I think, adding more runtime check in the enqueue() and dequeue() will have a > bad effect on the low-end cores too. > But I agree with the problem statement that in the virtualization use case, It > may be possible to have N virtual cores runs on a physical core. It is hard to imagine that there are data plane applications deployed in such environments. Wouldn't this affect the performance terribly? > > IMO, The best solution would be keeping the ring API same and have a > different flavor in "compile-time". Something like liburcu did for > accommodating different flavors. > > i.e urcu-qsbr.h and urcu-bp.h will identical definition of API. The > application > can simply include ONE header file in a C file based on the flavor. > If need both at runtime. Need to have function pointer or so in the > application > and define the function in different c file by including the approaite flavor > in C > file. > > #include /* QSBR RCU flavor */ #include /* > Bulletproof RCU flavor */ > > > > > > > > > > > > > > >
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
> -Original Message- > From: dev On Behalf Of Stephen Hemminger > Sent: Monday, February 24, 2020 1:35 PM > To: Jerin Jacob > Cc: Konstantin Ananyev ; dpdk-dev > ; Olivier Matz > Subject: Re: [dpdk-dev] [RFC 0/6] New sync modes for ring > > On Mon, 24 Feb 2020 23:29:57 +0530 > Jerin Jacob wrote: > > > On Mon, Feb 24, 2020 at 10:29 PM Stephen Hemminger > > wrote: > > > > > > On Mon, 24 Feb 2020 11:35:09 + > > > Konstantin Ananyev wrote: > > > > > > > Upfront note - that RFC is not a complete patch. > > > > It introduces an ABI breakage, plus it doesn't update ring_elem > > > > code properly, etc. > > > > I plan to deal with all these things in later versions. > > > > Right now I seek an initial feedback about proposed ideas. > > > > Would also ask people to repeat performance tests (see below) on > > > > their platforms to confirm the impact. > > > > > > > > More and more customers use(/try to use) DPDK based apps within > > > > overcommitted systems (multiple acttive threads over same pysical > cores): > > > > VM, container deployments, etc. > > > > One quite common problem they hit: Lock-Holder-Preemption with > rte_ring. > > > > LHP is quite a common problem for spin-based sync primitives > > > > (spin-locks, etc.) on overcommitted systems. > > > > The situation gets much worse when some sort of fair-locking > > > > technique is used (ticket-lock, etc.). > > > > As now not only lock-owner but also lock-waiters scheduling order > > > > matters a lot. > > > > This is a well-known problem for kernel within VMs: > > > > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf > > > > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf > > > > The problem with rte_ring is that while head accusion is sort of > > > > un-fair locking, waiting on tail is very similar to ticket lock > > > > schema - tail has to be updated in particular order. > > > > That makes current rte_ring implementation to perform really pure > > > > on some overcommited scenarios. > > > > > > Rather than reform rte_ring to fit this scenario, it would make more > > > sense to me to introduce another primitive. The current lockless > > > ring performs very well for the isolated thread model that DPDK was > > > built around. This looks like a case of customers violating the > > > usage model of the DPDK and then being surprised at the fallout. > > > > I agree with Stephen here. > > > > I think, adding more runtime check in the enqueue() and dequeue() will > > have a bad effect on the low-end cores too. > > But I agree with the problem statement that in the virtualization use > > case, It may be possible to have N virtual cores runs on a physical > > core. > > > > IMO, The best solution would be keeping the ring API same and have a > > different flavor in "compile-time". Something like liburcu did for > > accommodating different flavors. > > > > i.e urcu-qsbr.h and urcu-bp.h will identical definition of API. The > > application can simply include ONE header file in a C file based on > > the flavor. > > If need both at runtime. Need to have function pointer or so in the > > application and define the function in different c file by including > > the approaite flavor in C file. > > This would also be a good time to consider the tradeoffs of the heavy use of > inlining that is done in rte_ring vs the impact that has on API/ABI stability. > I was working on few requirements in rte_ring library for RCU defer APIs. RFC is at https://patchwork.dpdk.org/cover/66020/.
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
On Mon, 24 Feb 2020 23:29:57 +0530 Jerin Jacob wrote: > On Mon, Feb 24, 2020 at 10:29 PM Stephen Hemminger > wrote: > > > > On Mon, 24 Feb 2020 11:35:09 + > > Konstantin Ananyev wrote: > > > > > Upfront note - that RFC is not a complete patch. > > > It introduces an ABI breakage, plus it doesn't update ring_elem > > > code properly, etc. > > > I plan to deal with all these things in later versions. > > > Right now I seek an initial feedback about proposed ideas. > > > Would also ask people to repeat performance tests (see below) > > > on their platforms to confirm the impact. > > > > > > More and more customers use(/try to use) DPDK based apps within > > > overcommitted systems (multiple acttive threads over same pysical cores): > > > VM, container deployments, etc. > > > One quite common problem they hit: Lock-Holder-Preemption with rte_ring. > > > LHP is quite a common problem for spin-based sync primitives > > > (spin-locks, etc.) on overcommitted systems. > > > The situation gets much worse when some sort of > > > fair-locking technique is used (ticket-lock, etc.). > > > As now not only lock-owner but also lock-waiters scheduling > > > order matters a lot. > > > This is a well-known problem for kernel within VMs: > > > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf > > > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf > > > The problem with rte_ring is that while head accusion is sort of > > > un-fair locking, waiting on tail is very similar to ticket lock schema - > > > tail has to be updated in particular order. > > > That makes current rte_ring implementation to perform > > > really pure on some overcommited scenarios. > > > > Rather than reform rte_ring to fit this scenario, it would make > > more sense to me to introduce another primitive. The current lockless > > ring performs very well for the isolated thread model that DPDK > > was built around. This looks like a case of customers violating > > the usage model of the DPDK and then being surprised at the fallout. > > I agree with Stephen here. > > I think, adding more runtime check in the enqueue() and dequeue() will > have a bad effect on the low-end cores too. > But I agree with the problem statement that in the virtualization use > case, It may be possible to have N virtual cores runs on a physical > core. > > IMO, The best solution would be keeping the ring API same and have a > different flavor in "compile-time". Something like > liburcu did for accommodating different flavors. > > i.e urcu-qsbr.h and urcu-bp.h will identical definition of API. The > application can simply include ONE header file in a C file based on > the flavor. > If need both at runtime. Need to have function pointer or so in the > application and define the function in different c file by including > the approaite flavor in C file. This would also be a good time to consider the tradeoffs of the heavy use of inlining that is done in rte_ring vs the impact that has on API/ABI stability.
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
On Mon, Feb 24, 2020 at 10:29 PM Stephen Hemminger wrote: > > On Mon, 24 Feb 2020 11:35:09 + > Konstantin Ananyev wrote: > > > Upfront note - that RFC is not a complete patch. > > It introduces an ABI breakage, plus it doesn't update ring_elem > > code properly, etc. > > I plan to deal with all these things in later versions. > > Right now I seek an initial feedback about proposed ideas. > > Would also ask people to repeat performance tests (see below) > > on their platforms to confirm the impact. > > > > More and more customers use(/try to use) DPDK based apps within > > overcommitted systems (multiple acttive threads over same pysical cores): > > VM, container deployments, etc. > > One quite common problem they hit: Lock-Holder-Preemption with rte_ring. > > LHP is quite a common problem for spin-based sync primitives > > (spin-locks, etc.) on overcommitted systems. > > The situation gets much worse when some sort of > > fair-locking technique is used (ticket-lock, etc.). > > As now not only lock-owner but also lock-waiters scheduling > > order matters a lot. > > This is a well-known problem for kernel within VMs: > > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf > > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf > > The problem with rte_ring is that while head accusion is sort of > > un-fair locking, waiting on tail is very similar to ticket lock schema - > > tail has to be updated in particular order. > > That makes current rte_ring implementation to perform > > really pure on some overcommited scenarios. > > Rather than reform rte_ring to fit this scenario, it would make > more sense to me to introduce another primitive. The current lockless > ring performs very well for the isolated thread model that DPDK > was built around. This looks like a case of customers violating > the usage model of the DPDK and then being surprised at the fallout. I agree with Stephen here. I think, adding more runtime check in the enqueue() and dequeue() will have a bad effect on the low-end cores too. But I agree with the problem statement that in the virtualization use case, It may be possible to have N virtual cores runs on a physical core. IMO, The best solution would be keeping the ring API same and have a different flavor in "compile-time". Something like liburcu did for accommodating different flavors. i.e urcu-qsbr.h and urcu-bp.h will identical definition of API. The application can simply include ONE header file in a C file based on the flavor. If need both at runtime. Need to have function pointer or so in the application and define the function in different c file by including the approaite flavor in C file. #include /* QSBR RCU flavor */ #include /* Bulletproof RCU flavor */ >
Re: [dpdk-dev] [RFC 0/6] New sync modes for ring
On Mon, 24 Feb 2020 11:35:09 + Konstantin Ananyev wrote: > Upfront note - that RFC is not a complete patch. > It introduces an ABI breakage, plus it doesn't update ring_elem > code properly, etc. > I plan to deal with all these things in later versions. > Right now I seek an initial feedback about proposed ideas. > Would also ask people to repeat performance tests (see below) > on their platforms to confirm the impact. > > More and more customers use(/try to use) DPDK based apps within > overcommitted systems (multiple acttive threads over same pysical cores): > VM, container deployments, etc. > One quite common problem they hit: Lock-Holder-Preemption with rte_ring. > LHP is quite a common problem for spin-based sync primitives > (spin-locks, etc.) on overcommitted systems. > The situation gets much worse when some sort of > fair-locking technique is used (ticket-lock, etc.). > As now not only lock-owner but also lock-waiters scheduling > order matters a lot. > This is a well-known problem for kernel within VMs: > http://www-archive.xenproject.org/files/xensummitboston08/LHP.pdf > https://www.cs.hs-rm.de/~kaiser/events/wamos2017/Slides/selcuk.pdf > The problem with rte_ring is that while head accusion is sort of > un-fair locking, waiting on tail is very similar to ticket lock schema - > tail has to be updated in particular order. > That makes current rte_ring implementation to perform > really pure on some overcommited scenarios. Rather than reform rte_ring to fit this scenario, it would make more sense to me to introduce another primitive. The current lockless ring performs very well for the isolated thread model that DPDK was built around. This looks like a case of customers violating the usage model of the DPDK and then being surprised at the fallout.
