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 <rupav...@juniper.net>
> Obtained from: Juniper Networks, Inc.
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
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
