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


[Xen-devel] Re: [PATCH] Turn blktap tapfds into a link list

To: Andrew Warfield <andrew.warfield@xxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] Turn blktap tapfds into a link list
From: Steven Rostedt <srostedt@xxxxxxxxxx>
Date: Fri, 29 Sep 2006 14:32:29 -0400
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 29 Sep 2006 11:32:31 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <eacc82a40609291013x72ed2cf0x77e3e6b977146005@xxxxxxxxxxxxxx>
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>
References: <451C9A67.9000805@xxxxxxxxxx> <eacc82a40609291013x72ed2cf0x77e3e6b977146005@xxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird (X11/20060614)
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

Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>