svn commit: r226536 - head/sys/contrib/pf/net

[Ermal Luçi]

On Wed, Oct 19, 2011 at 1:04 PM, Bjoern A. Zeeb <bz at freebsd.org> wrote:
> Author: bz
> Date: Wed Oct 19 11:04:49 2011
> New Revision: 226536
> URL: http://svn.freebsd.org/changeset/base/226536
>
> Log:
>  De-virtualize the pf_task_mtx lock.  At the current state of pf locking
>  and 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!


>
>  Current state of art is to not virtualize these kinds of locks -
>  inp_group/hash/info/.. are all not virtualized either.
>
>  MFC after:    3 days
>
> Modified:
>  head/sys/contrib/pf/net/pf_ioctl.c
>  head/sys/contrib/pf/net/pfvar.h
>
> Modified: head/sys/contrib/pf/net/pf_ioctl.c
> ==============================================================================
> --- head/sys/contrib/pf/net/pf_ioctl.c  Wed Oct 19 10:16:42 2011        (r226535)
> +++ head/sys/contrib/pf/net/pf_ioctl.c  Wed Oct 19 11:04:49 2011        (r226536)
> @@ -266,7 +266,7 @@ static struct cdevsw pf_cdevsw = {
>  static volatile VNET_DEFINE(int, pf_pfil_hooked);
>  #define V_pf_pfil_hooked       VNET(pf_pfil_hooked)
>  VNET_DEFINE(int,               pf_end_threads);
> -VNET_DEFINE(struct mtx,                pf_task_mtx);
> +struct mtx                     pf_task_mtx;
>
>  /* pfsync */
>  pfsync_state_import_t          *pfsync_state_import_ptr = NULL;
> @@ -287,18 +287,18 @@ SYSCTL_VNET_INT(_debug, OID_AUTO, pfugid
>        &VNET_NAME(debug_pfugidhack), 0,
>        "Enable/disable pf user/group rules mpsafe hack");
>
> -void
> +static void
>  init_pf_mutex(void)
>  {
>
> -       mtx_init(&V_pf_task_mtx, "pf task mtx", NULL, MTX_DEF);
> +       mtx_init(&pf_task_mtx, "pf task mtx", NULL, MTX_DEF);
>  }
>
> -void
> +static void
>  destroy_pf_mutex(void)
>  {
>
> -       mtx_destroy(&V_pf_task_mtx);
> +       mtx_destroy(&pf_task_mtx);
>  }
>  void
>  init_zone_var(void)
> @@ -4381,11 +4381,8 @@ pf_load(void)
>
>        init_zone_var();
>        sx_init(&V_pf_consistency_lock, "pf_statetbl_lock");
> -       init_pf_mutex();
> -       if (pfattach() < 0) {
> -               destroy_pf_mutex();
> +       if (pfattach() < 0)
>                return (ENOMEM);
> -       }
>
>        return (0);
>  }
> @@ -4413,14 +4410,13 @@ pf_unload(void)
>        V_pf_end_threads = 1;
>        while (V_pf_end_threads < 2) {
>                wakeup_one(pf_purge_thread);
> -               msleep(pf_purge_thread, &V_pf_task_mtx, 0, "pftmo", hz);
> +               msleep(pf_purge_thread, &pf_task_mtx, 0, "pftmo", hz);
>        }
>        pfi_cleanup();
>        pf_osfp_flush();
>        pf_osfp_cleanup();
>        cleanup_pf_zone();
>        PF_UNLOCK();
> -       destroy_pf_mutex();
>        sx_destroy(&V_pf_consistency_lock);
>        return error;
>  }
> @@ -4432,10 +4428,12 @@ pf_modevent(module_t mod, int type, void
>
>        switch(type) {
>        case MOD_LOAD:
> +               init_pf_mutex();
>                pf_dev = make_dev(&pf_cdevsw, 0, 0, 0, 0600, PF_NAME);
>                break;
>        case MOD_UNLOAD:
>                destroy_dev(pf_dev);
> +               destroy_pf_mutex();
>                break;
>        default:
>                error = EINVAL;
>
> Modified: head/sys/contrib/pf/net/pfvar.h
> ==============================================================================
> --- head/sys/contrib/pf/net/pfvar.h     Wed Oct 19 10:16:42 2011        (r226535)
> +++ head/sys/contrib/pf/net/pfvar.h     Wed Oct 19 11:04:49 2011        (r226536)
> @@ -237,19 +237,18 @@ struct pfi_dynaddr {
>                uma_zdestroy(var)
>
>  #ifdef __FreeBSD__
> -VNET_DECLARE(struct mtx,        pf_task_mtx);
> -#define        V_pf_task_mtx            VNET(pf_task_mtx)
> +extern struct mtx pf_task_mtx;
>
> -#define        PF_LOCK_ASSERT()        mtx_assert(&V_pf_task_mtx, MA_OWNED)
> -#define        PF_UNLOCK_ASSERT()      mtx_assert(&V_pf_task_mtx, MA_NOTOWNED)
> +#define        PF_LOCK_ASSERT()        mtx_assert(&pf_task_mtx, MA_OWNED)
> +#define        PF_UNLOCK_ASSERT()      mtx_assert(&pf_task_mtx, MA_NOTOWNED)
>
>  #define        PF_LOCK()       do {                            \
>        PF_UNLOCK_ASSERT();                             \
> -       mtx_lock(&V_pf_task_mtx);                       \
> +       mtx_lock(&pf_task_mtx);                         \
>  } while(0)
>  #define        PF_UNLOCK()     do {                            \
>        PF_LOCK_ASSERT();                               \
> -       mtx_unlock(&V_pf_task_mtx);                     \
> +       mtx_unlock(&pf_task_mtx);                       \
>  } while(0)
>  #else
>  #define        PF_LOCK_ASSERT()
> @@ -270,9 +269,6 @@ VNET_DECLARE(struct mtx,     pf_task_mtx);
>        PF_LOCK();                                      \
>  } while(0)
>
> -extern void init_pf_mutex(void);
> -extern void destroy_pf_mutex(void);
> -
>  #define        PF_MODVER       1
>  #define        PFLOG_MODVER    1
>  #define        PFSYNC_MODVER   1
>



-- 
Ermal