From owner-svn-src-head@freebsd.org Fri Jul 6 16:49:09 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C8397103A28E for ; Fri, 6 Jul 2018 16:49:09 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-io0-x243.google.com (mail-io0-x243.google.com [IPv6:2607:f8b0:4001:c06::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 570F78A7D2 for ; Fri, 6 Jul 2018 16:49:09 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-io0-x243.google.com with SMTP id z20-v6so11424200iol.0 for ; Fri, 06 Jul 2018 09:49:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=MySFb3FRJ3T9ckp2fFPCFk5gJr+ddsI/c4SDWCNMACA=; b=J1X5d/7EkLnEFStUqvG993z/a7ZdVi6Fv+MAaDEAqdwrt/RnJSep4blir9g9sW+9QW wyk8J1yz6ISYQT/pSzJTvkgmcfu8utysXqkPps6wm6lc3ABsK9BP4a7YKLXBuHzBbnNf CwLayx5jmhJ3HALQXOvVGSRQe3orJlq/YMKUFVXCtOmx/PPqe6h2BXpU2kJwSvxbjNgf gbwlWH5tuOmD13K2HtrUuMpvWndCpbLc/Y6hotpxwAbegTQGIKJPpKqHfXnkqHtlmrA4 oj3cpSY0gVeqnC2vjb5ZRJINnPMfxicWvQM/CYp1h1VaSJf5VeNC76rjeqsxxxjCBsTE 41yQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=MySFb3FRJ3T9ckp2fFPCFk5gJr+ddsI/c4SDWCNMACA=; b=s3Ix6AAZMOAa79/fC8LkOguSw3pk+r8CXl5D7V/oDOY3cwpV6wcKttzz7M+FoEp8vA kPc7lKeozK5zPpgVH1/ssDC8hDZbc21gKZhSmitx2cxsY6EEGxzSGFPiSeamSZW22MEk TXQIduskL5goIR18PL9rg9vM6AxHXteJ/+GNx8JCPmb5G1ILcnJ+GZMxiV5/FEUP7eWU fCPxwVzijZU/c0yI1DEc6j4ozQZ2yVE+7TTe5A3t1tha3dBcn0XNv31f1x1Q02g7uHdL dprknbC25kMQ7UnYfCODTUo7mKPKTLHV1+90otmTZ6auy6G6HqDJKH8xzW1xi5MDw1jc EWIw== X-Gm-Message-State: APt69E0jBBev+rYwQ1JsIMoJAlS6Q3220EGpE7v3f8gL3I+2e/KC/Ivl rM3LfiH+RcSrb8IOSdmzrjFLE+LFt590BghKvXgpTw== X-Google-Smtp-Source: AAOMgpfQKbwuevpGoaB/8VLNn0vkEpVQt7nru4z9kOXL0QiVHi3HWEXzDXqXX8AAtf8ApvF2jr1hD/KWrYZKz+p0Hw0= X-Received: by 2002:a6b:d004:: with SMTP id x4-v6mr8883803ioa.299.1530895748303; Fri, 06 Jul 2018 09:49:08 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 2002:a4f:1183:0:0:0:0:0 with HTTP; Fri, 6 Jul 2018 09:49:07 -0700 (PDT) X-Originating-IP: [74.62.67.99] In-Reply-To: <201807061552.w66Fq0FX052931@pdx.rh.CN85.dnsmgr.net> References: <201807061552.w66Fq0FX052931@pdx.rh.CN85.dnsmgr.net> From: Warner Losh Date: Fri, 6 Jul 2018 10:49:07 -0600 X-Google-Sender-Auth: RLZJY7kHwhHgkSutSjmm28ZYtMQ Message-ID: Subject: Re: svn commit: r336025 - in head/sys: amd64/include i386/include To: "Rodney W. Grimes" Cc: Hans Petter Selasky , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.27 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Jul 2018 16:49:10 -0000 On Fri, Jul 6, 2018 at 9:52 AM, Rodney W. Grimes < freebsd@pdx.rh.cn85.dnsmgr.net> wrote: > > On Fri, Jul 6, 2018 at 9:32 AM, Rodney W. Grimes < > > freebsd@pdx.rh.cn85.dnsmgr.net> wrote: > > > > > > Author: hselasky > > > > Date: Fri Jul 6 10:13:42 2018 > > > > New Revision: 336025 > > > > URL: https://svnweb.freebsd.org/changeset/base/336025 > > > > > > > > Log: > > > > Make sure kernel modules built by default are portable between UP > and > > > > SMP systems by extending defined(SMP) to include > defined(KLD_MODULE). > > > > > > > > This is a regression issue after r335873 . > > > > > > > > Discussed with: mmacy@ > > > > Sponsored by: Mellanox Technologies > > > > > > Though this fixes the issue, it also means that now when > > > anyone intentionally builds a UP kernel with modules > > > they are getting SMP support in the modules and I am > > > not sure they would want that. I know I don't. > > > > > > > > > On UP systems, these additional opcodes are harmless. They take a few > extra > > cycles (since they lock an uncontested bus) and add a couple extra memory > > barriers (which will be NOPs). On MP systems, atomics now work by > default. > > Had we not defaulted like this, all modules built outside of a kernel > build > > env would have broken atomics. Given that (a) the overwhelming majority > > (99% or more) is SMP and (b) the MP code merely adds a few cycles to > what's > > already a not-too-expensive operation, this was the right choice. > > > > It simply doesn't matter for systems that are relevant to the project > > today. While one could try to optimize this a little (for example, by > > having SMP defined to be 0 or 1, say, and changing all the ifdef SMP to > if > > (defined(SMP) && SMP != 0)), it's likely not going to matter enough for > > anybody to make the effort. UP on x86 is simply not relevant enough to > > optimize for it. Even in VMs, people run SMP kernels typically even when > > they just allocate one CPU to the VM. > > > > So while we still support the UP config, and we'll let people build > > optimized kernels for x86, we've flipped the switch from pessimized for > SMP > > modules to pessimized for UP modules, which seems like quite the > reasonable > > trade-off. > > > > Were it practical to do so, I'd suggest de-orbiting UP on x86. However, > > it's a lot of work for not much benefit and we'd need to invent much > crazy > > to get there. > > Trivial to fix this with > +#if defined(SMP) || !defined(_KERNEL) || defined(KLD_MODULE) || > !defined(KLD_UP_MODULES) Nope. Not so trivial. Who defines KLD_UP_MODULES? And really, it's absolutely not worth it unless someone shows up with numbers to show the old 'function call to optimal routine' is actually faster than the new 'inline to slightly unoptimal code'. Since I think the function call overhead is larger than the pessmizations, I'm not sure what the fuss is about. Warner > > > > Warner > > > > > > > > Modified: > > > > head/sys/amd64/include/atomic.h > > > > head/sys/i386/include/atomic.h > > > > > > > > Modified: head/sys/amd64/include/atomic.h > > > > ============================================================ > > > ================== > > > > --- head/sys/amd64/include/atomic.h Fri Jul 6 10:10:00 2018 > > > (r336024) > > > > +++ head/sys/amd64/include/atomic.h Fri Jul 6 10:13:42 2018 > > > (r336025) > > > > @@ -132,7 +132,7 @@ void atomic_store_rel_##TYPE( > volatile > > > u_##TYPE *p, u_ > > > > * For userland, always use lock prefixes so that the binaries will > run > > > > * on both SMP and !SMP systems. > > > > */ > > > > -#if defined(SMP) || !defined(_KERNEL) > > > > +#if defined(SMP) || !defined(_KERNEL) || defined(KLD_MODULE) > > > > #define MPLOCKED "lock ; " > > > > #else > > > > #define MPLOCKED > > > > @@ -354,7 +354,7 @@ atomic_testandclear_long(volatile u_long *p, > u_int > > > v) > > > > */ > > > > #define OFFSETOF_MONITORBUF 0x100 > > > > > > > > -#if defined(SMP) > > > > +#if defined(SMP) || defined(KLD_MODULE) > > > > static __inline void > > > > __storeload_barrier(void) > > > > { > > > > > > > > Modified: head/sys/i386/include/atomic.h > > > > ============================================================ > > > ================== > > > > --- head/sys/i386/include/atomic.h Fri Jul 6 10:10:00 2018 > > > (r336024) > > > > +++ head/sys/i386/include/atomic.h Fri Jul 6 10:13:42 2018 > > > (r336025) > > > > @@ -143,7 +143,7 @@ void atomic_subtract_64(volatile > > > uint64_t *, uint64_t > > > > * For userland, always use lock prefixes so that the binaries will > run > > > > * on both SMP and !SMP systems. > > > > */ > > > > -#if defined(SMP) || !defined(_KERNEL) > > > > +#if defined(SMP) || !defined(_KERNEL) || defined(KLD_MODULE) > > > > #define MPLOCKED "lock ; " > > > > #else > > > > #define MPLOCKED > > > > @@ -302,7 +302,7 @@ atomic_testandclear_int(volatile u_int *p, > u_int v) > > > > */ > > > > > > > > #if defined(_KERNEL) > > > > -#if defined(SMP) > > > > +#if defined(SMP) || defined(KLD_MODULE) > > > > #define __storeload_barrier() __mbk() > > > > #else /* _KERNEL && UP */ > > > > #define __storeload_barrier() __compiler_membar() > > > > > > > > > > > > > > -- > > > Rod Grimes > > > rgrimes@freebsd.org > > > > > > > > -- > Rod Grimes > rgrimes@freebsd.org >