svn commit: r272925 - head/sys/kern

[John Baldwin]

On Saturday, October 11, 2014 02:02:59 AM Marcel Moolenaar wrote:
> Author: marcel
> Date: Sat Oct 11 02:02:58 2014
> New Revision: 272925
> URL: https://svnweb.freebsd.org/changeset/base/272925
> 
> Log:
>   Turn WITNESS_COUNT into a tunable and sysctl. This allows adjusting
>   the value without recompiling the kernel.  This is useful when
>   recompiling is not possible as an immediate solution. When we run out
>   of witness objects, witness is completely disabled. Not having an
>   immediate solution can therefore be problematic.
> 
>   Submitted by:	Sreekanth Rupavatharam <rupavath at juniper.net>
>   Obtained from:	Juniper Networks, Inc.

Thanks, just a few nits:

> Modified:
>   head/sys/kern/subr_witness.c
> 
> @@ -416,6 +414,13 @@ int	witness_skipspin = 0;
>  #endif
>  SYSCTL_INT(_debug_witness, OID_AUTO, skipspin, CTLFLAG_RDTUN,
> &witness_skipspin, 0, "");
> 
> +/* tunable for Witness count */
> +int witness_count = WITNESS_COUNT;
> +int badstack_sbuf_size = WITNESS_COUNT * 256;

This assignment isn't needed (it always gets overwritten).  Also, style-wise,
FreeBSD code tends to group a SYSCTL with its variable.  The comment also
just duplicates the code.

> +
> +SYSCTL_INT(_debug_witness, OID_AUTO, witness_count, CTLFLAG_RDTUN,
> +    &witness_count, 0, "");
> +
>  /*
>   * Call this to print out the relations between locks.
>   */
> @@ -450,7 +455,7 @@ SYSCTL_INT(_debug_witness, OID_AUTO, sle
>      "");
> 
>  static struct witness *w_data;
> -static uint8_t w_rmatrix[WITNESS_COUNT+1][WITNESS_COUNT+1];
> +static uint8_t **w_rmatrix;
>  static struct lock_list_entry w_locklistdata[LOCK_CHILDCOUNT];
>  static struct witness_hash w_hash;	/* The witness hash table. */
> 
> @@ -726,9 +731,18 @@ witness_initialize(void *dummy __unused)
>  	struct witness *w, *w1;
>  	int i;
> 
> -	w_data = malloc(sizeof (struct witness) * WITNESS_COUNT, M_WITNESS,
> +	w_data = malloc(sizeof (struct witness) * witness_count, M_WITNESS,
>  	    M_NOWAIT | M_ZERO);
> 
> +	w_rmatrix = malloc(sizeof(uint8_t *) * (witness_count+1),
> +	    M_WITNESS, M_NOWAIT | M_ZERO);
> +
> +	for(i = 0; i < witness_count+1; i++) {

Please use a space after 'for' and around '+' in 'witness_count + 1' (you do
Below).

> +		w_rmatrix[i] = malloc(sizeof(uint8_t) * (witness_count + 1),
> +		    M_WITNESS, M_NOWAIT | M_ZERO);

The malloc of w_rmatrix() used M_NOWAIT, but you didn't check its return
Value, so this will fault.  Either check the return value or use M_WAITOK.

> +	}
> +	badstack_sbuf_size = witness_count * 256;
> +
>  	/*
>  	 * We have to release Giant before initializing its witness
>  	 * structure so that WITNESS doesn't get confused.
> @@ -739,7 +753,7 @@ witness_initialize(void *dummy __unused)
>  	CTR1(KTR_WITNESS, "%s: initializing witness", __func__);
>  	mtx_init(&w_mtx, "witness lock", NULL, MTX_SPIN | MTX_QUIET |
>  	    MTX_NOWITNESS | MTX_NOPROFILE);
> -	for (i = WITNESS_COUNT - 1; i >= 0; i--) {
> +	for (i = witness_count - 1; i >= 0; i--) {
>  		w = &w_data[i];
>  		memset(w, 0, sizeof(*w));
>  		w_data[i].w_index = i;	/* Witness index never changes. */
> @@ -752,8 +766,10 @@ witness_initialize(void *dummy __unused)
>  	STAILQ_REMOVE_HEAD(&w_free, w_list);
>  	w_free_cnt--;
> 
> -	memset(w_rmatrix, 0,
> -	    (sizeof(**w_rmatrix) * (WITNESS_COUNT+1) * (WITNESS_COUNT+1)));
> +	for(i = 0; i < witness_count; i++) {
> +		memset(w_rmatrix[i], 0, sizeof(uint8_t) *
> +		    (witness_count + 1));
> +	}

More 'space after for'.  Also, 'sizeof(*w_rmatrix[i])' would more closely
match the old code.  Using 'sizeof(uint8_t)' means you still have to fix
this if you change the base matrix type later.

-- 
John Baldwin