svn commit: r343058 - in head/sys: compat/linuxkpi/common/src vm

Bruce Evans brde at optusnet.com.au
Wed Jan 16 10:23:54 UTC 2019


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


More information about the svn-src-all mailing list