From owner-svn-src-all@FreeBSD.ORG Fri Oct 21 19:24:05 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 824D3106566C; Fri, 21 Oct 2011 19:24:05 +0000 (UTC) (envelope-from ermal.luci@gmail.com) Received: from mail-iy0-f182.google.com (mail-iy0-f182.google.com [209.85.210.182]) by mx1.freebsd.org (Postfix) with ESMTP id 1F8B68FC0C; Fri, 21 Oct 2011 19:24:04 +0000 (UTC) Received: by iaky10 with SMTP id y10so6287976iak.13 for ; Fri, 21 Oct 2011 12:24:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=1xMKvmowJZIMOoXObfPAm1JFMipMrKY3SfLRezK2+7Y=; b=wo37feE/MMe23TL1w8gKj5mR0kPg0Fncdcy9ZYH30v1nrg/T58zkFTsfcrSpFLmbv2 fVIZI9Wbls03IB/JrOs5HVJwAU4pHoltHXw8vpV4DKWd1aaQOkPDqpkw6YVoUKrsk12h bnWa2oSF4nY8cpQfUGuDBPgN6lpyYt0lTyTBM= MIME-Version: 1.0 Received: by 10.231.69.146 with SMTP id z18mr6089076ibi.79.1319223467163; Fri, 21 Oct 2011 11:57:47 -0700 (PDT) Sender: ermal.luci@gmail.com Received: by 10.231.53.213 with HTTP; Fri, 21 Oct 2011 11:57:47 -0700 (PDT) In-Reply-To: <201110191104.p9JB4nlK021378@svn.freebsd.org> References: <201110191104.p9JB4nlK021378@svn.freebsd.org> Date: Fri, 21 Oct 2011 20:57:47 +0200 X-Google-Sender-Auth: tIv0lPO9hepT8ECJnopgHwjRkmk Message-ID: From: =?ISO-8859-1?Q?Ermal_Lu=E7i?= To: "Bjoern A. Zeeb" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r226536 - head/sys/contrib/pf/net X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Oct 2011 19:24:05 -0000 On Wed, Oct 19, 2011 at 1:04 PM, Bjoern A. Zeeb wrote: > Author: bz > Date: Wed Oct 19 11:04:49 2011 > New Revision: 226536 > URL: http://svn.freebsd.org/changeset/base/226536 > > Log: > =A0De-virtualize the pf_task_mtx lock. =A0At the current state of pf lock= ing > =A0and virtualization it is not helpful but complicates things. I would disagree with this since its a step backwards and different direction with pf(4) code in general. The patch to actually fix it for vimage enabled kernels was simpler! > > =A0Current state of art is to not virtualize these kinds of locks - > =A0inp_group/hash/info/.. are all not virtualized either. > > =A0MFC after: =A0 =A03 days > > Modified: > =A0head/sys/contrib/pf/net/pf_ioctl.c > =A0head/sys/contrib/pf/net/pfvar.h > > Modified: head/sys/contrib/pf/net/pf_ioctl.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/contrib/pf/net/pf_ioctl.c =A0Wed Oct 19 10:16:42 2011 =A0 = =A0 =A0 =A0(r226535) > +++ head/sys/contrib/pf/net/pf_ioctl.c =A0Wed Oct 19 11:04:49 2011 =A0 = =A0 =A0 =A0(r226536) > @@ -266,7 +266,7 @@ static struct cdevsw pf_cdevsw =3D { > =A0static volatile VNET_DEFINE(int, pf_pfil_hooked); > =A0#define V_pf_pfil_hooked =A0 =A0 =A0 VNET(pf_pfil_hooked) > =A0VNET_DEFINE(int, =A0 =A0 =A0 =A0 =A0 =A0 =A0 pf_end_threads); > -VNET_DEFINE(struct mtx, =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pf_task_mtx); > +struct mtx =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pf_task_mtx; > > =A0/* pfsync */ > =A0pfsync_state_import_t =A0 =A0 =A0 =A0 =A0*pfsync_state_import_ptr =3D = NULL; > @@ -287,18 +287,18 @@ SYSCTL_VNET_INT(_debug, OID_AUTO, pfugid > =A0 =A0 =A0 =A0&VNET_NAME(debug_pfugidhack), 0, > =A0 =A0 =A0 =A0"Enable/disable pf user/group rules mpsafe hack"); > > -void > +static void > =A0init_pf_mutex(void) > =A0{ > > - =A0 =A0 =A0 mtx_init(&V_pf_task_mtx, "pf task mtx", NULL, MTX_DEF); > + =A0 =A0 =A0 mtx_init(&pf_task_mtx, "pf task mtx", NULL, MTX_DEF); > =A0} > > -void > +static void > =A0destroy_pf_mutex(void) > =A0{ > > - =A0 =A0 =A0 mtx_destroy(&V_pf_task_mtx); > + =A0 =A0 =A0 mtx_destroy(&pf_task_mtx); > =A0} > =A0void > =A0init_zone_var(void) > @@ -4381,11 +4381,8 @@ pf_load(void) > > =A0 =A0 =A0 =A0init_zone_var(); > =A0 =A0 =A0 =A0sx_init(&V_pf_consistency_lock, "pf_statetbl_lock"); > - =A0 =A0 =A0 init_pf_mutex(); > - =A0 =A0 =A0 if (pfattach() < 0) { > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 destroy_pf_mutex(); > + =A0 =A0 =A0 if (pfattach() < 0) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (ENOMEM); > - =A0 =A0 =A0 } > > =A0 =A0 =A0 =A0return (0); > =A0} > @@ -4413,14 +4410,13 @@ pf_unload(void) > =A0 =A0 =A0 =A0V_pf_end_threads =3D 1; > =A0 =A0 =A0 =A0while (V_pf_end_threads < 2) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0wakeup_one(pf_purge_thread); > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 msleep(pf_purge_thread, &V_pf_task_mtx, 0, = "pftmo", hz); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 msleep(pf_purge_thread, &pf_task_mtx, 0, "p= ftmo", hz); > =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0pfi_cleanup(); > =A0 =A0 =A0 =A0pf_osfp_flush(); > =A0 =A0 =A0 =A0pf_osfp_cleanup(); > =A0 =A0 =A0 =A0cleanup_pf_zone(); > =A0 =A0 =A0 =A0PF_UNLOCK(); > - =A0 =A0 =A0 destroy_pf_mutex(); > =A0 =A0 =A0 =A0sx_destroy(&V_pf_consistency_lock); > =A0 =A0 =A0 =A0return error; > =A0} > @@ -4432,10 +4428,12 @@ pf_modevent(module_t mod, int type, void > > =A0 =A0 =A0 =A0switch(type) { > =A0 =A0 =A0 =A0case MOD_LOAD: > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 init_pf_mutex(); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pf_dev =3D make_dev(&pf_cdevsw, 0, 0, 0, 0= 600, PF_NAME); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break; > =A0 =A0 =A0 =A0case MOD_UNLOAD: > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0destroy_dev(pf_dev); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 destroy_pf_mutex(); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break; > =A0 =A0 =A0 =A0default: > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0error =3D EINVAL; > > Modified: head/sys/contrib/pf/net/pfvar.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/contrib/pf/net/pfvar.h =A0 =A0 Wed Oct 19 10:16:42 2011 =A0 = =A0 =A0 =A0(r226535) > +++ head/sys/contrib/pf/net/pfvar.h =A0 =A0 Wed Oct 19 11:04:49 2011 =A0 = =A0 =A0 =A0(r226536) > @@ -237,19 +237,18 @@ struct pfi_dynaddr { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0uma_zdestroy(var) > > =A0#ifdef __FreeBSD__ > -VNET_DECLARE(struct mtx, =A0 =A0 =A0 =A0pf_task_mtx); > -#define =A0 =A0 =A0 =A0V_pf_task_mtx =A0 =A0 =A0 =A0 =A0 =A0VNET(pf_task= _mtx) > +extern struct mtx pf_task_mtx; > > -#define =A0 =A0 =A0 =A0PF_LOCK_ASSERT() =A0 =A0 =A0 =A0mtx_assert(&V_pf_= task_mtx, MA_OWNED) > -#define =A0 =A0 =A0 =A0PF_UNLOCK_ASSERT() =A0 =A0 =A0mtx_assert(&V_pf_ta= sk_mtx, MA_NOTOWNED) > +#define =A0 =A0 =A0 =A0PF_LOCK_ASSERT() =A0 =A0 =A0 =A0mtx_assert(&pf_ta= sk_mtx, MA_OWNED) > +#define =A0 =A0 =A0 =A0PF_UNLOCK_ASSERT() =A0 =A0 =A0mtx_assert(&pf_task= _mtx, MA_NOTOWNED) > > =A0#define =A0 =A0 =A0 =A0PF_LOCK() =A0 =A0 =A0 do { =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0PF_UNLOCK_ASSERT(); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 \ > - =A0 =A0 =A0 mtx_lock(&V_pf_task_mtx); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 \ > + =A0 =A0 =A0 mtx_lock(&pf_task_mtx); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 \ > =A0} while(0) > =A0#define =A0 =A0 =A0 =A0PF_UNLOCK() =A0 =A0 do { =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0PF_LOCK_ASSERT(); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 \ > - =A0 =A0 =A0 mtx_unlock(&V_pf_task_mtx); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 \ > + =A0 =A0 =A0 mtx_unlock(&pf_task_mtx); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 \ > =A0} while(0) > =A0#else > =A0#define =A0 =A0 =A0 =A0PF_LOCK_ASSERT() > @@ -270,9 +269,6 @@ VNET_DECLARE(struct mtx, =A0 =A0 pf_task_mtx); > =A0 =A0 =A0 =A0PF_LOCK(); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0} while(0) > > -extern void init_pf_mutex(void); > -extern void destroy_pf_mutex(void); > - > =A0#define =A0 =A0 =A0 =A0PF_MODVER =A0 =A0 =A0 1 > =A0#define =A0 =A0 =A0 =A0PFLOG_MODVER =A0 =A01 > =A0#define =A0 =A0 =A0 =A0PFSYNC_MODVER =A0 1 > --=20 Ermal