Date: Tue, 17 May 2011 09:20:40 -0700 From: Max Laier <max@love2party.net> To: John Baldwin <jhb@FreeBSD.org> Cc: neel@freebsd.org, Andriy Gapon <avg@freebsd.org>, Attilio Rao <attilio@freebsd.org>, FreeBSD current <freebsd-current@freebsd.org>, Stephan Uphoff <ups@FreeBSD.org>, Peter Grehan <grehan@freebsd.org> Subject: Re: proposed smp_rendezvous change Message-ID: <4DD2A058.6050400@love2party.net> In-Reply-To: <4DD26720.3000001@FreeBSD.org> References: <4DCD357D.6000109@FreeBSD.org> <201105161738.53366.max@love2party.net> <BANLkTik8CqtiP9OgvBpL08dqK6Aj%2BLQ3OA@mail.gmail.com> <201105161805.51188.max@love2party.net> <4DD26720.3000001@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 05/17/2011 05:16 AM, John Baldwin wrote:
...
> Index: kern/kern_switch.c
> ===================================================================
> --- kern/kern_switch.c (revision 221536)
> +++ kern/kern_switch.c (working copy)
> @@ -192,15 +192,22 @@
> critical_exit(void)
> {
> struct thread *td;
> - int flags;
> + int flags, owepreempt;
>
> td = curthread;
> KASSERT(td->td_critnest != 0,
> ("critical_exit: td_critnest == 0"));
>
> if (td->td_critnest == 1) {
> + owepreempt = td->td_owepreempt;
> + td->td_owepreempt = 0;
> + /*
> + * XXX: Should move compiler_memory_barrier() from
> + * rmlock to a header.
> + */
XXX: If we get an interrupt at this point and td_owepreempt was zero,
the new interrupt will re-set it, because td_critnest is still non-zero.
So we still end up with a thread that is leaking an owepreempt *and*
lose a preemption.
We really need an atomic_readandclear() which gives us a local copy of
td_owepreempt *and* clears critnest in the same operation. Sadly, that
is rather expensive. It is possible to implement with a flag for
owepreempt, but that means that all writes to critnest must then be
atomic. Either because we know we have interrupts disabled (i.e.
setting owepreempt can be a RMW), or with a proper atomic_add/set/...
I'm not sure what the performance impact of this will be. One would
hope that atomic_add without a memory barrier isn't much more expensive
than a compiler generated read-modify-write, tho. Especially, since
this cacheline should be local and exclusive to us, anyway.
> + __asm __volatile("":::"memory");
> td->td_critnest = 0;
> - if (td->td_owepreempt) {
> + if (owepreempt) {
> td->td_critnest = 1;
> thread_lock(td);
> td->td_critnest--;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4DD2A058.6050400>