Re: [PATCH] Adding sysctl for errors statistics to ahd(4)

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.

My only real complaint is that there is not some generic reporting
mechanism for these types of statistics.  The ahd chips are legacy.
Will Sandvine or vendor X add similar but slightly different sysctls
to the next driver they need to integrate into their product?  Higher
level monitoring software can't be written to be agnostic of the storage
controller and thus survive largely untouched as the HW under the software
changes.  I don't expect Sandvine to address this, but the project should.
In the mean time, I'm okay with the patch going in since it is obviously
useful to at least one company.

--
Justin