Date: Mon, 17 Dec 2012 00:52:41 -0500 From: Mark Johnston <markjdb@gmail.com> To: Bruce Evans <brde@optusnet.com.au> Cc: Garrett Cooper <yanegomi@gmail.com>, Davide Italiano <davide@FreeBSD.org>, Alexander Motin <mav@FreeBSD.org>, FreeBSD Current <freebsd-current@FreeBSD.org>, freebsd-arch@FreeBSD.org Subject: Re: [RFC/RFT] calloutng Message-ID: <20121217055241.GA5228@oddish> In-Reply-To: <20121216175614.V1027@besplex.bde.org> References: <50CCAB99.4040308@FreeBSD.org> <20121215203458.GA22361@oddish> <35705A81-690A-4993-B0C3-C8BC0BC89C67@gmail.com> <20121216175614.V1027@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Dec 16, 2012 at 06:14:07PM +1100, Bruce Evans wrote: > On Sat, 15 Dec 2012, Garrett Cooper wrote: > > > On Dec 15, 2012, at 12:34 PM, Mark Johnston wrote: > > > >> On Sat, Dec 15, 2012 at 06:55:53PM +0200, Alexander Motin wrote: > >>> Hi. > >>> > >>> I'm sorry to interrupt review, but as usual good ideas came during the > >>> final testing, causing another round. :) Here is updated patch for > >>> HEAD, that includes several new changes: > >>> http://people.freebsd.org/~mav/calloutng_12_15.patch > >> > >> This patch breaks the libprocstat build. > >> > >> Specifically, the OpenSolaris sys/time.h defines the preprocessor > >> symbols gethrestime and gethrestime_sec. These symbols are also defined > >> in cddl/contrib/opensolaris/lib/libzpool/common/sys/zfs_context.h. > >> libprocstat:zfs.c is compiled using include paths that pick up the > >> OpenSolaris time.h, and with this patch _callout.h includes sys/time.h. > >> > >> zfs.c includes taskqueue.h (with _KERNEL defined), which includes > >> _callout.h, so both time.h and zfs_context.h are included in zfs.c, and > >> the symbols are thus defined twice. > > Gross namespace pollution. sys/_callout.h exists so that the full > namespace pollution of sys/callout.h doesn't get included nested. But > sys/time.h is much more polluted than sys/callout.h. > > However, sys/time.h is old standard pollution in sys/param.h, and > sys/callout.h is not so old standard pollution in sys/systm.h. It is > a bug to not include sys/param.h and sys/systm.h in most kernel source > code, so these nested includes are just style bugs -- they have no > effect for correct kernel source code. > > >> The patch below fixes the build for me. Another approach might be to > >> include sys/_task.h instead of taskqueue.h at the beginning of zfs.c. > > Good if it works. The diff below is what I had in mind. taskqueue.h is used so that sizeof(struct task) can be used, but I don't see why that's preferable to just including _task.h. -Mark diff --git a/lib/libprocstat/zfs.c b/lib/libprocstat/zfs.c index aa6d78e..f04eedf 100644 --- a/lib/libprocstat/zfs.c +++ b/lib/libprocstat/zfs.c @@ -27,15 +27,12 @@ */ #include <sys/param.h> +#include <sys/_task.h> #define _KERNEL #include <sys/mount.h> -#include <sys/taskqueue.h> #undef _KERNEL #include <sys/sysctl.h> -#undef lbolt -#undef lbolt64 -#undef gethrestime_sec #include <sys/zfs_context.h> #include <sys/spa.h> #include <sys/spa_impl.h>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121217055241.GA5228>