From owner-freebsd-current@FreeBSD.ORG Mon Nov 7 20:00:52 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1A3481065674; Mon, 7 Nov 2011 20:00:52 +0000 (UTC) (envelope-from lacombar@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 1D4E18FC15; Mon, 7 Nov 2011 20:00:50 +0000 (UTC) Received: by wyg36 with SMTP id 36so6619258wyg.13 for ; Mon, 07 Nov 2011 12:00:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=qdWPIJ7KwLK5EhXrFYU82B7bN/RbLQ/74oDYO4jIars=; b=HL07IigEZ0FOexWBLN0rkB9ztI6s9uraOW5Zwt0yKSWBgt8zNegGrafI3Dw7mDGRq7 FdyJR0PE04JFVlUBfpP9DXJ4ptx4YgoOeOmQ0W+tXDPRFmdwjEWIfCUzBA/Is3EkK5zo pSbcPYSntvwzViMX7Hl6T/HOvg7X7XtxQ7jrs= MIME-Version: 1.0 Received: by 10.180.90.148 with SMTP id bw20mr9670738wib.33.1320696049963; Mon, 07 Nov 2011 12:00:49 -0800 (PST) Received: by 10.180.81.200 with HTTP; Mon, 7 Nov 2011 12:00:49 -0800 (PST) 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> <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> Date: Mon, 7 Nov 2011 15:00:49 -0500 Message-ID: From: Arnaud Lacombe To: Kostik Belousov Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: mdf@freebsd.org, "K. Macy" , Alan Cox , Andriy Gapon , freebsd-current@freebsd.org, Benjamin Kaduk , Penta Upa Subject: Re: vm_page_t related KBI [Was: Re: panic at vm_page_wire with FreeBSD 9.0 Beta 3] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Nov 2011 20:00:52 -0000 Hi, On Mon, Nov 7, 2011 at 2:35 PM, Kostik Belousov 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) >