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: "Steven Rostedt" <srostedt@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] Turn blktap tapfds into a link list
From: "Andrew Warfield" <andrew.warfield@xxxxxxxxxxxx>
Date: Fri, 29 Sep 2006 10:13:02 -0700
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 29 Sep 2006 10:13:25 -0700
Domainkey-signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth; b=dS5Tfg2R8KvBdETlS9GCT0M+K3LAUEqq/gHbUpnfAuKP7uE8skOyVX5LjnP/Gk64tpulsobh8DLFFdUa8ycHA5j7IkbVHCq3fMi5F7sDFtTsdimwNTQsccx3W+8pPTgCWgwiBzEARXgLJJdRYk1Ru7rj12Zm0R6DP6rmwhP4pm8=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <451C9A67.9000805@xxxxxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Hi Steven,

  thanks for getting to this so quickly!  The patch generally looks good -- 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,
   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?

 - 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.

 - With dynamic allocation, MAX_TAP_DEV seems a little unnecessary.  Shouldn't
   we just allocate until we run out of minors now?

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?


On 9/28/06, Steven Rostedt <srostedt@xxxxxxxxxx> wrote:
The current implementation of blktap has the tapfds descriptors as a
static array.  Which means that what isn't used is taking up memory.  It
also means that if we ever want to dynamically increase the number of
descriptors that we allow, we can't do so.

So this patch converts the tapfds from an array into a link list.

I use the list_add_rcu and list_for_each_entry_rcu so that I don't need
to protect the list while walking or adding items to it.  I never delete
a device, so once it is added to the list, it stays. But it wouldn't be
hard with the rcu list operations to add a deletion in the future.

This patch also fixed a few bugs that were lying in there.  Some where
the tapfds[idx] was offset outside the checking of the idx size
(although they would probably be very hard to hit).

Anyway, I booted this up and it runs. I started a guest with a block
device on the blktap, twice, and there was no problems.

But... I have to admit, I wrote this when I was tired, so it may have
introduced a few bugs as well. I went over it twice, but I could have
missed something twice.  Please review!  But there shouldn't be anything
major wrong with it.

-- Steve

Signed-off-by: Steven Rostedt <srostedt@xxxxxxxxxx>

Xen-devel mailing list

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