From owner-svn-src-all@FreeBSD.ORG Wed Oct 24 19:58:22 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 725C6127 for ; Wed, 24 Oct 2012 19:58:22 +0000 (UTC) (envelope-from jroberson@jroberson.net) Received: from mail-pb0-f54.google.com (mail-pb0-f54.google.com [209.85.160.54]) by mx1.freebsd.org (Postfix) with ESMTP id 34F208FC14 for ; Wed, 24 Oct 2012 19:58:21 +0000 (UTC) Received: by mail-pb0-f54.google.com with SMTP id rp8so1558185pbb.13 for ; Wed, 24 Oct 2012 12:58:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:x-x-sender:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type:x-gm-message-state; bh=kabrGDOq4oheVLS3hF4Ffw8TK9UxaxIjvhbpRw9XMfU=; b=EAAgLCX/1eT6n2uBYKuUSDDs5AY8fOKjSRN3dE4lmURlDgSTlUbo3eI36wieoKyHUS GxuCEvZqo6+KEdkkJ+W3888TOerOa8XAYBjzXKOCM6FwY9Z2XRo/o3gno6fKiNvr1rsY HpQ/XceTEbSzHhCt8QIuj3g5it6tA6o3oC9WMPXrnT5MnLUMHzQ0W/clNYqvNC9Js1q8 5psasxLqdHK0vwbm5rwF3H1P3nv+PPC3ZLNt3LSLQgQ+h1kqsFSKIKDG1Q2LDecrMwIq hLSaur8cUEeOqwp8tUdzXU8KZPbLLJGB60sXESADQ6jNNXoYLiohgW8aRruA17w8hTQD cwOA== Received: by 10.68.136.98 with SMTP id pz2mr53133501pbb.2.1351108701410; Wed, 24 Oct 2012 12:58:21 -0700 (PDT) Received: from rrcs-66-91-135-210.west.biz.rr.com (rrcs-66-91-135-210.west.biz.rr.com. [66.91.135.210]) by mx.google.com with ESMTPS id ro5sm9897028pbc.66.2012.10.24.12.58.18 (version=SSLv3 cipher=OTHER); Wed, 24 Oct 2012 12:58:20 -0700 (PDT) Date: Wed, 24 Oct 2012 09:57:18 -1000 (HST) From: Jeff Roberson X-X-Sender: jroberson@desktop To: Attilio Rao Subject: Re: svn commit: r242014 - head/sys/kern In-Reply-To: Message-ID: References: <201210241836.q9OIafqo073002@svn.freebsd.org> <50883EA8.1010308@freebsd.org> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Gm-Message-State: ALoCoQk2ghsuB8v5UfF3eiuiojrLnaqiAPzz5OZCpzj7SK9RB2+pftYRqjjm6N0HH2+VHLm6U7TE Cc: Adrian Chadd , src-committers@freebsd.org, Andre Oppermann , svn-src-all@freebsd.org, svn-src-head@freebsd.org, Jim Harris X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Oct 2012 19:58:22 -0000 On Wed, 24 Oct 2012, Attilio Rao wrote: > On Wed, Oct 24, 2012 at 8:16 PM, Andre Oppermann wrote: >> On 24.10.2012 20:56, Jim Harris wrote: >>> >>> On Wed, Oct 24, 2012 at 11:41 AM, Adrian Chadd wrote: >>>> >>>> On 24 October 2012 11:36, Jim Harris wrote: >>>> >>>>> Pad tdq_lock to avoid false sharing with tdq_load and tdq_cpu_idle. >>>> >>>> >>>> Ok, but.. >>>> >>>> >>>>> struct mtx tdq_lock; /* run queue lock. */ >>>>> + char pad[64 - sizeof(struct mtx)]; >>>> >>>> >>>> .. don't we have an existing compile time macro for the cache line >>>> size, which can be used here? >>> >>> >>> Yes, but I didn't use it for a couple of reasons: >>> >>> 1) struct tdq itself is currently using __aligned(64), so I wanted to >>> keep it consistent. >>> 2) CACHE_LINE_SIZE is currently defined as 128 on x86, due to >>> NetBurst-based processors having 128-byte cache sectors a while back. >>> I had planned to start a separate thread on arch@ about this today on >>> whether this was still appropriate. >> >> >> See also the discussion on svn-src-all regarding global struct mtx >> alignment. >> >> Thank you for proving my point. ;) >> >> Let's go back and see how we can do this the sanest way. These are >> the options I see at the moment: >> >> 1. sprinkle __aligned(CACHE_LINE_SIZE) all over the place > > This is wrong because it doesn't give padding. > >> 2. use a macro like MTX_ALIGN that can be SMP/UP aware and in >> the future possibly change to a different compiler dependent >> align attribute > > What is this macro supposed to do? I don't understand that from your > description. > >> 3. embed __aligned(CACHE_LINE_SIZE) into struct mtx itself so it >> automatically gets aligned in all cases, even when dynamically >> allocated. > > This works but I think it is overkill for structures including sleep > mutexes which are the vast majority. So I wouldn't certainly be in > favor of such a patch. I agree. For locks with little contention we probably want smaller structures. For example, you wouldn't want to put a huge lock in every file descriptor. It would be nice to have an automatic way to pad every global lock though. I think it should be done as needed. Jeff > > Attilio > > > -- > Peace can only be achieved by understanding - A. Einstein >