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

[Kieran Mansley <kmansley@xxxxxxxxxxxxxx>]

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. 

Kieran


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel