On Mon, Apr 08, 2013 at 10:05:30PM -0400, Ted Unangst wrote:
> I was given a scare today by an unnamed party because calling
> pool_init twice causes bad things to happen. And whenever bad things
> happen in pool, I get blamed. To turn future tedu panics into kernel
> panics, here's a diff.
>
> Whenever pool_init is called, we check the list of existing
> pools to make sure it's not already there. Along similar lines,
> shuffle the pool_destroy code around to add the reverse check and do
> so before touching any of the fields inside the pool.
Tested on i386. Okay.
>
> Index: subr_pool.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/subr_pool.c,v
> retrieving revision 1.118
> diff -u -p -r1.118 subr_pool.c
> --- subr_pool.c 6 Apr 2013 13:41:11 -0000 1.118
> +++ subr_pool.c 9 Apr 2013 01:56:35 -0000
> @@ -244,6 +244,14 @@ pool_init(struct pool *pp, size_t size,
> const char *wchan, struct pool_allocator *palloc)
> {
> int off, slack;
> +#ifdef DIAGNOSTIC
> + struct pool *iter;
> +
> + SIMPLEQ_FOREACH(iter, &pool_head, pr_poollist) {
> + if (iter == pp)
> + panic("init pool already on list");
> + }
> +#endif
>
> #ifdef MALLOC_DEBUG
> if ((flags & PR_DEBUG) && (ioff != 0 || align != 0))
> @@ -417,17 +425,6 @@ pool_destroy(struct pool *pp)
> struct pool_item_header *ph;
> struct pool *prev, *iter;
>
> -#ifdef DIAGNOSTIC
> - if (pp->pr_nout != 0)
> - panic("pool_destroy: pool busy: still out: %u", pp->pr_nout);
> -#endif
> -
> - /* Remove all pages */
> - while ((ph = LIST_FIRST(&pp->pr_emptypages)) != NULL)
> - pr_rmpage(pp, ph, NULL);
> - KASSERT(LIST_EMPTY(&pp->pr_fullpages));
> - KASSERT(LIST_EMPTY(&pp->pr_partpages));
> -
> /* Remove from global pool list */
> if (pp == SIMPLEQ_FIRST(&pool_head))
> SIMPLEQ_REMOVE_HEAD(&pool_head, pr_poollist);
> @@ -437,11 +434,26 @@ pool_destroy(struct pool *pp)
> if (iter == pp) {
> SIMPLEQ_REMOVE_AFTER(&pool_head, prev,
> pr_poollist);
> - break;
> + goto removed;
> }
> prev = iter;
> }
> +#ifdef DIAGNOSTIC
> + panic("destroyed pool not on list");
> +#endif
> }
> +removed:
> +#ifdef DIAGNOSTIC
> + if (pp->pr_nout != 0)
> + panic("pool_destroy: pool busy: still out: %u", pp->pr_nout);
> +#endif
> +
> + /* Remove all pages */
> + while ((ph = LIST_FIRST(&pp->pr_emptypages)) != NULL)
> + pr_rmpage(pp, ph, NULL);
> + KASSERT(LIST_EMPTY(&pp->pr_fullpages));
> + KASSERT(LIST_EMPTY(&pp->pr_partpages));
> +
> }
>
> struct pool_item_header *
>