[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Muli Ben-Yehuda <muli@xxxxxxxxxx>, Kieran Mansley <kmansley@xxxxxxxxxxxxxx>
  • From: Keir Fraser <keir@xxxxxxxxxxxxx>
  • Date: Wed, 09 May 2007 14:09:13 +0100
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 09 May 2007 06:08:14 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AceSOz0me4DLzv4uEduwogAX8io7RQ==
  • Thread-topic: [Xen-devel] [PATCH 3/4] Support accelerated network plugin modules

On 9/5/07 13:17, "Muli Ben-Yehuda" <muli@xxxxxxxxxx> wrote:

>> The first macro I'm happy to get rid of - I noticed after Keir
>> commented on the use of caps in the name that it's no longer used.
>> The second I think is enough code that it would unnecessarily
>> clutter the existing functions.  For this reason I'd rather leave it
>> in (with a lower-case name).
> 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. 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.

 -- Keir

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.