svn commit: r242014 - head/sys/kern

[Jeff Roberson]

On Wed, 24 Oct 2012, Attilio Rao wrote:

> On Wed, Oct 24, 2012 at 8:16 PM, Andre Oppermann <andre at freebsd.org> wrote:
>> On 24.10.2012 20:56, Jim Harris wrote:
>>>
>>> On Wed, Oct 24, 2012 at 11:41 AM, Adrian Chadd <adrian at freebsd.org> wrote:
>>>>
>>>> On 24 October 2012 11:36, Jim Harris <jimharris at 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.

I agree.  For locks with little contention we probably want smaller 
structures.  For example, you wouldn't want to put a huge lock in every 
file descriptor.  It would be nice to have an automatic way to pad every 
global lock though.  I think it should be done as needed.

Jeff

>
> Attilio
>
>
> -- 
> Peace can only be achieved by understanding - A. Einstein
>