Date: Wed, 24 Oct 2012 21:29:21 +0100 From: Attilio Rao <attilio@freebsd.org> To: Andre Oppermann <andre@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Jim Harris <jim.harris@gmail.com>, John Baldwin <jhb@freebsd.org> Subject: Re: svn commit: r242014 - head/sys/kern Message-ID: <CAJ-FndBH2cjF2ukt4373BwMsoWEeLykBhPQNnXUgpJ=4jd3LJg@mail.gmail.com> In-Reply-To: <50884E9F.3090706@freebsd.org> References: <201210241836.q9OIafqo073002@svn.freebsd.org> <201210241443.25988.jhb@freebsd.org> <CAJP=Hc9wLv02sX%2BWnzZtaKccSAFzqg8jT0oP13nLw1jMfwOEBQ@mail.gmail.com> <CAJ-FndDzBdq8q6J7QKqf=abi_702s_ia=pa3XbBv80rxbGb-SA@mail.gmail.com> <50884E9F.3090706@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Oct 24, 2012 at 9:25 PM, Andre Oppermann <andre@freebsd.org> wrote: > On 24.10.2012 21:06, Attilio Rao wrote: >> >> On Wed, Oct 24, 2012 at 8:00 PM, Jim Harris <jim.harris@gmail.com> wrote: >>> >>> On Wed, Oct 24, 2012 at 11:43 AM, John Baldwin <jhb@freebsd.org> wrote: >>>> >>>> On Wednesday, October 24, 2012 2:36:41 pm Jim Harris wrote: >>>>> >>>>> Author: jimharris >>>>> Date: Wed Oct 24 18:36:41 2012 >>>>> New Revision: 242014 >>>>> URL: http://svn.freebsd.org/changeset/base/242014 >>>>> >>>>> Log: >>>>> Pad tdq_lock to avoid false sharing with tdq_load and tdq_cpu_idle. >>>>> >>>>> This enables CPU searches (which read tdq_load) to operate >>>>> independently >>>>> of any contention on the spinlock. Some scheduler-intensive >>>>> workloads >>>>> running on an 8C single-socket SNB Xeon show considerable >>>>> improvement with >>>>> this change (2-3% perf improvement, 5-6% decrease in CPU util). >>>>> >>>>> Sponsored by: Intel >>>>> Reviewed by: jeff >>>>> >>>>> Modified: >>>>> head/sys/kern/sched_ule.c >>>>> >>>>> Modified: head/sys/kern/sched_ule.c >>>>> >>>> >>>> ============================================================================== >>>>> >>>>> --- head/sys/kern/sched_ule.c Wed Oct 24 18:33:44 2012 (r242013) >>>>> +++ head/sys/kern/sched_ule.c Wed Oct 24 18:36:41 2012 (r242014) >>>>> @@ -223,8 +223,13 @@ static int sched_idlespinthresh = -1; >>>>> * locking in sched_pickcpu(); >>>>> */ >>>>> struct tdq { >>>>> - /* Ordered to improve efficiency of cpu_search() and switch(). */ >>>>> + /* >>>>> + * Ordered to improve efficiency of cpu_search() and switch(). >>>>> + * tdq_lock is padded to avoid false sharing with tdq_load and >>>>> + * tdq_cpu_idle. >>>>> + */ >>>>> struct mtx tdq_lock; /* run queue lock. */ >>>>> + char pad[64 - sizeof(struct mtx)]; >>>> >>>> >>>> Can this use 'tdq_lock __aligned(CACHE_LINE_SIZE)' instead? >>>> >>> >>> No - that doesn't pad it. I believe that only works if it's global, >>> i.e. not part of a data structure. >> >> >> As I've already said in another thread __align() doesn't work on >> object declaration, so what that won't pad it either if it is global >> or part of a struct. >> It is just implemented as __attribute__((aligned(X))): >> http://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Type-Attributes.html > > > Actually it seems gcc itself doesn't really care and it up to the > linker to honor that. Yes but the concept being that if you use __aligned() properly (when defining a struct) the object will be correctly sized, so you will get padding automatically. Attilio -- 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?CAJ-FndBH2cjF2ukt4373BwMsoWEeLykBhPQNnXUgpJ=4jd3LJg>