[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.