WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH linux-2.6.18-xen] blktap: make max # of tap devi

On Thu, 2011-02-24 at 11:40 -0500, Jan Beulich wrote:
> >>> On 23.02.11 at 11:38, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:
> > Any reason why in .32 and newer you still don't use
> > __register_chrdev() (and __unregister_chrdev())? And even
> > if this still needs to be this way, I note that unregister_chrdev()
> > calls __unregister_chrdev_region() before cdev_del(), while
> > blktap_ring_exit() does it the other way around?

I wrote this for .27 iirc and haven't looked into char_dev since then.
I'm always for prettification.

> I think this could be cleaned up like this:
> 
> --- a/drivers/xen/blktap/ring.c
> +++ b/drivers/xen/blktap/ring.c
> @@ -8,7 +8,6 @@
>  #include "blktap.h"
>  
>  int blktap_ring_major;
> -static struct cdev blktap_ring_cdev;
>  
>   /* 
>    * BLKTAP - immediately before the mmap area,
> @@ -511,26 +510,16 @@ blktap_ring_debug(struct blktap *tap, ch
>  int __init
>  blktap_ring_init(void)
>  {
> -     dev_t dev = 0;
>       int err;
>  
> -     cdev_init(&blktap_ring_cdev, &blktap_ring_file_operations);
> -     blktap_ring_cdev.owner = THIS_MODULE;
> -
> -     err = alloc_chrdev_region(&dev, 0, MAX_BLKTAP_DEVICE, "blktap2");
> +     err = __register_chrdev(0, 0, MAX_BLKTAP_DEVICE, "blktap2",
> +                             &blktap_ring_file_operations);
>       if (err < 0) {
>               BTERR("error registering ring devices: %d\n", err);
>               return err;
>       }
>  
> -     err = cdev_add(&blktap_ring_cdev, dev, MAX_BLKTAP_DEVICE);
> -     if (err) {
> -             BTERR("error adding ring device: %d\n", err);
> -             unregister_chrdev_region(dev, MAX_BLKTAP_DEVICE);
> -             return err;
> -     }
> -
> -     blktap_ring_major = MAJOR(dev);
> +     blktap_ring_major = err;
>       BTINFO("blktap ring major: %d\n", blktap_ring_major);
>  
>       return 0;
> @@ -542,9 +531,8 @@ blktap_ring_exit(void)
>       if (!blktap_ring_major)
>               return;
>  
> -     cdev_del(&blktap_ring_cdev);
> -     unregister_chrdev_region(MKDEV(blktap_ring_major, 0),
> -                              MAX_BLKTAP_DEVICE);
> +     __unregister_chrdev(blktap_ring_major, 0, MAX_BLKTAP_DEVICE,
> +                         "blktap2");
>  
>       blktap_ring_major = 0;
>  }
> 
> And probably converting MAX_BLKTAP_DEVICE into a configure
> option would also be reasonable.

If you could confirm that __register_chrdev doesn't grow O(n) in space
anywhere (I guess it doesn't), then I don't think that people should be
required to spend much thought on it.

It just to matter because the the *blktaps[minor] vector was statically
allocated. Nowadays it grows during ALLOC_TAP, base-2.

We can set it to either something insanely large, provided toolstacks
don't shoot themselves by running tap-ctl allocate in a loop.

Or keep it large enough so few would ever have to care (I thought 1024
would be just that), and add a sysfs node to override that limit.

> In any case, if I wanted to formally submit patches to clean up
> this and some other things in the pv-ops variant, what (preferably
> non-topic) branch should those be against? If I'm not mistaken,
> xen/next-2.6.38 for example doesn't even have a blktap - or did
> it get moved out of drivers/xen/? 

I've got a patch for .38 mostly ready. It's going to move into
drivers/block/blktap.

Plus and some thoughts on logical block size, barrier support and some
early trim stuff.

> And what would be the most
> up-to-date non-experimental branch to pull blktap bits from?

That'd be my tree on xenbits after I pushed some more stuff up there.
Which apparently is gone. Anyone knowing what's going on there? Did I
miss some organizational change or is it just broken?

Used to be
http://xenbits.xensource.com/gitweb/?p=people/dstodden/linux.git/.git;a=summary

Daniel


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel