This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH 3/4] Support accelerated network plugin modules

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] [PATCH 3/4] Support accelerated network plugin modules
From: Kieran Mansley <kmansley@xxxxxxxxxxxxxx>
Date: Wed, 09 May 2007 14:25:17 +0100
Delivery-date: Wed, 09 May 2007 06:23:44 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <C2678689.E96F%keir@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C2678689.E96F%keir@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Wed, 2007-05-09 at 14:09 +0100, Keir Fraser wrote:
> > It's a matter of taste, but I'd prefer it if it was obvious when
> > looking at the code that the hook is being called with a spinlock
> > held. That's actually another thing - why must every hook be called
> > with the spinlock held? if it's to protect the accelerator from going
> > away, what's actually needed is a ref count (struct kref) on the
> > accelerator.
> I agree the lock should go. Removing the accelerator from under the feet of
> an active vif just doesn't seem a sane action to support.

It's been quite useful for me to be able to do so, and not supporting it
is in some ways more effort than supporting it.

>  And it should be
> possible to support atomic-enough addition of an accelerator without need
> for heavyweight locking. We don't want another lock-with-irqs-off on our
> netfront data paths.

I'm happy to replace that spinlock with a kref as suggested by Muli, and
this should then make that macro redundant too.  However, I'd been
careful to avoid spinlocks on the data path (it uses netif_poll_disable
() and netif_tx_disable() to protect those when removing) so this won't
deliver a benefit there as you might expect it to. 


Xen-devel mailing list