Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 13 Oct 2013 15:26:12 -0600
From:      "Justin T. Gibbs" <gibbs@FreeBSD.org>
To:        Alexander Motin <mav@FreeBSD.org>
Cc:        svn-src-projects@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r256295 - in projects/camlock/sys: geom kern
Message-ID:  <2A8C3ACD-A37E-48CB-9103-DA359B3B0536@FreeBSD.org>
In-Reply-To: <201310102003.r9AK3smQ038364@svn.freebsd.org>
References:  <201310102003.r9AK3smQ038364@svn.freebsd.org>

index | next in thread | previous in thread | raw e-mail

On Oct 10, 2013, at 2:03 PM, Alexander Motin <mav@FreeBSD.org> wrote:

> Author: mav
> Date: Thu Oct 10 20:03:54 2013
> New Revision: 256295
> URL: http://svnweb.freebsd.org/changeset/base/256295
> 
> Log:
>  Use the same satistics for disk and its GEOM provider.
> 
>  Avoiding double accounting allows to reduce CPU load on I/O, especially on
>  machines with slow timecounter.  As side effect it also makes gstat show
>  statistics for raw disk providers even when kern.geom.collectstats is set
>  to zero, making that mode more usable.

…

> Modified: projects/camlock/sys/geom/geom_io.c
> ==============================================================================
> --- projects/camlock/sys/geom/geom_io.c	Thu Oct 10 19:56:31 2013	(r256294)
> +++ projects/camlock/sys/geom/geom_io.c	Thu Oct 10 20:03:54 2013	(r256295)
> @@ -510,7 +510,7 @@ g_io_request(struct bio *bp, struct g_co
> 
> 	KASSERT(!(bp->bio_flags & BIO_ONQUEUE),
> 	    ("Bio already on queue bp=%p", bp));
> -	if (g_collectstats)
> +	if ((g_collectstats & ~(pp->stat ? 0 : 1)) != 0)
> 		binuptime(&bp->bio_t0);
> 	else
> 		getbinuptime(&bp->bio_t0);


This is pretty obscure and further compounds the original bug that
there is no enum defining the bits in g_collectstats.  Can you
please:

1) Add an enum with constants for the two bits in g_collectstats
   with nice comments for what they do.

2) Use those constants everywhere and change your original logic to make it
   more readable?

e.g.

	if ((g_collecstats & G_CONSUMER_STATS) != 0 ||
	    ((g_collecstats & G_PROVIDER_STATS) != 0 && pp->stat != NULL))

I would bet the compiler optimizes this just as well as your original logic.

Thanks,
Justin



help

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2A8C3ACD-A37E-48CB-9103-DA359B3B0536>