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

[Steven Rostedt <srostedt@redhat.com>]

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