Date: Tue, 1 Mar 2005 16:29:49 -0800 From: Luigi Rizzo <rizzo@icir.org> To: John Baldwin <jhb@FreeBSD.org> Cc: net@FreeBSD.org Subject: Re: Giant-free polling [PATCH] Message-ID: <20050301162949.A64187@xorpc.icir.org> In-Reply-To: <200503011642.48813.jhb@FreeBSD.org>; from jhb@FreeBSD.org on Tue, Mar 01, 2005 at 04:42:48PM -0500 References: <E1D1nPr-000IE5-00._pppp-mail-ru@f37.mail.ru> <20050217161031.A46700@xorpc.icir.org> <200503011642.48813.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[cc-ing net@freebsd.org to get more opinions]
On Tue, Mar 01, 2005 at 04:42:48PM -0500, John Baldwin wrote:
> On Thursday 17 February 2005 07:10 pm, Luigi Rizzo wrote:
> > i am no expert about the locking but i see places where
> > you grab polling_lock followed by ifnet_lock, and others where
> > you use the opposite order. This seems prone to deadlock...
>
> Yes, it basically seems that the polling_lock should be removed and just the
> ifnet_lock used because the the ifnet_lock is already always held when the
> polling_lock is locked.
this said, if the lock requests are blocking, you basically end
up with the polling loops always contending for the locks, with only one
doing actual work and the other one always busy-waiting.
Assuming the primitive exists, I think the correct way to implement
SMP polling is to use non-blocking locks, something like this
(in pseudocode)
poll_loop() {
have_polling_lock = try_get_lock(polling_lock)
foreach(ifp in polled_interfaces) {
if (have_polling_lock)
have_ifp_lock = get_lock(ifp) // returns true
else
have_ifp_lock = try_get_lock(ifp)
if (have_ifp_lock) {
ifp->poll();
release_lock(ifp)
}
}
if (have_polling_lock)
release_lock(polling_lock);
}
so additional polling processes after the first one will not
block on busy interfaces and move forward instead.
This should remove a bit of contention, and let separate processes actually
work on different interfaces.
cheers
luigi
> > On Thu, Feb 17, 2005 at 06:18:07PM +0300, dima wrote:
> > > I have removed Giant lock from the kern_poll.c. It is actually replaced
> > > with a pair of ifnet_lock and polling_lock (added by me). The ifnet_lock
> > > is needed considering the situation when a device is dettached during the
> > > polling cycle. I successuflly tested it on 5.3-RELEASE-p5 i386/UP with
> > > fxp NIC and going to test i386/SMP with 2 em NICs next weekend (the SMP
> > > #error can also be removed I think). There is still a problem with the
> > > patch since mtx_destroy() for polling_lock is not called.
> > >
> > > Please review the patch and tell me what you think about it.
>
> --
> John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
> "Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050301162949.A64187>