Re: vm_page_t related KBI [Was: Re: panic at vm_page_wire with FreeBSD 9.0 Beta 3]

Hi,

On Mon, Nov 7, 2011 at 2:35 PM, Kostik Belousov <kostikbel_at_gmail.com> wrote:
> On Mon, Nov 07, 2011 at 11:45:38AM -0600, Alan Cox wrote:
>> Ok.  I'll offer one final suggestion.  Please consider an alternative
>> suffix to "func".  Perhaps, "kbi" or "KBI".  In other words, something
>> that hints at the function's reason for existing.
>
> Sure. Below is the extraction of only vm_page_lock() bits, together
> with the suggested rename. When Attilio provides the promised simplification
> of the mutex KPI, this can be reduced.
>
If you want to be pedantic, you also must hide the definition of
`struct vm_page' when KLD_MODULE is defined. You want to be sure no
one is gonna mess with the internal of the structure (ie. either
directly dereference fields, compute size or any offset...)  and will
have no other choice but to use assessors.

Maybe you are addressing this in another patch.

 - Arnaud

> diff --git a/sys/vm/vm_page.c b/sys/vm/vm_page.c
> index 389aea5..ea4ea34 100644
> --- a/sys/vm/vm_page.c
> +++ b/sys/vm/vm_page.c
> _at__at_ -2677,6 +2677,44 _at__at_ vm_page_test_dirty(vm_page_t m)
>                vm_page_dirty(m);
>  }
>
> +void
> +vm_page_lock_KBI(vm_page_t m, const char *file, int line)
> +{
> +
> +#if LOCK_DEBUG > 0 || defined(MUTEX_NOINLINE)
> +       _mtx_lock_flags(vm_page_lockptr(m), 0, file, line);
> +#else
> +       __mtx_lock(vm_page_lockptr(m), 0, file, line);
> +#endif
> +}
> +
> +void
> +vm_page_unlock_KBI(vm_page_t m, const char *file, int line)
> +{
> +
> +#if LOCK_DEBUG > 0 || defined(MUTEX_NOINLINE)
> +       _mtx_unlock_flags(vm_page_lockptr(m), 0, file, line);
> +#else
> +       __mtx_unlock(vm_page_lockptr(m), curthread, 0, file, line);
> +#endif
> +}
> +
> +int
> +vm_page_trylock_KBI(vm_page_t m, const char *file, int line)
> +{
> +
> +       return (_mtx_trylock(vm_page_lockptr(m), 0, file, line));
> +}
> +
> +void
> +vm_page_lock_assert_KBI(vm_page_t m, int a, const char *file, int line)
> +{
> +
> +#ifdef INVARIANTS
> +       _mtx_assert(vm_page_lockptr(m), a, file, line);
> +#endif
> +}
> +
>  int so_zerocp_fullpage = 0;
>
>  /*
> diff --git a/sys/vm/vm_page.h b/sys/vm/vm_page.h
> index 7099b70..a5604b7 100644
> --- a/sys/vm/vm_page.h
> +++ b/sys/vm/vm_page.h
> _at__at_ -218,11 +218,23 _at__at_ extern struct vpglocks pa_lock[];
>
>  #define        PA_LOCK_ASSERT(pa, a)   mtx_assert(PA_LOCKPTR(pa), (a))
>
> +#ifdef KLD_MODULE
> +#define        vm_page_lock(m)         vm_page_lock_KBI((m), LOCK_FILE, LOCK_LINE)
> +#define        vm_page_unlock(m)       vm_page_unlock_KBI((m), LOCK_FILE, LOCK_LINE)
> +#define        vm_page_trylock(m)      vm_page_trylock_KBI((m), LOCK_FILE, LOCK_LINE)
> +#ifdef INVARIANTS
> +#define        vm_page_lock_assert(m, a)       \
> +    vm_page_lock_assert_KBI((m), (a), LOCK_FILE, LOCK_LINE)
> +#else
> +#define        vm_page_lock_assert(m, a)
> +#endif
> +#else  /* KLD_MODULE */
>  #define        vm_page_lockptr(m)      (PA_LOCKPTR(VM_PAGE_TO_PHYS((m))))
>  #define        vm_page_lock(m)         mtx_lock(vm_page_lockptr((m)))
>  #define        vm_page_unlock(m)       mtx_unlock(vm_page_lockptr((m)))
>  #define        vm_page_trylock(m)      mtx_trylock(vm_page_lockptr((m)))
>  #define        vm_page_lock_assert(m, a)       mtx_assert(vm_page_lockptr((m)), (a))
> +#endif
>
>  #define        vm_page_queue_free_mtx  vm_page_queue_free_lock.data
>  /*
> _at__at_ -403,6 +415,11 _at__at_ void vm_page_cowfault (vm_page_t);
>  int vm_page_cowsetup(vm_page_t);
>  void vm_page_cowclear (vm_page_t);
>
> +void vm_page_lock_KBI(vm_page_t m, const char *file, int line);
> +void vm_page_unlock_KBI(vm_page_t m, const char *file, int line);
> +int vm_page_trylock_KBI(vm_page_t m, const char *file, int line);
> +void vm_page_lock_assert_KBI(vm_page_t m, int a, const char *file, int line);
> +
>  #ifdef INVARIANTS
>  void vm_page_object_lock_assert(vm_page_t m);
>  #define        VM_PAGE_OBJECT_LOCK_ASSERT(m)   vm_page_object_lock_assert(m)
>