From owner-svn-src-user@FreeBSD.ORG Thu Oct 25 16:21:22 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 C445BE06; Thu, 25 Oct 2012 16:21:22 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-la0-f54.google.com (mail-la0-f54.google.com [209.85.215.54]) by mx1.freebsd.org (Postfix) with ESMTP id 6D3B38FC08; Thu, 25 Oct 2012 16:21:21 +0000 (UTC) Received: by mail-la0-f54.google.com with SMTP id e12so2138702lag.13 for ; Thu, 25 Oct 2012 09:21:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:reply-to:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=+JoBUvvQPpwROaS8BZwtiFNpLPMmchvdKqHnFSL5haw=; b=uKhORiViP7sB+YPDiG48yxuwwSPNZ8ItVPuhULyWDP3d/i+qFxFW1nIJxCqOqENOKn HTiebuOnnpZ/4hAcxD4S/tPpiwNGsDRUHFI8Du56niMMWGj+fjlqUyTBF2/R7RB8NDYa xpSYYn7Fmdzzh9zSYMfUC8rLkRF7uFa/qmIvbFclTPaoyGthvNe4YTUPAz9xXXOAPmUy g0M7dTz5Br1Vt1ygAr+bgokY4wuTWNErhB0My/UP7WUWVU8imgcU6zoAAL6SefccidE2 pTT1/aZdz8sEMfOSITcJW7tGE2lXlsL6PcnN8OGcGUNyc17lKwayo5EKhRQxG8YlYbjt F1Ow== MIME-Version: 1.0 Received: by 10.152.148.169 with SMTP id tt9mr18103451lab.15.1351182081042; Thu, 25 Oct 2012 09:21:21 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.112.30.37 with HTTP; Thu, 25 Oct 2012 09:21:20 -0700 (PDT) In-Reply-To: <508965B3.2020705@freebsd.org> References: <201210221418.q9MEINkr026751@svn.freebsd.org> <201210241136.06154.jhb@freebsd.org> <201210241414.30723.jhb@freebsd.org> <508965B3.2020705@freebsd.org> Date: Thu, 25 Oct 2012 17:21:20 +0100 X-Google-Sender-Auth: iaMz1hijo0GQAydYFBlXnEiff54 Message-ID: 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/... From: Attilio Rao To: Andre Oppermann Content-Type: text/plain; charset=UTF-8 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 Reply-To: attilio@FreeBSD.org 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: Thu, 25 Oct 2012 16:21:23 -0000 On Thu, Oct 25, 2012 at 5:15 PM, Andre Oppermann wrote: > On 25.10.2012 05:14, Attilio Rao wrote: >> >> On Wed, Oct 24, 2012 at 9:13 PM, Attilio Rao wrote: >>> >>> On Wed, Oct 24, 2012 at 7:14 PM, John Baldwin wrote: >>>> >>>> On Wednesday, October 24, 2012 11:41:24 am Attilio Rao wrote: >>>>> >>>>> On Wed, Oct 24, 2012 at 4:36 PM, John Baldwin wrote: >>>>>> >>>>>> On Wednesday, October 24, 2012 11:24:22 am Attilio Rao wrote: >>>>>>> >>>>>>> On Wed, Oct 24, 2012 at 4:09 PM, Attilio Rao >>>>>>> wrote: >>>>>> >>>>>> 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. >>>> >>>> >>>> Sure they do. They still call a function that takes a 'struct mtx *' >>>> even >>>> if it isn't called in the fast path. If you pass a 'struct sx *' to >>>> mtx_lock() it will fail to compile. That needs to stay that way. >>> >>> >>> I think that with some trickery using CTASSERT() and typeof() we may >>> be able to enforce sanity even with void * arguments. >> >> >> 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? I think that mutexes being part of structures usually don't want that and it is really overkill. It is not the amount of wasted memory the problem (which is also important, anyway) but the fact that not-really-contentended sleep mutexes, will interfree with normal structure members caching. I think it is a very bad idea. Attilio -- Peace can only be achieved by understanding - A. Einstein