On Wed, 24 Oct 2012, Attilio Rao wrote:

On Wed, Oct 24, 2012 at 8:16 PM, Andre Oppermann <an...@freebsd.org> wrote:
On 24.10.2012 20:56, Jim Harris wrote:

On Wed, Oct 24, 2012 at 11:41 AM, Adrian Chadd <adr...@freebsd.org> wrote:

On 24 October 2012 11:36, Jim Harris <jimhar...@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

_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to