From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Rostedt Subject: Re: [PATCH] Turn blktap tapfds into a link list Date: Fri, 29 Sep 2006 14:32:29 -0400 Message-ID: <451D66BD.3040205@redhat.com> References: <451C9A67.9000805@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Andrew Warfield Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org Andrew Warfield wrote: > Hi Steven, > > thanks for getting to this so quickly! The patch generally looks good Thanks, it was some what fresh in my head, so I decided to dump it out. > -- a > couple of quick thoughts: > > - Linear searches of the tapfds list are a little grim where they > appear in > the data path (blktap_ioctl, blktap_kick_user, fast_flush_area, I didn't like this either. Perhaps I could switch it back to an array of pointers. And I could even have the array be able to resize, with the use of rcu locks. > do_block_io_op, dispatch_rw_block_io). If we are happy with a limit of > 254 concurrent devices for the immediate term, I wonder if a lookup > array > indexed by minor and allocated on use might be better? Yeah, I think I do agree with you on this. I really don't like that linear search. Maybe I did it because I was tired and it seemed cool. ;) > > - I enjoyed seeing the domid_translate array go away, I think we can kill > this translation all together though by moving the domid/busid lookup > out of blktapctrl and into xenbus, and filling it in directly when a > new vbd is connected. This is a separate issue, and would need to be looked at later. (I'm not to sure on the interworkings of that code). > > - With dynamic allocation, MAX_TAP_DEV seems a little unnecessary. > Shouldn't > we just allocate until we run out of minors now? Sure! I just was keeping it in sync with what was there. The old code didn't allocate more than MAX_TAP_DEV so I wasn't about to change it. > > This is a great improvement. I know of at least one person that is > regularly > running blktap with 60-80 vbds -- I'd like to get them to try out the > patch as > an additional check. Also, because of the changes in allocation and > locking > I'm inclined to wait until immediately after the 3.0.3 barrier with this > one. > Sound okay? Sounds fine with me. Thanks, -- Steve