Date: Wed, 24 Oct 2012 16:41:24 +0100 From: Attilio Rao <attilio@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: mdf@freebsd.org, src-committers@freebsd.org, Andre Oppermann <andre@freebsd.org>, Bruce Evans <brde@optusnet.com.au>, svn-src-user@freebsd.org Subject: Re: svn commit: r241889 - in user/andre/tcp_workqueue/sys: arm/arm cddl/compat/opensolaris/kern cddl/contrib/opensolaris/uts/common/dtrace cddl/contrib/opensolaris/uts/common/fs/zfs ddb dev/acpica dev/... Message-ID: <CAJ-FndAG-Qp%2B1aQvoL7YRj=R151Qe9_wNrUeOAaDsdYao_-zCQ@mail.gmail.com> In-Reply-To: <201210241136.06154.jhb@freebsd.org> References: <201210221418.q9MEINkr026751@svn.freebsd.org> <CAJ-FndC=zV%2BHN1wr_CnSEY93VHT--w9cYPMhH8P53y%2BLvBSO7g@mail.gmail.com> <CAJ-FndCDeB7w30PgSwOpMbWT6e%2BR7iQHfa8PU8Pn0NLagCiJxA@mail.gmail.com> <201210241136.06154.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Oct 24, 2012 at 4:36 PM, John Baldwin <jhb@freebsd.org> wrote:
> On Wednesday, October 24, 2012 11:24:22 am Attilio Rao wrote:
>> On Wed, Oct 24, 2012 at 4:09 PM, Attilio Rao <attilio@freebsd.org> wrote:
>> > On Wed, Oct 24, 2012 at 3:45 PM, John Baldwin <jhb@freebsd.org> wrote:
>> >> On Wednesday, October 24, 2012 10:34:34 am Attilio Rao wrote:
>> >>> On Wed, Oct 24, 2012 at 3:05 PM, John Baldwin <jhb@freebsd.org> wrote:
>> >>> > On Tuesday, October 23, 2012 7:20:04 pm Andre Oppermann wrote:
>> >>> >> On 24.10.2012 00:15, mdf@FreeBSD.org wrote:
>> >>> >> > On Tue, Oct 23, 2012 at 7:41 AM, Andre Oppermann <andre@freebsd.org>
>> >> wrote:
>> >>> >> >> Struct mtx and MTX_SYSINIT always occur as pair next to each other.
>> >>> >> >
>> >>> >> > That doesn't matter. Language basics like variable definitions should
>> >>> >> > not be obscured by macros. It either takes longer to figure out what
>> >>> >> > a variable is (because one needs to look up the definition of the
>> >>> >> > macro) or makes it almost impossible (because now e.g. cscope doesn't
>> >>> >> > know this is a variable definition.
>> >>> >>
>> >>> >> Sigh, cscope doesn't expand macros?
>> >>> >>
>> >>> >> Is there a way to do the cache line alignment in a sane way without
>> >>> >> littering __aligned(CACHE_LINE_SIZE) all over the place?
>> >>> >
>> >>> > I was hoping to do something with an anonymous union or some such like:
>> >>> >
>> >>> > union mtx_aligned {
>> >>> > struct mtx;
>> >>> > char[roundup2(sizeof(struct mtx), CACHE_LINE_SIZE)];
>> >>> > }
>> >>> >
>> >>> > I don't know if there is a useful way to define an 'aligned mutex' type
>> >>> > that will transparently map to a 'struct mtx', e.g.:
>> >>> >
>> >>> > typedef struct mtx __aligned(CACHE_LINE_SIZE) aligned_mtx_t;
>> >>>
>> >>> Unfortunately that doesn't work as I've verified with alc@ few months ago.
>> >>> The __aligned() attribute only works with structures definition, not
>> >>> objects declaration.
>> >>
>> >> Are you saying that the typedef doesn't (I expect it doesn't), or that this
>> >> doesn't:
>> >>
>> >> struct mtx foo __aligned(CACHE_LINE_SIZE);
>> >
>> > I meant to say that such notation won't address the padding issue
>> > which is as import as the alignment. Infact, for sensitive locks,
>> > having just an aligned object is not really useful if the cacheline
>> > gets shared.
>> > In the end you will need to use explicit padding or use __aligned in
>> > the struct definition, which cannot be used as a general pattern.
>>
>> The quickest way I see this can be made general is to have a specific
>> struct defined in sys/_mutex.h like that
>>
>> struct mtx_unshare {
>> struct mtx lock;
>> char _pad[CACHE_LINE_SIZE - sizeof(struct mtx)];
>> } __aligned(CACHE_LINE_SIZE);
>
> I think instead you want my union above that uses roundup2 in case a lock
> eats up multiple cache lines:
Do you think locks can eat more than one cacheline? This would be
absolutely killer for performance.
> union mtx_foo {
> struct mtx lock;
> char junk[roundup2(sizeof(struct mtx), CACHE_LINE_SIZE)];
> } __aligned_CACHE_LINE_SIZE;
>
>> then let mtx_* functions to accept void ptrs and cast them to struct
>> mtx as long as the functions enter.
>
> Eh, that removes all compile time type checks. That seems very dubious to me.
Well right now fast path already has a fair amount of macros wrapping
the operations, which don't really enforce any type checks.
Another way to do this would be to forsee an helper function to
extract the struct mtx from the mtx_unshare and pass it to underlying
mtx_* functions.
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-FndAG-Qp%2B1aQvoL7YRj=R151Qe9_wNrUeOAaDsdYao_-zCQ>