Re: [RFC/RFT] calloutng

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
_at__at_ -27,15 +27,12 _at__at_
  */
 
 #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>