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 *
> 

Reply via email to