Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 19 Jun 2002 21:43:08 -0600
From:      "Kenneth D. Merry" <ken@kdm.org>
To:        Bosko Milekic <bmilekic@unixdaemons.com>
Cc:        Dag-Erling Smorgrav <des@ofug.org>, current@FreeBSD.ORG, net@FreeBSD.ORG
Subject:   Re: new zero copy sockets snapshot
Message-ID:  <20020619214308.A8221@panzer.kdm.org>
In-Reply-To: <20020619120641.A18434@unixdaemons.com>; from bmilekic@unixdaemons.com on Wed, Jun 19, 2002 at 12:06:41PM -0400
References:  <20020618223635.A98350@panzer.kdm.org> <xzpelf3ida1.fsf@flood.ping.uio.no> <20020619090046.A2063@panzer.kdm.org> <20020619120641.A18434@unixdaemons.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jun 19, 2002 at 12:06:41 -0400, Bosko Milekic wrote:
> On Wed, Jun 19, 2002 at 09:00:46AM -0600, Kenneth D. Merry wrote:
> > On Wed, Jun 19, 2002 at 15:55:18 +0200, Dag-Erling Smorgrav wrote:
> > > "Kenneth D. Merry" <ken@kdm.org> writes:
> > > > With those fixes, plus several fixes that have gone into -current over the
> > > > past week or so, the zero copy sockets code runs without any WITNESS
> > > > warnings at all now.
> > > 
> > > Planning to commit soon?
> > 
> > That's the plan.  I need to see if any comments come up, and then make sure
> > I'll be in town for a few days at least to handle any problems that come
> > up.
> 
>   I have a few comments and questions:
> 
>   - In uipc_jumbo.c, you appear to avoid using some of the SLIST macros,
>     notably when grabbing something off the inuse or free lists.

Thanks for pointing that out.  I only see two instances, the attached patch
should fix them.

>   - Is the mutex really necessary in jumbo_vm_init()? I suspect that
>     if you need to ensure mutual exclusion here, there's something
>     wrong; (why would you be running the jumbo vm code before having
>     initialized it?) - perhaps you could init the mtx at runtime in the
>     jumbo_vm_init routine.

The reason for the mutex protection in jumbo_vm_init() is to protect
against simultaneous callers.

I'm going on the assumption that I don't know exactly who might be calling
jumbo_vm_init(), or when, so it's better to make sure it is locked down
than risk duplicate callers.

>   - It's kind of unfortunate that uipc_jumbo.c has to roll its own
>     allocator;  There are a couple of alternatives; to just use 
>     UMA or mb_alloc for this sort of thing.  mb_alloc will wire
>     all pages allocated through it, though, so maybe it would be worth
>     looking at what UMA can do in this respect.  If it cannot do exactly
>     what you're looking for, then modifying it to do what you want
>     should be relatively simple.  The added advantage is that you'll now
>     be using per-CPU caches for allocations of jumbo bufs, and not the
>     single freelist that you currently use.

See Drew's response.  (thanks Drew!)

>   - I don't know anything about the if_ti code, but it sure looks like
>     it's holding the ti mutex a long time in some places (not only in
>     your patch).

(backround information)

Bill did the conversion in if_ti.c in many places by substituting TI_LOCK()
for splimp().  (from looking at rev 1.36)

There were a couple of additional places where he adding locking as well.

I followed more or less the same pattern with converting the new portions
of the ti(4) driver.

I need to take a close look at the driver to see whether some of the places
where locks are held can be reduced somewhat.  If you've got some
suggestions on places that might be tweaked, I'd like to hear them!

If you're referring to the memory copy routines (ti_copy_mem(),
ti_copy_scratch()), I'd rather leave the locking in for them.  They modify
the window/segment pointers to read/write memory, and it would cause
problems if those pointers got changed out from under us.

In any case, those routines are only used for debugging, so they're not on
the normal critical code path.

I'd be more concerned with reducing the amount of time under lock for the
general packet paths.

I see one instance where the lock can be taken out -- ti_newbuf_jumbo()
(the new version) doesn't need it, since it is always called with the lock
held anyway.

Thanks for the feedback!

Ken
-- 
Kenneth Merry
ken@kdm.org

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020619214308.A8221>