Date: Wed, 16 Jan 2019 21:23:38 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Gleb Smirnoff <glebius@freebsd.org> Cc: Justin Hibbits <chmeeedalf@gmail.com>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r343058 - in head/sys: compat/linuxkpi/common/src vm Message-ID: <20190116204533.X1031@besplex.bde.org> In-Reply-To: <20190115200617.GZ18452@FreeBSD.org> References: <201901151933.x0FJXl8a069317@repo.freebsd.org> <20190115134623.139064b2@titan.knownspace> <20190115200617.GZ18452@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 15 Jan 2019, Gleb Smirnoff wrote: > On Tue, Jan 15, 2019 at 01:46:23PM -0600, Justin Hibbits wrote: > J> Why not #include counter.h in the relevant vm_machdep.c files instead? > > This also is ugly :( Not sure more or less. Probably less, but I > urged to plug all possible compilation failures at a time. It is better, but it is a bug if vm_machdep.c files include uma's internal header. Perhaps Justin meant vm files other than vm_machdep.c. The only non-uma ones in vm/* chummy with uma's internals are memguard.c and vm_page.c. > What is ugly is that most files just need counter_u64_t size, > and they don't use counter(9) KPI. > > The fact that vm_machdep or Linux KPI want to look into internal > type uma_zone_t is also ugly. This is a bug in uma. style(9) forbids using typedefs for struct types and pointers to struct types, since these are usually just foot shooting. They usually give the opposite of opaque types, since the header that declarers them usually also declares lots of pollution and often declares the internals of the types that are supposed to be opaque. I, partially fixed this in my version about 15 years ago: XX Index: uma.h XX =================================================================== XX RCS file: /home/ncvs/src/sys/vm/uma.h,v XX retrieving revision 1.18 XX diff -u -2 -r1.18 uma.h XX --- uma.h 1 Jun 2004 01:36:26 -0000 1.18 XX +++ uma.h 1 Jun 2004 08:52:15 -0000 XX @@ -25,17 +25,21 @@ XX * XX * $FreeBSD: src/sys/vm/uma.h,v 1.18 2004/06/01 01:36:26 bmilekic Exp $ XX - * XX */ XX XX +#ifndef _VM_UMA_H_ XX +#define _VM_UMA_H_ XX + XX /* XX * uma.h - External definitions for the Universal Memory Allocator XX - * XX -*/ XX + */ XX XX -#ifndef VM_UMA_H XX -#define VM_UMA_H XX +#include <sys/_null.h> XX XX -#include <sys/param.h> /* For NULL */ XX -#include <sys/malloc.h> /* For M_* */ XX +/* Shared with <sys/malloc.h>. */ XX +#define M_NOWAIT 0x0001 /* do not block */ XX +#define M_WAITOK 0x0002 /* do not block */ XX +#define M_ZERO 0x0100 /* bzero the allocation */ XX +#define M_NOVM 0x0200 /* don't ask VM for pages */ XX +#define M_USE_RESERVE 0x0400 /* can alloc out of reserve memory */ XX XX /* User visable parameters */ uma.h has grosser pollution for NULL and M_*. I don't like lots of little headers like sys/_null.h, but it may as well be used when it exists. Replicating the definitions of M_* is easier than replicating the definition of NULL. XX @@ -44,7 +48,6 @@ XX /* Types and type defs */ XX XX -struct uma_zone; XX /* Opaque type used as a handle to the zone */ XX -typedef struct uma_zone * uma_zone_t; XX +typedef struct uma_zone * uma_zone_t; /* XXX actually non-opaque. */ XX XX /* I didn't completely fix this. The full fix would remove the typedef. XX @@ -254,6 +257,8 @@ XX * returned if the zone is empty or the ctor failed. XX */ XX - XX +#ifndef _UMA_ZALLOC_ARG_DECLARED XX void *uma_zalloc_arg(uma_zone_t zone, void *arg, int flags); XX +#define _UMA_ZALLOC_ARG_DECLARED XX +#endif XX XX /* XX @@ -282,6 +287,8 @@ XX * Nothing. XX */ XX - XX +#ifndef _UMA_ZFREE_ARG_DECLARED XX void uma_zfree_arg(uma_zone_t zone, void *item, void *arg); XX +#define _UMA_ZFREE_ARG_DECLARED XX +#endif XX XX /* These ifdefs fix the largest source of pollution. The declarations are repeated in <sys/mbuf.h> so that mbuf.h doesn't need to include <vm/uma.h>. These functions aren't even inline, so calling them in inline functions in mbuf.h (and inlining these functions) is especially useless. I also fixed mbuf.h to not include <sys/malloc.h>. <sys/malloc.h> and <sys/mbuf.h> are among the few files that I completed removal of pollution in 20-25 years ago. Now they are much more polluted than when I started cleaning them. mbuf.h now includes: sys/param.h, sys/systm.h (_KERNEL only), sys/queue.h (this is not considered pollution, and I didn't clean it), sys/_lock.h, sys/_mutex.h, machine/_limits.h (the last 3 are needed for too much inlining, but are not considered pollution). malloc.h now includes (in bogus order): sys/queue.h, sys/systm.h, vm/uma.h and its nested pollution (last 2 _KERNEL only), sys/lock.h (WITNESS only), sys/sdt.h (_KERNEL only, and not in the suckage section). XX @@ -504,3 +511,3 @@ XX u_int32_t *uma_find_refcnt(uma_zone_t zone, void *item); XX XX -#endif XX +#endif /* !_VM_UMA_H_ */ The patch is for a 2004 version. The current version starts with the same bugs and ends with 2 bugs in the fix for the missing comment. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190116204533.X1031>