Date: Mon, 7 Nov 2011 15:00:49 -0500 From: Arnaud Lacombe <lacombar@gmail.com> To: Kostik Belousov <kostikbel@gmail.com> Cc: mdf@freebsd.org, "K. Macy" <kmacy@freebsd.org>, Alan Cox <alc@rice.edu>, Andriy Gapon <avg@freebsd.org>, freebsd-current@freebsd.org, Benjamin Kaduk <kaduk@mit.edu>, Penta Upa <bsdboot@gmail.com> Subject: Re: vm_page_t related KBI [Was: Re: panic at vm_page_wire with FreeBSD 9.0 Beta 3] Message-ID: <CACqU3MVpeoG25yc--T47_WsGNaPhHu9FHG6pFSccA5KQfPa=qA@mail.gmail.com> In-Reply-To: <20111107193516.GA50300@deviant.kiev.zoral.com.ua> References: <4EB40015.5040100@rice.edu> <20111104153004.GK50300@deviant.kiev.zoral.com.ua> <4EB4095D.3030303@rice.edu> <20111104160339.GM50300@deviant.kiev.zoral.com.ua> <20111105141306.GW50300@deviant.kiev.zoral.com.ua> <CAMBSHm86TaJnRRgmPA_t7tiPfQsPyoTqz3ymdHSY1H3t5G864Q@mail.gmail.com> <20111105151530.GX50300@deviant.kiev.zoral.com.ua> <4EB595FA.4020500@rice.edu> <20111106124331.GP50300@deviant.kiev.zoral.com.ua> <4EB81942.70501@rice.edu> <20111107193516.GA50300@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi,
On Mon, Nov 7, 2011 at 2:35 PM, Kostik Belousov <kostikbel@gmail.com> wrote=
:
> On Mon, Nov 07, 2011 at 11:45:38AM -0600, Alan Cox wrote:
>> Ok. =A0I'll offer one final suggestion. =A0Please consider an alternativ=
e
>> suffix to "func". =A0Perhaps, "kbi" or "KBI". =A0In other words, somethi=
ng
>> 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 simplificat=
ion
> 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
> @@ -2677,6 +2677,44 @@ vm_page_test_dirty(vm_page_t m)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vm_page_dirty(m);
> =A0}
>
> +void
> +vm_page_lock_KBI(vm_page_t m, const char *file, int line)
> +{
> +
> +#if LOCK_DEBUG > 0 || defined(MUTEX_NOINLINE)
> + =A0 =A0 =A0 _mtx_lock_flags(vm_page_lockptr(m), 0, file, line);
> +#else
> + =A0 =A0 =A0 __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)
> + =A0 =A0 =A0 _mtx_unlock_flags(vm_page_lockptr(m), 0, file, line);
> +#else
> + =A0 =A0 =A0 __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)
> +{
> +
> + =A0 =A0 =A0 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
> + =A0 =A0 =A0 _mtx_assert(vm_page_lockptr(m), a, file, line);
> +#endif
> +}
> +
> =A0int so_zerocp_fullpage =3D 0;
>
> =A0/*
> 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
> @@ -218,11 +218,23 @@ extern struct vpglocks pa_lock[];
>
> =A0#define =A0 =A0 =A0 =A0PA_LOCK_ASSERT(pa, a) =A0 mtx_assert(PA_LOCKPTR=
(pa), (a))
>
> +#ifdef KLD_MODULE
> +#define =A0 =A0 =A0 =A0vm_page_lock(m) =A0 =A0 =A0 =A0 vm_page_lock_KBI(=
(m), LOCK_FILE, LOCK_LINE)
> +#define =A0 =A0 =A0 =A0vm_page_unlock(m) =A0 =A0 =A0 vm_page_unlock_KBI(=
(m), LOCK_FILE, LOCK_LINE)
> +#define =A0 =A0 =A0 =A0vm_page_trylock(m) =A0 =A0 =A0vm_page_trylock_KBI=
((m), LOCK_FILE, LOCK_LINE)
> +#ifdef INVARIANTS
> +#define =A0 =A0 =A0 =A0vm_page_lock_assert(m, a) =A0 =A0 =A0 \
> + =A0 =A0vm_page_lock_assert_KBI((m), (a), LOCK_FILE, LOCK_LINE)
> +#else
> +#define =A0 =A0 =A0 =A0vm_page_lock_assert(m, a)
> +#endif
> +#else =A0/* KLD_MODULE */
> =A0#define =A0 =A0 =A0 =A0vm_page_lockptr(m) =A0 =A0 =A0(PA_LOCKPTR(VM_PA=
GE_TO_PHYS((m))))
> =A0#define =A0 =A0 =A0 =A0vm_page_lock(m) =A0 =A0 =A0 =A0 mtx_lock(vm_pag=
e_lockptr((m)))
> =A0#define =A0 =A0 =A0 =A0vm_page_unlock(m) =A0 =A0 =A0 mtx_unlock(vm_pag=
e_lockptr((m)))
> =A0#define =A0 =A0 =A0 =A0vm_page_trylock(m) =A0 =A0 =A0mtx_trylock(vm_pa=
ge_lockptr((m)))
> =A0#define =A0 =A0 =A0 =A0vm_page_lock_assert(m, a) =A0 =A0 =A0 mtx_asser=
t(vm_page_lockptr((m)), (a))
> +#endif
>
> =A0#define =A0 =A0 =A0 =A0vm_page_queue_free_mtx =A0vm_page_queue_free_lo=
ck.data
> =A0/*
> @@ -403,6 +415,11 @@ void vm_page_cowfault (vm_page_t);
> =A0int vm_page_cowsetup(vm_page_t);
> =A0void 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 l=
ine);
> +
> =A0#ifdef INVARIANTS
> =A0void vm_page_object_lock_assert(vm_page_t m);
> =A0#define =A0 =A0 =A0 =A0VM_PAGE_OBJECT_LOCK_ASSERT(m) =A0 vm_page_objec=
t_lock_assert(m)
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACqU3MVpeoG25yc--T47_WsGNaPhHu9FHG6pFSccA5KQfPa=qA>