From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH linux-2.6.18-xen] blktap: make max # of tap devices a module parameter Date: Thu, 24 Feb 2011 16:40:08 +0000 Message-ID: <4D6697F802000078000338AD@vpn.id2.novell.com> References: <4D63C644.1000503@redhat.com> <4D63E918020000780003327C@vpn.id2.novell.com> <4D63F3B7.90108@redhat.com> <1298396687.27394.10.camel@agari.van.xensource.com> <4D64F19F0200007800033455@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <4D64F19F0200007800033455@vpn.id2.novell.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Daniel Stodden Cc: "xen-devel@lists.xensource.com" , Laszlo Ersek List-Id: xen-devel@lists.xenproject.org >>> On 23.02.11 at 11:38, "Jan Beulich" 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 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" =20 int blktap_ring_major; -static struct cdev blktap_ring_cdev; =20 /*=20 * 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 =3D 0; int err; =20 - cdev_init(&blktap_ring_cdev, &blktap_ring_file_operations); - blktap_ring_cdev.owner =3D THIS_MODULE; - - err =3D alloc_chrdev_region(&dev, 0, MAX_BLKTAP_DEVICE, "blktap2");= + err =3D __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; } =20 - err =3D 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 =3D MAJOR(dev); + blktap_ring_major =3D err; BTINFO("blktap ring major: %d\n", blktap_ring_major); =20 return 0; @@ -542,9 +531,8 @@ blktap_ring_exit(void) if (!blktap_ring_major) return; =20 - 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"); =20 blktap_ring_major =3D 0; } And probably converting MAX_BLKTAP_DEVICE into a configure option would also be reasonable. 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/? And what would be the most up-to-date non-experimental branch to pull blktap bits from? Jan