From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH 3/4] Support accelerated network plugin modules Date: Wed, 09 May 2007 14:09:13 +0100 Message-ID: References: <20070509121707.GQ4313@rhun.haifa.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20070509121707.GQ4313@rhun.haifa.ibm.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Muli Ben-Yehuda , Kieran Mansley Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On 9/5/07 13:17, "Muli Ben-Yehuda" 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