Agreed, that correct programming would only have the owner attempt to unlock, but what about other threads simultaneously attempting to lock while the owner is unlocking? Maybe I don't fully appreciate the memmodel semantics, but there can be arbitrary delays between any two instructions executed by a single core due to interrupts.
On Wed, Dec 3, 2014 at 2:07 PM, Ola Liljedahl <[email protected]> wrote: > 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 >
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
