Re: tmpfs module and pool allocator

[Bernd Ernesti <netbsd%lists.veego.de@localhost>]

On Wed, Dec 30, 2009 at 01:03:50PM -0500, Elad Efrat wrote:
> 
> >I've "fixed it" by explicitly calling mutex_destroy() in
> >tmpfs_modcmd():MODULE_CMD_FINI, but I think the right fix would be to
> >set a flag indicating that the pool allocator used is not a "system
> >allocator" (or that it is a "custom allocator", or whatever) and
> >should be destroyed as well, or at least its mutex...
> 
> Matt@ suggested we use a reference count on the pool allocator instead
> of the PA_INITIALIZED flag, and when it drops to zero, pool_destroy can
> destroy the mutex as well.
> 
> I've attached a diff that supposedly implements it... But I don't know
> if it's correct. It fixes the problem for me, on a GEENRIC kernel built
> with DIAGNOSTIC and LOCKDEBUG.
> 
> Please have a look and let me know if it's okay to commit.

I can't comment if it is okay or not, but see below.

> Index: sys/pool.h
> ===================================================================
> RCS file: /cvsroot/src/sys/sys/pool.h,v
> retrieving revision 1.67
> diff -u -p -r1.67 pool.h
> --- sys/pool.h        15 Oct 2009 20:50:12 -0000      1.67
> +++ sys/pool.h        30 Dec 2009 17:58:17 -0000
> @@ -64,7 +64,7 @@ struct pool_allocator {
>       kmutex_t        pa_lock;
>       TAILQ_HEAD(, pool) pa_list;     /* list of pools using this allocator */
>       int             pa_flags;
> -#define      PA_INITIALIZED  0x01
> +     uint32_t        pa_refcnt;      /* number of pools using this allocator 
> */
>       int             pa_pagemask;
>       int             pa_pageshift;
>       struct vm_map *pa_backingmap;

You are adding a new field in the middle of a struct.
Is that really ok?

Bernd