From owner-svn-src-user@FreeBSD.ORG Fri Oct 26 13:02:55 2012 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id E2C8A2E8 for ; Fri, 26 Oct 2012 13:02:54 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.freebsd.org (Postfix) with ESMTP id 3207E8FC19 for ; Fri, 26 Oct 2012 13:02:53 +0000 (UTC) Received: (qmail 55910 invoked from network); 26 Oct 2012 14:40:21 -0000 Received: from c00l3r.networx.ch (HELO [127.0.0.1]) ([62.48.2.2]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 26 Oct 2012 14:40:21 -0000 Message-ID: <508A89EF.5070805@freebsd.org> Date: Fri, 26 Oct 2012 15:02:39 +0200 From: Andre Oppermann User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/20121010 Thunderbird/16.0.1 MIME-Version: 1.0 To: attilio@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/... References: <201210221418.q9MEINkr026751@svn.freebsd.org> <201210241136.06154.jhb@freebsd.org> <201210241414.30723.jhb@freebsd.org> <508965B3.2020705@freebsd.org> <5089A913.2040603@freebsd.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: mdf@freebsd.org, src-committers@freebsd.org, John Baldwin , svn-src-user@freebsd.org, Jeff Roberson , Bruce Evans X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Oct 2012 13:02:55 -0000 On 26.10.2012 08:27, Attilio Rao wrote: > On Thu, Oct 25, 2012 at 10:03 PM, Andre Oppermann 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. > 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? > It is not yet throughfully tested, because I first want to hear > opinions also on nits. Different though: Since cases where an aligned and padded mutex is wanted it has to be specified explicitly anyway. Can't we simplify and have the aligned one be a container of the normal one and do some union magic? That way the lock_* functions can stay the same and current mutex size doesn't change. > 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 -- Andre