Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 24 Oct 2012 20:49:50 +0100
From:      Attilio Rao <attilio@freebsd.org>
To:        Andre Oppermann <andre@freebsd.org>
Cc:        svn-src-head@freebsd.org, Adrian Chadd <adrian@freebsd.org>, src-committers@freebsd.org, Jim Harris <jim.harris@gmail.com>, svn-src-all@freebsd.org
Subject:   Re: svn commit: r242014 - head/sys/kern
Message-ID:  <CAJ-FndAJwuD=oUHyVK4xHoZNrf0r%2Bq2WxZ9rUMW%2B-zzMo_8QuA@mail.gmail.com>
In-Reply-To: <50883EA8.1010308@freebsd.org>
References:  <201210241836.q9OIafqo073002@svn.freebsd.org> <CAJ-VmonpdJ445hXVaoHqFgS0v7QRwqHWodQrVHm2CN9T661www@mail.gmail.com> <CAJP=Hc9XmvfW3MrDjvK15OAx1fyfjPk%2BEhqHUOzoEpChu5imtg@mail.gmail.com> <50883EA8.1010308@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Oct 24, 2012 at 8:16 PM, Andre Oppermann <andre@freebsd.org> wrote:
> On 24.10.2012 20:56, Jim Harris wrote:
>>
>> On Wed, Oct 24, 2012 at 11:41 AM, Adrian Chadd <adrian@freebsd.org> wrote:
>>>
>>> On 24 October 2012 11:36, Jim Harris <jimharris@freebsd.org> wrote:
>>>
>>>>    Pad tdq_lock to avoid false sharing with tdq_load and tdq_cpu_idle.
>>>
>>>
>>> Ok, but..
>>>
>>>
>>>>          struct mtx      tdq_lock;               /* run queue lock. */
>>>> +       char            pad[64 - sizeof(struct mtx)];
>>>
>>>
>>> .. don't we have an existing compile time macro for the cache line
>>> size, which can be used here?
>>
>>
>> Yes, but I didn't use it for a couple of reasons:
>>
>> 1) struct tdq itself is currently using __aligned(64), so I wanted to
>> keep it consistent.
>> 2) CACHE_LINE_SIZE is currently defined as 128 on x86, due to
>> NetBurst-based processors having 128-byte cache sectors a while back.
>> I had planned to start a separate thread on arch@ about this today on
>> whether this was still appropriate.
>
>
> See also the discussion on svn-src-all regarding global struct mtx
> alignment.
>
> Thank you for proving my point. ;)
>
> Let's go back and see how we can do this the sanest way.  These are
> the options I see at the moment:
>
>  1. sprinkle __aligned(CACHE_LINE_SIZE) all over the place

This is wrong because it doesn't give padding.

>  2. use a macro like MTX_ALIGN that can be SMP/UP aware and in
>     the future possibly change to a different compiler dependent
>     align attribute

What is this macro supposed to do? I don't understand that from your
description.

>  3. embed __aligned(CACHE_LINE_SIZE) into struct mtx itself so it
>     automatically gets aligned in all cases, even when dynamically
>     allocated.

This works but I think it is overkill for structures including sleep
mutexes which are the vast majority. So I wouldn't certainly be in
favor of such a patch.

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-FndAJwuD=oUHyVK4xHoZNrf0r%2Bq2WxZ9rUMW%2B-zzMo_8QuA>