From: EXT Bill Fischofer [mailto:[email protected]] Sent: Thursday, January 07, 2016 2:11 PM To: Savolainen, Petri (Nokia - FI/Espoo) Cc: LNG ODP Mailman List Subject: Re: [lng-odp] [API-NEXT PATCH 5/5] linux-generic: removed spin_internal
On Thu, Jan 7, 2016 at 2:50 AM, Savolainen, Petri (Nokia - FI/Espoo) <[email protected]<mailto:[email protected]>> wrote: From: lng-odp [mailto:[email protected]<mailto:[email protected]>] On Behalf Of EXT Savolainen, Petri (Nokia - FI/Espoo) Sent: Thursday, January 07, 2016 10:44 AM To: EXT Bill Fischofer Cc: LNG ODP Mailman List Subject: Re: [lng-odp] [API-NEXT PATCH 5/5] linux-generic: removed spin_internal From: EXT Bill Fischofer [mailto:[email protected]] Sent: Tuesday, January 05, 2016 8:01 PM To: Savolainen, Petri (Nokia - FI/Espoo) Cc: LNG ODP Mailman List Subject: Re: [lng-odp] [API-NEXT PATCH 5/5] linux-generic: removed spin_internal diff --git a/platform/linux-generic/odp_barrier.c b/platform/linux-generic/odp_barrier.c index 53d83c0..0bfc0f0 100644 --- a/platform/linux-generic/odp_barrier.c +++ b/platform/linux-generic/odp_barrier.c @@ -6,7 +6,7 @@ #include <odp/barrier.h> #include <odp/sync.h> -#include <odp_spin_internal.h> +#include <odp/cpu.h> #include <odp_atomic_internal.h> void odp_barrier_init(odp_barrier_t *barrier, int count) @@ -43,7 +43,7 @@ void odp_barrier_wait(odp_barrier_t *barrier) } else { while ((odp_atomic_load_u32(&barrier->bar) < barrier->count) == wasless) - odp_spin(); + odp_cpu_pause(); Why wouldn't you just want to change the implementation of odp_spin() to use the pause instruction here? These are spin lock waits but nowhere does ODP specify that spins must be implemented inefficiently. Since odp_cpu_pause() is intended for exactly this use case, it’s better to use the call also internally to highlight the correct usage (and eat our own dog food). The implementation is arch specific (“pause” vs. “nop” vs. empty). For example, on multi-core x86 “pause” improves lock performance since CPUs are not stressing the interconnect at 100% speed. Pause instruction was designed for this use case. -Petri Also odp_cpu_pause() is implemented as static inline function - so there’s no function call, it will be the “pause” instruction directly. I agree completely with the implementation change for all the reasons you cite, just questioning why it needs to be reflected in an API name change that implies a specific implementation. What if SoC X doesn't have a pause instruction but instead uses a delay instruction or some other technique for efficient waiting? odp_spin() is as good a "neutral" name as any to say "I don't have anything to do here other than delay for a bit". We expect each ODP API to be implemented as efficiently as possible, and on X86 that means using the native pause instruction, so that's just how odp_spin() would be implemented here. It would also be an inline function on this architecture. odp_spin() was an internal function, not an API. Odp_cpu_pause() is a new API call. It does not do any spinning but pauses the cpu for a very short while. Actual spin logic calls the pause. It’s in the API so that when application implements its own lock-free, etc algorithms it can also pause within its spin loop. -Petri -Petri } _ODP_FULL_BARRIER();
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
