On 3 December 2014 at 18:29, Mario Torrecillas Rodriguez <[email protected]> wrote: > I don¹t think cur_ticket will be read/written by more than one thread > simultaneously, only the one which owns the lock will attempt to update it. Exactly.
Do I take that as a review Mario? > > Mario. > > On 03/12/2014 17:16, "Maxim Uvarov" <[email protected]> wrote: > >>On 12/03/2014 05:20 PM, Ola Liljedahl wrote: >>> Ping! >> >>Needed more review for this. >>> >>> On 1 December 2014 at 14:34, Ola Liljedahl <[email protected]> >>>wrote: >>>> Signed-off-by: Ola Liljedahl <[email protected]> >>>> --- >>>> Replaced an atomic RMW add with separate load, add and store >>>>operations. >>>> This avoids generating a "locked" instruction on x86 which implies >>>>unnecessary >>>> strong memory ordering and improves performance. This change could >>>>also prove >>>> beneficial on other architectures. >>>> >>>> platform/linux-generic/odp_ticketlock.c | 5 ++++- >>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/platform/linux-generic/odp_ticketlock.c >>>>b/platform/linux-generic/odp_ticketlock.c >>>> index 6c5e74e..1e67ff5 100644 >>>> --- a/platform/linux-generic/odp_ticketlock.c >>>> +++ b/platform/linux-generic/odp_ticketlock.c >>>> @@ -32,7 +32,10 @@ void odp_ticketlock_lock(odp_ticketlock_t >>>>*ticketlock) >>>> >>>> void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock) >>>> { >>>> - _odp_atomic_u32_add_mm(&ticketlock->cur_ticket, 1, >>>>_ODP_MEMMODEL_RLS); >>>> + uint32_t cur = _odp_atomic_u32_load_mm(&ticketlock->cur_ticket, >>>> + _ODP_MEMMODEL_RLX); >>>> + _odp_atomic_u32_store_mm(&ticketlock->cur_ticket, cur + 1, >>>> + _ODP_MEMMODEL_RLS); >>>> >>Isn't this code racy? Some threads can read cur at the same time, then >>delay (interrupt or whatever) then do store. >>And as result store will have wrong cut_ticket value. >> >>Maxim. >> >>>> #if defined __OCTEON__ >>>> odp_sync_stores(); /* SYNCW to flush write buffer */ >>>> -- >>>> 1.9.1 >>>> >>> _______________________________________________ >>> lng-odp mailing list >>> [email protected] >>> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >>_______________________________________________ >>lng-odp mailing list >>[email protected] >>http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
