From owner-freebsd-arch@FreeBSD.ORG Sun Dec 16 07:14:18 2012 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 017C5BF8; Sun, 16 Dec 2012 07:14:18 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail28.syd.optusnet.com.au (mail28.syd.optusnet.com.au [211.29.133.169]) by mx1.freebsd.org (Postfix) with ESMTP id 865DD8FC17; Sun, 16 Dec 2012 07:14:17 +0000 (UTC) Received: from c122-106-175-26.carlnfd1.nsw.optusnet.com.au (c122-106-175-26.carlnfd1.nsw.optusnet.com.au [122.106.175.26]) by mail28.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id qBG7E7jR006811 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 16 Dec 2012 18:14:09 +1100 Date: Sun, 16 Dec 2012 18:14:07 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Garrett Cooper Subject: Re: [RFC/RFT] calloutng In-Reply-To: <35705A81-690A-4993-B0C3-C8BC0BC89C67@gmail.com> Message-ID: <20121216175614.V1027@besplex.bde.org> References: <50CCAB99.4040308@FreeBSD.org> <20121215203458.GA22361@oddish> <35705A81-690A-4993-B0C3-C8BC0BC89C67@gmail.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=Zvi1sKHG c=1 sm=1 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=mUAV9h2nInsA:10 a=6I5d2MoRAAAA:8 a=QqY8FBgQtirPoTa2VqoA:9 a=CjuIK1q_8ugA:10 a=bxQHXO5Py4tHmhUgaywp5w==:117 Cc: Davide Italiano , freebsd-arch@FreeBSD.org, Alexander Motin , FreeBSD Current , Mark Johnston X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 16 Dec 2012 07:14:18 -0000 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. > I had a patch open once upon a time to cleanup inclusion of sys/time.h all over the tree and deal with the sys/time.h <-> time.h pollution issue, but it got dropped due to lack of interest (20~30 apps/libs were affected IIRC and I only really got assistance in fixing the UFS and bsnmpd pieces, and gave up due to lack of response from maintainers). dtrace/zfs is a definite instigator in this pollution (I remember nasty cddl/... pollution with the compat sys/time.h header). Please use the unix newline character in mail. The above is difficult to quote. The standard sys/time.h pollution in sys/param.h is only in the kernel, and there aren't many direct includes of sys/time.h in the kernel. Userland is different and many of the direct includes were correct. But not POSIX specifies that struct timespec and struct timeval be defined in most places where they are needed, so the includes of sys/time.h are not necessary for POSIX or FreeBSD, although FreeBSD man pages still say that they are necessary. The sys/time.h <-> time.h pollution issue is also only for userland. Many places depend on one including the other, and include the wrong one themself. > Bottom line: make sure anything new you're defining isn't already defined via POSIX or other OSes, and if so please try to make the implementations match (so that eventual POSIX inclusion might be possible) and when in doubt I suggest consulting standards@ / brde@. Bruce