Date: Sat, 27 Oct 2012 15:35:05 +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-FndC%2BujncG54_RMwKwdtvNCRJ2QojNuADWn59puwayCHs6A@mail.gmail.com> In-Reply-To: <508A89EF.5070805@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> <CAJ-FndApwOo8xmZQyeqq0bGp8P13QWRqgmSDNg1_hbm7nrpOAQ@mail.gmail.com> <508A89EF.5070805@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 10/26/12, Andre Oppermann <andre@freebsd.org> wrote: > On 26.10.2012 08:27, Attilio Rao wrote: >> On Thu, Oct 25, 2012 at 10:03 PM, Andre Oppermann <andre@freebsd.org> >> wrote: >>> On 25.10.2012 18:21, Attilio Rao wrote: >>>> 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. > > I apologize too for being a bit difficult and taking some time understand > the differences in __aligned() regarding padding behavior. > >> 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. > > I'm wary of micro-optimizing and generally prefer clean and for a > reader obvious approaches. That said my assumption on the distribution > of mutex use cases in the kernel was wrong. By counting from a grep > it seems that about half of the mutexes could possibly benefit from > being padded and the other half doesn't because it is in structures > and next to its data. Besides that, you are likely misunderstanding something about what I propose: what I'm proposing is completely transparent to developers. You will just need to declare a mtx like: struct mtx_unshare Giant; and then you can use the mtx(9) interface on it without any issue. I don't see how this is less clean than what you propose. It just enables the alignment/padding on a selection basis rather than 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. > > This seems rather complicated. Instead of mtxlock2mtx() wouldn't > __containerof() work just as well? The __DEVOLATILE() looks a bit > dangerous. Are you sure the compiler won't reorder things it should > not? What do you mean with "rather complicated"? For the users of the primitive nothing changes at all. For the people that might read the code it is pretty much self-explanatory, in particular if you know how lock classes work in our locking scheme. Maybe I can add a comment or two to clarify. About the __DEVOLATILE your statement makes absolutely no sense. We are not accessing the cookie, we are just modifying the value of the pointer to go lower and then point to the struct mtx. This has nothing to do with cache effects of the lock cookie. 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-FndC%2BujncG54_RMwKwdtvNCRJ2QojNuADWn59puwayCHs6A>