Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Jan 2010 22:20:46 +0100
From:      Attilio Rao <attilio@freebsd.org>
To:        John Baldwin <jhb@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, xcllnt@mac.com, marcel@freebsd.org, svn-src-head@freebsd.org, "M. Warner Losh" <imp@bsdimp.com>
Subject:   Re: svn commit: r202889 - head/sys/kern
Message-ID:  <3bbf2fe11001261320k654b2b8ck3d03c4d94ae8d9c1@mail.gmail.com>
In-Reply-To: <201001261551.52206.jhb@freebsd.org>
References:  <3bbf2fe11001260058i65604619l664bd0e49c1dbbd@mail.gmail.com> <C6A8F7A7-F0A9-4F63-B61E-DDC5332DC495@mac.com> <20100126.130932.722022410132669562.imp@bsdimp.com> <201001261551.52206.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
2010/1/26 John Baldwin <jhb@freebsd.org>:
> On Tuesday 26 January 2010 3:09:32 pm M. Warner Losh wrote:
>> In message: <C6A8F7A7-F0A9-4F63-B61E-DDC5332DC495@mac.com>
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Marcel Moolenaar <xcllnt@mac.c=
om> writes:
>> : Maybe what is in order right now is a description (using pseudo
>> : code if you like) of what exactly needs to happen with the 3rd
>> : argument, when and how (i.e. what must be atomic and what does
>> : not have to be atomic).
>>
>> I believe the proper pseudo code should be:
>>
>> cpu_switch(struct thread *old, struct thread *new, struct mutext *mtx)
>> {
>> =C2=A0 =C2=A0 =C2=A0 /* Save the registers to the pcb */
>> =C2=A0 =C2=A0 =C2=A0 old->td_lock =3D mtx;
>> #if defined(SMP) && defined(SCHED_ULE)
>> =C2=A0 =C2=A0 =C2=A0 /* s/long/int/ if sizeof(long) !=3D sizeof(void *) =
*/
>> =C2=A0 =C2=A0 =C2=A0 /* as we have no 'void *' version of the atomics */
>> =C2=A0 =C2=A0 =C2=A0 while (atomic_load_acq_long(&new->td_lock) =3D=3D (=
long)&blocked_lock)
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue;
>> #endif
>> =C2=A0 =C2=A0 =C2=A0 /* Switch to new context */
>> }
>
> FYI, that is what the '_ptr' variants of atomic ops are for. =C2=A0I do t=
hink that
> the 'acq' membar on the load is needed to ensure the CPU doesn't try to
> speculatively read any of the 'new' thread's PCB until it sees the update=
 to
> td_lock.

I think it is more important to store the old lock via a rel barrier instea=
d:

- old->td_lock =3D mtx;
+ atomic_store_rel_ptr(&old->td_lock, mtx);

Thanks,
Attilio


--=20
Peace can only be achieved by understanding - A. Einstein



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe11001261320k654b2b8ck3d03c4d94ae8d9c1>