Date: Fri, 26 Oct 2012 07:27:00 +0100 From: Attilio Rao <attilio@freebsd.org> To: Andre Oppermann <andre@freebsd.org> Cc: mdf@freebsd.org, src-committers@freebsd.org, John Baldwin <jhb@freebsd.org>, svn-src-user@freebsd.org, Jeff Roberson <jroberson@jroberson.net>, Bruce Evans <brde@optusnet.com.au> 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-FndApwOo8xmZQyeqq0bGp8P13QWRqgmSDNg1_hbm7nrpOAQ@mail.gmail.com> In-Reply-To: <5089A913.2040603@freebsd.org> References: <201210221418.q9MEINkr026751@svn.freebsd.org> <201210241136.06154.jhb@freebsd.org> <CAJ-FndAG-Qp%2B1aQvoL7YRj=R151Qe9_wNrUeOAaDsdYao_-zCQ@mail.gmail.com> <201210241414.30723.jhb@freebsd.org> <CAJ-FndAu6BGeMMbtFTLaSqy82mbhM9CVEyJ3Lb1WhAogJr59yA@mail.gmail.com> <CAJ-FndBqRpkBhCntd2aqwVYPu%2B2EHGeuXr5srLtrNNDK-ButxA@mail.gmail.com> <508965B3.2020705@freebsd.org> <CAJ-FndCMMH2Qy=rzzxagNVfgO9vF0xZY6B_vrnjmv_dXKNB5Dg@mail.gmail.com> <5089A913.2040603@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Oct 25, 2012 at 10:03 PM, Andre Oppermann <andre@freebsd.org> wrote: > On 25.10.2012 18:21, Attilio Rao wrote: >> >> On Thu, Oct 25, 2012 at 5:15 PM, Andre Oppermann <andre@freebsd.org> >> wrote: >>> >>> On 25.10.2012 05:14, Attilio Rao wrote: >>>> >>>> >>>> On Wed, Oct 24, 2012 at 9:13 PM, Attilio Rao <attilio@freebsd.org> >>>> wrote: >>>> I think I've had a better idea for this. >>>> In our locking scheme we already rely on the fact that lock_object >>>> will always be present in all our locking primitives and that it will >>>> be the first object of every locking primitives. This is an assumption >>>> we must live with in order to correctly implement lock classes. I >>>> think we can use the same concept in order to use the same KPI for the >>>> 2 different structures (struct mtx and struct mtx_unshare) and keep >>>> the compile time ability to find stupid bugs. >>> >>> >>> >>> I think we're completely overdoing it. On amd64 the size difference >>> of 64B cache line aligning and padding all mutex, sx and rw_lock >>> structures adds the tiny amount of 16K on a GENERIC kernel of 19MB. >>> That is a 0.009% increase in size. Of course dynamically allocated >>> memory that includes a mutex grows a tiny bit at well. >>> >>> Hence I propose to unconditionally slap __aligned(CACHE_LINE_SIZE) into >>> all locking structures and be done with it. As an added benefit we >>> don't have to worry about individual micro-optimizations on a case by >>> case basis. >> >> >> Did you see my (and also Jeff) objection to your proposal about this? >> You are deliberating ignoring this? > > > Well, I'm allowed to have a different opinion, am I? I'm not ignoring > your objection in the sense as I'm not trying to commit any of this to > HEAD while it is disputed. > > Mind you this whole conversation was started because I was trying to solve > a problem with unfortunate cache line sharing for global mutexes in the > kernel .bss section on my *personal* svn branch. Andre, I'm sorry if you felt I was being harsh or confrontative. This was really not my intention and I apologize. Said that, I fail in seeing a proper technical discussion on your side on how what I propose is "overdoing it" or how do you plan to address the concerns people are raising with your proposal of bumping all lock sizes indiscriminately. However, here is the first half of the patch I'd like to see in: http:///www.freebsd.org/~attilio/mtx_decoupled.patch This is just the part to give the ability to crunch different structures to the mtx KPI. Please note that from the users perspective the mtx KPI remains absolutely the same, so there is theoretically no KPI discontinuity, the support is absolutely transparent. It is not yet throughfully tested, because I first want to hear opinions also on nits. For example: - Do we need to implement mtxlock2mtx() only in kern_mutex.c? I'd say yes, as a general rule, but as long as this functionality relies on the ABI maybe it is better to leave it in _mutex.h as done in the patch. - MTX_SYSINIT() doesn't work for hypotetical mtx_unshare right now so it will need to have its own version. Should I add it now? Should I add it if and only if there is a real need? (I suspect this is likely because several external mutexes, that we want to convert, may use it) - others that came to your mind Thanks, 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-FndApwOo8xmZQyeqq0bGp8P13QWRqgmSDNg1_hbm7nrpOAQ>