From owner-freebsd-current@FreeBSD.ORG Thu Nov 12 03:25:29 2009 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3BC13106566B; Thu, 12 Nov 2009 03:25:29 +0000 (UTC) (envelope-from gibbs@scsiguy.com) Received: from aslan.scsiguy.com (mail.scsiguy.com [70.89.174.89]) by mx1.freebsd.org (Postfix) with ESMTP id 9E6368FC17; Thu, 12 Nov 2009 03:25:28 +0000 (UTC) Received: from [192.168.0.6] (tumnus.scsiguy.com [192.168.0.6]) (authenticated bits=0) by aslan.scsiguy.com (8.14.3/8.14.3) with ESMTP id nAC3PQlV074683 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 11 Nov 2009 20:25:27 -0700 (MST) (envelope-from gibbs@scsiguy.com) Message-ID: <4AFB801F.5020806@scsiguy.com> Date: Wed, 11 Nov 2009 20:25:19 -0700 From: "Justin T. Gibbs" User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.4pre) Gecko/20090915 Thunderbird/3.0b4 ThunderBrowse/3.2.6.5 MIME-Version: 1.0 To: Attilio Rao References: <3bbf2fe10911060643g60079e31y7679b32dac670fd8@mail.gmail.com> <4AFAEE89.3020009@scsiguy.com> <3bbf2fe10911111746g48c8fb82r74744dffedf49758@mail.gmail.com> In-Reply-To: <3bbf2fe10911111746g48c8fb82r74744dffedf49758@mail.gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: gibbs@freebsd.org, freebsd-current@freebsd.org, Scott Long , Ed Maste Subject: Re: [PATCH] Adding sysctl for errors statistics to ahd(4) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 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: Thu, 12 Nov 2009 03:25:29 -0000 On 11/11/2009 6:46 PM, Attilio Rao wrote: > 2009/11/11 Justin T. Gibbs: >> On 11/6/2009 7:43 AM, Attilio Rao wrote: >>> This patch introduces some mechanisms for collecting informations on >>> errors frequency and debugging (and relative sysctls for printing them >>> out) for the ahd(4) driver: >>> http://www.freebsd.org/~attilio/Sandvine/STABLE_8/ahd/ahd-current2.diff >>> >>> The usage of array for sysctls is a bit too paranoid but it allows for >>> further extendibility of the code and doesn't loose of cleaness. >>> This code has been contributed back by Sandvine Incorporated with some >>> cleanups. >>> Please review. >>> >>> Thanks, >>> Attilio >> >> In general, I think the patch is fine. It violates the existing style >> of the driver in some places (e.g. the aic7xxx drivers wrap function >> arguments to the opening '(' not to a 4 space indent), which should >> probably be addressed so the code remains consistent. > > Sorry, I cannot find where these existing style breakage happens, > could you be a bit more precise? > (I just found an un-sorted header introduction in aic79xx.h that I will fix). For example this code: + ahd->sysctl_tree[AHD_SYSCTL_ROOT] = SYSCTL_ADD_NODE( + &ahd->sysctl_ctx[AHD_SYSCTL_ROOT], SYSCTL_STATIC_CHILDREN(_hw), + OID_AUTO, device_get_nameunit(ahd->dev_softc), CTLFLAG_RD, + 0, ahd_sysctl_node_descriptions[AHD_SYSCTL_ROOT]); + SYSCTL_ADD_PROC(&ahd->sysctl_ctx[AHD_SYSCTL_ROOT], + SYSCTL_CHILDREN(ahd->sysctl_tree[AHD_SYSCTL_ROOT]), OID_AUTO, + "clear", CTLTYPE_UINT | CTLFLAG_RW, ahd, 0, ahd_clear_allcounters, + "IU", "Clear all counters"); should look like this, not style(9), to match the existing style of the driver: ahd->sysctl_tree[AHD_SYSCTL_ROOT] = SYSCTL_ADD_NODE(&ahd->sysctl_ctx[AHD_SYSCTL_ROOT], SYSCTL_STATIC_CHILDREN(_hw), OID_AUTO, device_get_nameunit(ahd->dev_softc), CTLFLAG_RD, 0, ahd_sysctl_node_descriptions[AHD_SYSCTL_ROOT]); SYSCTL_ADD_PROC(&ahd->sysctl_ctx[AHD_SYSCTL_ROOT], SYSCTL_CHILDREN(ahd->sysctl_tree[AHD_SYSCTL_ROOT]), OID_AUTO, "clear", CTLTYPE_UINT|CTLFLAG_RW, ahd, 0, ahd_clear_allcounters, "IU", "Clear all counters"); -- Justin