On Saturday 24 November 2007, Max Laier wrote:
> On Friday 23 November 2007, Robert Watson wrote:
> > On Fri, 23 Nov 2007, Max Laier wrote:
> > > attached is a diff to switch the pfil(9) subsystem to rmlocks,
> > > which are more suited for the task.  I'd like some exposure before
> > > doing the switch, but I don't expect any fallout.  This email is
> > > going through the patched pfil already - twice.
> >
> > Max,
> >
> > Have you done performance measurements that show rmlocks to be a win
> > in this scenario?  I did some patchs for UNIX domain sockets to
> > replace the rwlock there but it appeared not to have a measurable
> > impact on SQL benchmarks, presumbaly because the read/write blend
> > wasn't right and/or that wasnt a significant source of overhead in
> > the benchmark.  I'd anticipate a much more measurable improvement for
> > pfil, but would be interested in learning how much is seen?
>
> I had to roll an artificial benchmark in order to see a significant
> change (attached - it's a hack!).

attached again text/x-csrc attachment got stripped.

> Using 3 threads on a 4 CPU machine I get the following results:
> null hook: ~13% +/- 2
> mtx hook: up to 40% [*]
> rw hook: ~5% +/- 1
> rm hook: ~35% +/- 5
>
> [*] The mtx hook is inconclusive as my measurements vary a lot.  If one
> thread gets lucky and keeps running the overall time obviously goes
> down by a magnitude.  It seems however, that rmlocks greatly increase
> the chance of that happening - not sure if that's a good thing, though.
>  If all threads receive approximately equal runtime (which is almost
> always the case for rwlocks) the difference is somewhere around 10%.



-- 
/"\  Best regards,                      | [EMAIL PROTECTED]
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | [EMAIL PROTECTED]
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News
/*-
 * Copyright (c) 2000 Max Laier <[EMAIL PROTECTED]>
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the distribution.
 *
 * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 *
 *     $FreeBSD$
 */

#include <sys/types.h>
#include <sys/param.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/rwlock.h>
#include <sys/rmlock.h>
#include <sys/systm.h>
#include <sys/module.h>
#include <sys/mbuf.h>
#include <sys/sysctl.h>
#include <sys/socket.h>
#include <sys/kernel.h>
#include <sys/kthread.h>
#include <sys/time.h>

#include <net/pfil.h>
#include <net/if.h>

static struct mtx pb_mtx;
MTX_SYSINIT(pb_mtx, &pb_mtx, "pfilbench thread mtx", MTX_DEF);

static struct mtx hl_mtx;
MTX_SYSINIT(hl_mtx, &hl_mtx, "pfilbench mtx", MTX_DEF);
static struct rwlock hl_rw;
RW_SYSINIT(hl_rw, &hl_rw, "pfilbench rwlock");
static struct rmlock hl_rm;
RM_SYSINIT(hl_rm, &hl_rm, "pfilbench rmlock", 0);

static struct pfil_head pb_hook;
static struct ifnet pb_dummy;

SYSCTL_NODE(_debug, OID_AUTO, pfilbench, CTLFLAG_RW, 0, "pfilbench");

static unsigned long count = 1000*1000*10;
SYSCTL_ULONG(_debug_pfilbench, OID_AUTO, count, CTLFLAG_RW, &count, 0,
    "count");
static int die = 0;
SYSCTL_INT(_debug_pfilbench, OID_AUTO, die, CTLFLAG_RW, &die, 0,
    "die");
static int threads = 1;
SYSCTL_INT(_debug_pfilbench, OID_AUTO, threads, CTLFLAG_RW, &threads, 0,
    "threads");
static int mode = 0;    /* 0 = NULL, 1 = mtx, 2 = rw, 3 = rm */
SYSCTL_INT(_debug_pfilbench, OID_AUTO, mode, CTLFLAG_RW, &mode, 0,
    "mode");
static int start = 0;
static int init = 0;

static int done;
static void
testloop(void *arg) {
        struct mbuf *m;
        unsigned long i;
        struct timeval stime, etime;

        m = m_gethdr(M_TRYWAIT, MT_HEADER);
        if (m == NULL)
                return;
        m->m_pkthdr.rcvif = &pb_dummy;

        while (init && !die && !*(int *)arg)
                tsleep(arg, 0, "ready", hz);

        microtime(&stime);
        for (i = 0; i < count && !die; i++) {
                if (pfil_run_hooks(&pb_hook, &m, &pb_dummy, PFIL_IN, NULL) !=
                    0)
                        return;
                if (m == NULL)
                        return;
        }
        microtime(&etime);
        if (etime.tv_usec - stime.tv_usec < 0)
                etime.tv_sec--;
        etime.tv_usec = abs(etime.tv_usec - stime.tv_usec);
        etime.tv_sec = etime.tv_sec - stime.tv_sec;
        
        mtx_lock(&pb_mtx);
        printf("RESULT: %jd/%jd.%jd\n", i + 1, etime.tv_sec, etime.tv_usec);
        done++;
        mtx_unlock(&pb_mtx);
        wakeup(&done);

        kproc_exit(0);
}

static int
null_hook(void *arg, struct mbuf **mp, struct ifnet *ifp, int dir,
   struct inpcb *arg1)
{
        DELAY(1);
        return 0;
}

static int
mtx_hook(void *arg, struct mbuf **mp, struct ifnet *ifp, int dir,
   struct inpcb *arg1)
{
        mtx_lock(&hl_mtx);
        DELAY(1);
        mtx_unlock(&hl_mtx);

        return 0;
}

static int
rw_hook(void *arg, struct mbuf **mp, struct ifnet *ifp, int dir,
   struct inpcb *arg1)
{
        rw_rlock(&hl_rw);
        DELAY(1);
        rw_runlock(&hl_rw);

        return 0;
}

static int
rm_hook(void *arg, struct mbuf **mp, struct ifnet *ifp, int dir,
   struct inpcb *arg1)
{
        struct rm_priotracker rmpt;

        rm_rlock(&hl_rm, &rmpt);
        DELAY(1);
        rm_runlock(&hl_rm, &rmpt);

        return 0;
}

static int
sysctl_pb_init(SYSCTL_HANDLER_ARGS)
{
        int _init = init;
        int error, i;

        error = sysctl_handle_int(oidp, &_init, 0, req);
        _init = _init ? 1 : 0;

        if (init == _init)
                return (error);

        if (init == 1) {
                switch (mode) {
                case 0:
                        pfil_remove_hook(null_hook, NULL, PFIL_ALL,
                            &pb_hook);
                        break;
                case 1:
                        pfil_remove_hook(mtx_hook, NULL, PFIL_ALL,
                            &pb_hook);
                        break;
                case 2:
                        pfil_remove_hook(rw_hook, NULL, PFIL_ALL,
                            &pb_hook);
                        break;
                case 3:
                        pfil_remove_hook(rm_hook, NULL, PFIL_ALL,
                            &pb_hook);
                        break;
                }

                mtx_lock(&pb_mtx);
                die = 1;
                init = 0;
                mtx_unlock(&pb_mtx);
                wakeup(&start);
        }

        if (init == 0) {
                mtx_lock(&pb_mtx);
                init = 1;
                mtx_unlock(&pb_mtx);
                for (i = 0; i < threads; i++) {
                        kproc_create(testloop, &start, NULL, 0, 0,
                            "pb_loop_%d", i);
                }
                switch (mode) {
                case 0:
                        pfil_add_hook(null_hook, NULL, PFIL_ALL|PFIL_WAITOK,
                            &pb_hook);
                        break;
                case 1:
                        pfil_add_hook(mtx_hook, NULL, PFIL_ALL|PFIL_WAITOK,
                            &pb_hook);
                        break;
                case 2:
                        pfil_add_hook(rw_hook, NULL, PFIL_ALL|PFIL_WAITOK,
                            &pb_hook);
                        break;
                case 3:
                        pfil_add_hook(rm_hook, NULL, PFIL_ALL|PFIL_WAITOK,
                            &pb_hook);
                        break;
                }
                done = 0;
        }

        return (error);
}
SYSCTL_PROC(_debug_pfilbench, OID_AUTO, init, CTLTYPE_INT|CTLFLAG_RW,
    &init, 0, &sysctl_pb_init, "I", "Init the benchmark");

static int
sysctl_pb_start(SYSCTL_HANDLER_ARGS)
{
        int _start = start;
        int error;

        error = sysctl_handle_int(oidp, &_start, 0, req);
        _start = _start ? 1 : 0;

        if (init == 0)
                return (EINVAL);

        if (_start) {
                start = 1;
                wakeup(&start);
        } else {
                start = 0;
        }

        return (error);
}
SYSCTL_PROC(_debug_pfilbench, OID_AUTO, start, CTLTYPE_INT|CTLFLAG_RW,
    &start, 0, &sysctl_pb_start, "I", "Start the benchmark");


static int
load (module_t mod, int cmd, void *arg)
{
        int error;

        error = 0;
        switch (cmd) {
        case MOD_LOAD:
                pb_hook.ph_type = PFIL_TYPE_IFNET;
                pb_hook.ph_ifnet = &pb_dummy;
                pfil_head_register(&pb_hook);
                break;
        case MOD_UNLOAD:
                die = 1;
                wakeup(&start);
                pause("stoping", hz);
                pfil_head_unregister(&pb_hook);
                break;
        default :
                error = EOPNOTSUPP;
                break;
        }
        return error;
}

static moduledata_t mod_data= {
        "pfilbench",
        load,
        0
};

DECLARE_MODULE(pfilbench, mod_data, SI_SUB_EXEC, SI_ORDER_ANY);

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to