From owner-freebsd-current@FreeBSD.ORG Mon Dec 17 05:52:51 2012 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 49703739; Mon, 17 Dec 2012 05:52:51 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-ie0-f182.google.com (mail-ie0-f182.google.com [209.85.223.182]) by mx1.freebsd.org (Postfix) with ESMTP id D32CE8FC0C; Mon, 17 Dec 2012 05:52:50 +0000 (UTC) Received: by mail-ie0-f182.google.com with SMTP id s9so8564453iec.13 for ; Sun, 16 Dec 2012 21:52:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=pEQVqOAblMbQuUoYbG8D0QuMm6MYG5VIQNhxjQxaN3A=; b=NWNsiHw1pYoEK8NB1Bw9/Tsdv33jdbJOrJZKp33o6C8Q5P1JJutzuVAk0DATnBIQQ8 w0e+Is3+TmGdmWRa94ZWZRmxRL0zhnlnYRMJo5P5J76gepzFZaJvPZtdyPyOugAqA88U CGQH3WnkcxesjI4EWniTaxgCiOEq/z9tRo/iyaksIspEjzWbmNbRmUAEYxU5bzHxL48T xF23Sfc4wN3aRVODgjWaJcJAI1lxaGfU5h5blf29zJCD035kMcf/h8/4PlUccBd0G7JP jSKf/6kdE4731zYKU2MK2N+RExTQMIca6g6QF8Hz7gr4A3kt7e0FXboAhB86Rz+qqr7D P3kQ== X-Received: by 10.50.34.225 with SMTP id c1mr8011375igj.67.1355723569999; Sun, 16 Dec 2012 21:52:49 -0800 (PST) Received: from oddish ([66.11.160.25]) by mx.google.com with ESMTPS id ex10sm5498832igc.15.2012.12.16.21.52.48 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 16 Dec 2012 21:52:49 -0800 (PST) Date: Mon, 17 Dec 2012 00:52:41 -0500 From: Mark Johnston To: Bruce Evans Subject: Re: [RFC/RFT] calloutng Message-ID: <20121217055241.GA5228@oddish> References: <50CCAB99.4040308@FreeBSD.org> <20121215203458.GA22361@oddish> <35705A81-690A-4993-B0C3-C8BC0BC89C67@gmail.com> <20121216175614.V1027@besplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20121216175614.V1027@besplex.bde.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Garrett Cooper , Davide Italiano , Alexander Motin , FreeBSD Current , freebsd-arch@FreeBSD.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Dec 2012 05:52:51 -0000 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 +#include #define _KERNEL #include -#include #undef _KERNEL #include -#undef lbolt -#undef lbolt64 -#undef gethrestime_sec #include #include #include