Date: Fri, 6 Apr 2018 03:12:08 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Warner Losh <imp@bsdimp.com> Cc: =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= <royger@freebsd.org>, Ian Lepore <ian@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r332072 - head/sys/sys Message-ID: <20180406022720.I3453@besplex.bde.org> In-Reply-To: <CANCZdfputnwHg-gChVKfvZdHn1nHcHXpE0WVLfejHUZfeC8jLg@mail.gmail.com> References: <201804051431.w35EVtg4047897@repo.freebsd.org> <1522942377.49673.245.camel@freebsd.org> <20180405154619.q3blip266qa3z5ut@MacBook-Pro-de-Roger.local> <CANCZdfputnwHg-gChVKfvZdHn1nHcHXpE0WVLfejHUZfeC8jLg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 5 Apr 2018, Warner Losh wrote: > On Thu, Apr 5, 2018 at 9:46 AM, Roger Pau Monn=C3=A9 <royger@freebsd.org>= wrote: > >> On Thu, Apr 05, 2018 at 09:32:57AM -0600, Ian Lepore wrote: >>> On Thu, 2018-04-05 at 14:31 +0000, Roger Pau Monn=C3=A9 wrote: >>>> Log: >>>> introduce GiB and MiB macros >>>> ... >>>> +/* Unit conversion macros. */ >>>> +#define GiB(v) (v ## ULL << 30) >>>> +#define MiB(v) (v ## ULL << 20) >>>> + >>>> #endif /* _SYS_PARAM_H_ */ >>> >>> These names don't make it clear whether the conversion is bytes->GiB or >>> GiB->bytes. The names seem way too generic for a public namespace in a >>> file as heavily included behind your back as param.h is. >>> >>> Also, this completely reasonable usage won't work, likely with >>> confusing compile error messages: >>> >>> int bytes, gibytes; >>> ... >>> bytes =3D GiB(gibytes); >> >> I find those helpful for their specific usage. I could introduce >> static inline functions like: >> >> size_t gb_to_bytes(size_t)... >> >> But I assume this is also going to cause further discussion. Yes, it gives even more namespace pollution and type errors. Macros at least don't expose their internals if they are not used. size_t is actually already part of the undocumented namespace pollution in <sys/param.h>. The type errors are restriction to just one type in another way. Type- generic APIs that avoid such restrictions are much harder to implement using inline functions than macros. > Yea, traditional macro names would be "gibtob" and "btogib" but I didn't > just reply to bikeshed a name: > > But you don't need to specify a type, consider the current btodb macro: > #define btodb(bytes) /* calculates (bytes / DEV_BSIZE) > */ \ > (sizeof (bytes) > sizeof(long) \ > ? (daddr_t)((unsigned long long)(bytes) >> DEV_BSHIFT) \ > : (daddr_t)((unsigned long)(bytes) >> DEV_BSHIFT)) > > which shows how to do this in a macro, which is orthogonal to any name yo= u > may choose. I can also bikeshed function vs macro :) This macro is mostly my mistake in 1995-1996. The long long abominations in it were supposed to be temporary (until C99 standardized something better). It was originally MD for i386 only and then the sizes of almost all types are known and fixed so it is easier to hard-code minimal sizes that work. The optimization of avoiding using 64-bit types was more needed in 1995-1996 since CPUs were slower and compilers did less strength reducti= on. btodb() is much easier than dbtob() since it shifts right, so it can't overflow unless the cast of 'bytes' is wrong so that it truncations. dbtob() doesn't try hard to be optimal. It just always upcasts to off_t. jake later convinced me (in connection with his PAE and sparc64 work) that it should be the caller's responsibility to avoid overflow. Any casts in the macro limits it to the types in it. This is why the page to byte conversion macros don't have any casts in them. PAE usually needs 64-bit results, but this would just be a pessimization for normal i386, and deciding the casts in the macro as above is complicated. So correct GB() macros would look like ((v) << 30), where the caller must cast v to a large enough type. E.g., for variable v which might be larger than 4, on 32-bit systems, the caller must write something like GB((uintmax_t)v). But it is easier for writing to just multiply v by 1G. This is also easier for reading since it is unclear that GB() is even a conversion or which direction it goes in. A longer descriptive name would be about as clear and long as an explicit multiplication. I usually write 1G as ((type)1024 * 1024 * 1024) since the decimal and even hex values of 1G have too many digits to be clear, and multiplication is clearer than shifting and allows the type to be in the factor. Disk block size conversions need to use macros since the DEV_BSIZE =3D 512 was variable in theory (in practice this is now a fixed virtual size). Conversions to G don't need macros since the magic number in them is no more magic than the G in their name. Bruce From owner-svn-src-head@freebsd.org Thu Apr 5 18:17:47 2018 Return-Path: <owner-svn-src-head@freebsd.org> 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 7D343F97FF8; Thu, 5 Apr 2018 18:17:47 +0000 (UTC) (envelope-from brooks@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E4B416961B; Thu, 5 Apr 2018 18:17:46 +0000 (UTC) (envelope-from brooks@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id DF24F262CC; Thu, 5 Apr 2018 18:17:46 +0000 (UTC) (envelope-from brooks@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w35IHkin059733; Thu, 5 Apr 2018 18:17:46 GMT (envelope-from brooks@FreeBSD.org) Received: (from brooks@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w35IHkau059730; Thu, 5 Apr 2018 18:17:46 GMT (envelope-from brooks@FreeBSD.org) Message-Id: <201804051817.w35IHkau059730@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: brooks set sender to brooks@FreeBSD.org using -f From: Brooks Davis <brooks@FreeBSD.org> Date: Thu, 5 Apr 2018 18:17:46 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r332080 - in head/lib/libc: aarch64/sys arm/sys mips/sys riscv/sys X-SVN-Group: head X-SVN-Commit-Author: brooks X-SVN-Commit-Paths: in head/lib/libc: aarch64/sys arm/sys mips/sys riscv/sys X-SVN-Commit-Revision: 332080 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current <svn-src-head.freebsd.org> List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-src-head>, <mailto:svn-src-head-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/svn-src-head/>; List-Post: <mailto:svn-src-head@freebsd.org> List-Help: <mailto:svn-src-head-request@freebsd.org?subject=help> List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-src-head>, <mailto:svn-src-head-request@freebsd.org?subject=subscribe> X-List-Received-Date: Thu, 05 Apr 2018 18:17:47 -0000 Author: brooks Date: Thu Apr 5 18:17:46 2018 New Revision: 332080 URL: https://svnweb.freebsd.org/changeset/base/332080 Log: Remove architecture specific shmat.S files. These files are identical to the generated system calls. In the case of MIPS, the file was already disconnected from the build. Submitted by: Ali Mashtizadeh <ali@mashtizadeh.com> Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D14976 Deleted: head/lib/libc/aarch64/sys/shmat.S head/lib/libc/arm/sys/shmat.S head/lib/libc/mips/sys/shmat.S head/lib/libc/riscv/sys/shmat.S Modified: head/lib/libc/aarch64/sys/Makefile.inc head/lib/libc/arm/sys/Makefile.inc head/lib/libc/riscv/sys/Makefile.inc Modified: head/lib/libc/aarch64/sys/Makefile.inc ============================================================================== --- head/lib/libc/aarch64/sys/Makefile.inc Thu Apr 5 17:26:03 2018 (r332079) +++ head/lib/libc/aarch64/sys/Makefile.inc Thu Apr 5 18:17:46 2018 (r332080) @@ -5,7 +5,6 @@ MIASM:= ${MIASM:Nfreebsd[467]_*} SRCS+= __vdso_gettc.c MDASM= cerror.S \ - shmat.S \ syscall.S \ vfork.S Modified: head/lib/libc/arm/sys/Makefile.inc ============================================================================== --- head/lib/libc/arm/sys/Makefile.inc Thu Apr 5 17:26:03 2018 (r332079) +++ head/lib/libc/arm/sys/Makefile.inc Thu Apr 5 18:17:46 2018 (r332080) @@ -2,7 +2,7 @@ SRCS+= __vdso_gettc.c -MDASM= Ovfork.S brk.S cerror.S sbrk.S shmat.S syscall.S +MDASM= Ovfork.S brk.S cerror.S sbrk.S syscall.S # Don't generate default code for these syscalls: NOASM+= vfork.o Modified: head/lib/libc/riscv/sys/Makefile.inc ============================================================================== --- head/lib/libc/riscv/sys/Makefile.inc Thu Apr 5 17:26:03 2018 (r332079) +++ head/lib/libc/riscv/sys/Makefile.inc Thu Apr 5 18:17:46 2018 (r332080) @@ -2,9 +2,7 @@ SRCS+= trivial-vdso_tc.c -#MDASM= ptrace.S MDASM= cerror.S \ - shmat.S \ syscall.S \ vfork.S
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180406022720.I3453>