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: Muli Ben-Yehuda <muli@xxxxxxxxxx>, Kieran Mansley <kmansley@xxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 3/4] Support accelerated network plugin modules
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
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20070509121707.GQ4313@xxxxxxxxxxxxxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AceSOz0me4DLzv4uEduwogAX8io7RQ==
Thread-topic: [Xen-devel] [PATCH 3/4] Support accelerated network plugin modules
User-agent: Microsoft-Entourage/
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