lkml.org 
[lkml]   [2007]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] remove artificial software max_loop limit
 > OK, here is a re-spin patch that I tested as module or
> link-in-vmlinux. Both produce satisfactory result for me.

Is there a plan to include this brilliant code in mainline Kernel?
It works excellent, tested with 15000 loop devices, it's simply cool.

Thank you for your consideration.

Tomas M
slax.org



Ken Chen wrote:
> On 3/31/07, Andrew Morton <akpm@linux-foundation.org> wrote:
>> > Yes, the distros do, and they recommend it to their users a lot.
>>
>> Thanks. In that case I think we should retain the max_loop module
>> parameter
>> for now.
>>
>> Ken, when you respin that patch could you restore max_loop, and make its
>> use trigger a warning along the lines of "loop: the max_loop option is
>> obsolete
>> and will be removed in March 2008"?
>
> OK, here is a re-spin patch that I tested as module or
> link-in-vmlinux. Both produce satisfactory result for me. I also
> enclosed the patch as an attachment just in case my email client
> decide to eat away white spaces for the in-line text.
>
> ------------------------------------------------------
> Subject: remove artificial software max_loop limit
> From: "Ken Chen" <kenchen@google.com>
>
> Remove artificial maximum 256 loop device that can be created due to a
> legacy device number limit. Searching through lkml archive, there are
> several instances where users complained about the artificial limit
> that the loop driver impose. There is no reason to have such limit.
>
> This patch rid the limit entirely and make loop device and associated
> block queue instantiation on demand. With on-demand instantiation, it
> also gives the benefit of not wasting memory if these devices are not
> in use (compare to current implementation that always create 8 loop
> devices), a net improvement in both areas. This version is both
> tested with creation of large number of loop devices and is compatible
> with existing losetup/mount user land tools.
>
> There are a number of people who worked on this and provided valuable
> suggestions, in no particular order, by:
>
> Jens Axboe
> Jan Engelhardt
> Christoph Hellwig
> Thomas M
>
> Signed-off-by: Ken Chen <kenchen@google.com>
> Cc: Jan Engelhardt <jengelh@linux01.gwdg.de>
> Cc: Christoph Hellwig <hch@lst.de>
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 6b5b642..605c1d3 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -77,9 +77,8 @@
>
> #include <asm/uaccess.h>
>
> -static int max_loop = 8;
> -static struct loop_device *loop_dev;
> -static struct gendisk **disks;
> +static LIST_HEAD(loop_devices);
> +static DEFINE_MUTEX(loop_devices_mutex);
>
> /*
> * Transfer functions
> @@ -183,7 +182,7 @@ figure_loop_size(struct loop_device *lo)
> if (unlikely((loff_t)x != size))
> return -EFBIG;
>
> - set_capacity(disks[lo->lo_number], x);
> + set_capacity(lo->lo_disk, x);
> return 0;
> }
>
> @@ -812,7 +811,7 @@ static int loop_set_fd
> lo->lo_queue->queuedata = lo;
> lo->lo_queue->unplug_fn = loop_unplug;
>
> - set_capacity(disks[lo->lo_number], size);
> + set_capacity(lo->lo_disk, size);
> bd_set_size(bdev, size << 9);
>
> set_blocksize(bdev, lo_blocksize);
> @@ -832,7 +831,7 @@ out_clr:
> lo->lo_device = NULL;
> lo->lo_backing_file = NULL;
> lo->lo_flags = 0;
> - set_capacity(disks[lo->lo_number], 0);
> + set_capacity(lo->lo_disk, 0);
> invalidate_bdev(bdev, 0);
> bd_set_size(bdev, 0);
> mapping_set_gfp_mask(mapping, lo->old_gfp_mask);
> @@ -918,7 +917,7 @@ static int loop_clr_fd
> memset(lo->lo_crypt_name, 0, LO_NAME_SIZE);
> memset(lo->lo_file_name, 0, LO_NAME_SIZE);
> invalidate_bdev(bdev, 0);
> - set_capacity(disks[lo->lo_number], 0);
> + set_capacity(lo->lo_disk, 0);
> bd_set_size(bdev, 0);
> mapping_set_gfp_mask(filp->f_mapping, gfp);
> lo->lo_state = Lo_unbound;
> @@ -1322,6 +1321,18 @@ static long lo_compat_ioctl
> }
> #endif
>
> +static struct loop_device *loop_find_dev(int number)
> +{
> + struct loop_device *lo;
> +
> + list_for_each_entry(lo, &loop_devices, lo_list) {
> + if (lo->lo_number == number)
> + return lo;
> + }
> + return NULL;
> +}
> +
> +static struct loop_device *loop_init_one(int i);
> static int lo_open(struct inode *inode, struct file *file)
> {
> struct loop_device *lo = inode->i_bdev->bd_disk->private_data;
> @@ -1330,6 +1341,11 @@ static int lo_open
> lo->lo_refcnt++;
> mutex_unlock(&lo->lo_ctl_mutex);
>
> + mutex_lock(&loop_devices_mutex);
> + if (!loop_find_dev(lo->lo_number + 1))
> + loop_init_one(lo->lo_number + 1);
> + mutex_unlock(&loop_devices_mutex);
> +
> return 0;
> }
>
> @@ -1357,8 +1373,9 @@ static struct block_device_operations lo_fops = {
> /*
> * And now the modules code and kernel interface.
> */
> +static int max_loop;
> module_param(max_loop, int, 0);
> -MODULE_PARM_DESC(max_loop, "Maximum number of loop devices (1-256)");
> +MODULE_PARM_DESC(max_loop, "obsolete, loop device is created on-demand");
> MODULE_LICENSE("GPL");
> MODULE_ALIAS_BLOCKDEV_MAJOR(LOOP_MAJOR);
>
> @@ -1383,7 +1400,7 @@ int loop_unregister_transfer(int number)
>
> xfer_funcs[n] = NULL;
>
> - for (lo = &loop_dev[0]; lo < &loop_dev[max_loop]; lo++) {
> + list_for_each_entry(lo, &loop_devices, lo_list) {
> mutex_lock(&lo->lo_ctl_mutex);
>
> if (lo->lo_encryption == xfer)
> @@ -1398,91 +1415,110 @@ int loop_unregister_transfer(int number)
> EXPORT_SYMBOL(loop_register_transfer);
> EXPORT_SYMBOL(loop_unregister_transfer);
>
> -static int __init loop_init(void)
> +static struct loop_device *loop_init_one(int i)
> +{
> + struct loop_device *lo;
> + struct gendisk *disk;
> +
> + lo = kzalloc(sizeof(*lo), GFP_KERNEL);
> + if (!lo)
> + goto out;
> +
> + lo->lo_queue = blk_alloc_queue(GFP_KERNEL);
> + if (!lo->lo_queue)
> + goto out_free_dev;
> +
> + disk = lo->lo_disk = alloc_disk(1);
> + if (!disk)
> + goto out_free_queue;
> +
> + mutex_init(&lo->lo_ctl_mutex);
> + lo->lo_number = i;
> + lo->lo_thread = NULL;
> + init_waitqueue_head(&lo->lo_event);
> + spin_lock_init(&lo->lo_lock);
> + disk->major = LOOP_MAJOR;
> + disk->first_minor = i;
> + disk->fops = &lo_fops;
> + disk->private_data = lo;
> + disk->queue = lo->lo_queue;
> + sprintf(disk->disk_name, "loop%d", i);
> + add_disk(disk);
> + list_add_tail(&lo->lo_list, &loop_devices);
> + return lo;
> +
> +out_free_queue:
> + blk_cleanup_queue(lo->lo_queue);
> +out_free_dev:
> + kfree(lo);
> +out:
> + return ERR_PTR(-ENOMEM);
> +}
> +
> +static void loop_del_one(struct loop_device *lo)
> {
> - int i;
> + del_gendisk(lo->lo_disk);
> + blk_cleanup_queue(lo->lo_queue);
> + put_disk(lo->lo_disk);
> + list_del(&lo->lo_list);
> + kfree(lo);
> +}
>
> - if (max_loop < 1 || max_loop > 256) {
> - printk(KERN_WARNING "loop: invalid max_loop (must be between"
> - " 1 and 256), using default (8)\n");
> - max_loop = 8;
> - }
> +static struct kobject *loop_probe(dev_t dev, int *part, void *data)
> +{
> + unsigned int number = dev & MINORMASK;
> + struct loop_device *lo;
> +
> + mutex_lock(&loop_devices_mutex);
> + lo = loop_find_dev(number);
> + if (lo == NULL)
> + lo = loop_init_one(number);
> + mutex_unlock(&loop_devices_mutex);
> +
> + *part = 0;
> + if (IS_ERR(lo))
> + return (void *)lo;
> + else
> + return &lo->lo_disk->kobj;
> +}
> +
> +static int __init loop_init(void)
> +{
> + struct loop_device *lo;
>
> if (register_blkdev(LOOP_MAJOR, "loop"))
> return -EIO;
> + blk_register_region(MKDEV(LOOP_MAJOR, 0), 1UL << MINORBITS,
> + THIS_MODULE, loop_probe, NULL, NULL);
>
> - loop_dev = kmalloc(max_loop * sizeof(struct loop_device), GFP_KERNEL);
> - if (!loop_dev)
> - goto out_mem1;
> - memset(loop_dev, 0, max_loop * sizeof(struct loop_device));
> + lo = loop_init_one(0);
> + if (IS_ERR(lo))
> + goto out;
>
> - disks = kmalloc(max_loop * sizeof(struct gendisk *), GFP_KERNEL);
> - if (!disks)
> - goto out_mem2;
> + if (max_loop) {
> + printk(KERN_INFO "loop: the max_loop option is obsolete "
> + "and will be removed in March 2008\n");
>
> - for (i = 0; i < max_loop; i++) {
> - disks[i] = alloc_disk(1);
> - if (!disks[i])
> - goto out_mem3;
> }
> -
> - for (i = 0; i < max_loop; i++) {
> - struct loop_device *lo = &loop_dev[i];
> - struct gendisk *disk = disks[i];
> -
> - memset(lo, 0, sizeof(*lo));
> - lo->lo_queue = blk_alloc_queue(GFP_KERNEL);
> - if (!lo->lo_queue)
> - goto out_mem4;
> - mutex_init(&lo->lo_ctl_mutex);
> - lo->lo_number = i;
> - lo->lo_thread = NULL;
> - init_waitqueue_head(&lo->lo_event);
> - spin_lock_init(&lo->lo_lock);
> - disk->major = LOOP_MAJOR;
> - disk->first_minor = i;
> - disk->fops = &lo_fops;
> - sprintf(disk->disk_name, "loop%d", i);
> - disk->private_data = lo;
> - disk->queue = lo->lo_queue;
> - }
> -
> - /* We cannot fail after we call this, so another loop!*/
> - for (i = 0; i < max_loop; i++)
> - add_disk(disks[i]);
> - printk(KERN_INFO "loop: loaded (max %d devices)\n", max_loop);
> + printk(KERN_INFO "loop: module loaded\n");
> return 0;
>
> -out_mem4:
> - while (i--)
> - blk_cleanup_queue(loop_dev[i].lo_queue);
> - i = max_loop;
> -out_mem3:
> - while (i--)
> - put_disk(disks[i]);
> - kfree(disks);
> -out_mem2:
> - kfree(loop_dev);
> -out_mem1:
> +out:
> unregister_blkdev(LOOP_MAJOR, "loop");
> printk(KERN_ERR "loop: ran out of memory\n");
> return -ENOMEM;
> }
>
> -static void loop_exit(void)
> +static void __exit loop_exit(void)
> {
> - int i;
> + struct loop_device *lo, *next;
>
> - for (i = 0; i < max_loop; i++) {
> - del_gendisk(disks[i]);
> - blk_cleanup_queue(loop_dev[i].lo_queue);
> - put_disk(disks[i]);
> - }
> + list_for_each_entry_safe(lo, next, &loop_devices, lo_list)
> + loop_del_one(lo);
> +
> + blk_unregister_region(MKDEV(LOOP_MAJOR, 0), 1UL << MINORBITS);
> if (unregister_blkdev(LOOP_MAJOR, "loop"))
> printk(KERN_WARNING "loop: cannot unregister blkdev\n");
> -
> - kfree(disks);
> - kfree(loop_dev);
> }
>
> module_init(loop_init);
> diff --git a/include/linux/loop.h b/include/linux/loop.h
> index 191a595..0b99b31 100644
> --- a/include/linux/loop.h
> +++ b/include/linux/loop.h
> @@ -64,6 +64,8 @@ struct loop_device {
> wait_queue_head_t lo_event;
>
> request_queue_t *lo_queue;
> + struct gendisk *lo_disk;
> + struct list_head lo_list;
> };
>
> #endif /* __KERNEL__ */
>
>
> ------------------------------------------------------------------------
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 6b5b642..605c1d3 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -77,9 +77,8 @@
>
> #include <asm/uaccess.h>
>
> -static int max_loop = 8;
> -static struct loop_device *loop_dev;
> -static struct gendisk **disks;
> +static LIST_HEAD(loop_devices);
> +static DEFINE_MUTEX(loop_devices_mutex);
>
> /*
> * Transfer functions
> @@ -183,7 +182,7 @@ figure_loop_size(struct loop_device *lo)
> if (unlikely((loff_t)x != size))
> return -EFBIG;
>
> - set_capacity(disks[lo->lo_number], x);
> + set_capacity(lo->lo_disk, x);
> return 0;
> }
>
> @@ -812,7 +811,7 @@ static int loop_set_fd
> lo->lo_queue->queuedata = lo;
> lo->lo_queue->unplug_fn = loop_unplug;
>
> - set_capacity(disks[lo->lo_number], size);
> + set_capacity(lo->lo_disk, size);
> bd_set_size(bdev, size << 9);
>
> set_blocksize(bdev, lo_blocksize);
> @@ -832,7 +831,7 @@ out_clr:
> lo->lo_device = NULL;
> lo->lo_backing_file = NULL;
> lo->lo_flags = 0;
> - set_capacity(disks[lo->lo_number], 0);
> + set_capacity(lo->lo_disk, 0);
> invalidate_bdev(bdev, 0);
> bd_set_size(bdev, 0);
> mapping_set_gfp_mask(mapping, lo->old_gfp_mask);
> @@ -918,7 +917,7 @@ static int loop_clr_fd
> memset(lo->lo_crypt_name, 0, LO_NAME_SIZE);
> memset(lo->lo_file_name, 0, LO_NAME_SIZE);
> invalidate_bdev(bdev, 0);
> - set_capacity(disks[lo->lo_number], 0);
> + set_capacity(lo->lo_disk, 0);
> bd_set_size(bdev, 0);
> mapping_set_gfp_mask(filp->f_mapping, gfp);
> lo->lo_state = Lo_unbound;
> @@ -1322,6 +1321,18 @@ static long lo_compat_ioctl
> }
> #endif
>
> +static struct loop_device *loop_find_dev(int number)
> +{
> + struct loop_device *lo;
> +
> + list_for_each_entry(lo, &loop_devices, lo_list) {
> + if (lo->lo_number == number)
> + return lo;
> + }
> + return NULL;
> +}
> +
> +static struct loop_device *loop_init_one(int i);
> static int lo_open(struct inode *inode, struct file *file)
> {
> struct loop_device *lo = inode->i_bdev->bd_disk->private_data;
> @@ -1330,6 +1341,11 @@ static int lo_open
> lo->lo_refcnt++;
> mutex_unlock(&lo->lo_ctl_mutex);
>
> + mutex_lock(&loop_devices_mutex);
> + if (!loop_find_dev(lo->lo_number + 1))
> + loop_init_one(lo->lo_number + 1);
> + mutex_unlock(&loop_devices_mutex);
> +
> return 0;
> }
>
> @@ -1357,8 +1373,9 @@ static struct block_device_operations lo_fops = {
> /*
> * And now the modules code and kernel interface.
> */
> +static int max_loop;
> module_param(max_loop, int, 0);
> -MODULE_PARM_DESC(max_loop, "Maximum number of loop devices (1-256)");
> +MODULE_PARM_DESC(max_loop, "obsolete, loop device is created on-demand");
> MODULE_LICENSE("GPL");
> MODULE_ALIAS_BLOCKDEV_MAJOR(LOOP_MAJOR);
>
> @@ -1383,7 +1400,7 @@ int loop_unregister_transfer(int number)
>
> xfer_funcs[n] = NULL;
>
> - for (lo = &loop_dev[0]; lo < &loop_dev[max_loop]; lo++) {
> + list_for_each_entry(lo, &loop_devices, lo_list) {
> mutex_lock(&lo->lo_ctl_mutex);
>
> if (lo->lo_encryption == xfer)
> @@ -1398,91 +1415,110 @@ int loop_unregister_transfer(int number)
> EXPORT_SYMBOL(loop_register_transfer);
> EXPORT_SYMBOL(loop_unregister_transfer);
>
> -static int __init loop_init(void)
> +static struct loop_device *loop_init_one(int i)
> +{
> + struct loop_device *lo;
> + struct gendisk *disk;
> +
> + lo = kzalloc(sizeof(*lo), GFP_KERNEL);
> + if (!lo)
> + goto out;
> +
> + lo->lo_queue = blk_alloc_queue(GFP_KERNEL);
> + if (!lo->lo_queue)
> + goto out_free_dev;
> +
> + disk = lo->lo_disk = alloc_disk(1);
> + if (!disk)
> + goto out_free_queue;
> +
> + mutex_init(&lo->lo_ctl_mutex);
> + lo->lo_number = i;
> + lo->lo_thread = NULL;
> + init_waitqueue_head(&lo->lo_event);
> + spin_lock_init(&lo->lo_lock);
> + disk->major = LOOP_MAJOR;
> + disk->first_minor = i;
> + disk->fops = &lo_fops;
> + disk->private_data = lo;
> + disk->queue = lo->lo_queue;
> + sprintf(disk->disk_name, "loop%d", i);
> + add_disk(disk);
> + list_add_tail(&lo->lo_list, &loop_devices);
> + return lo;
> +
> +out_free_queue:
> + blk_cleanup_queue(lo->lo_queue);
> +out_free_dev:
> + kfree(lo);
> +out:
> + return ERR_PTR(-ENOMEM);
> +}
> +
> +static void loop_del_one(struct loop_device *lo)
> {
> - int i;
> + del_gendisk(lo->lo_disk);
> + blk_cleanup_queue(lo->lo_queue);
> + put_disk(lo->lo_disk);
> + list_del(&lo->lo_list);
> + kfree(lo);
> +}
>
> - if (max_loop < 1 || max_loop > 256) {
> - printk(KERN_WARNING "loop: invalid max_loop (must be between"
> - " 1 and 256), using default (8)\n");
> - max_loop = 8;
> - }
> +static struct kobject *loop_probe(dev_t dev, int *part, void *data)
> +{
> + unsigned int number = dev & MINORMASK;
> + struct loop_device *lo;
> +
> + mutex_lock(&loop_devices_mutex);
> + lo = loop_find_dev(number);
> + if (lo == NULL)
> + lo = loop_init_one(number);
> + mutex_unlock(&loop_devices_mutex);
> +
> + *part = 0;
> + if (IS_ERR(lo))
> + return (void *)lo;
> + else
> + return &lo->lo_disk->kobj;
> +}
> +
> +static int __init loop_init(void)
> +{
> + struct loop_device *lo;
>
> if (register_blkdev(LOOP_MAJOR, "loop"))
> return -EIO;
> + blk_register_region(MKDEV(LOOP_MAJOR, 0), 1UL << MINORBITS,
> + THIS_MODULE, loop_probe, NULL, NULL);
>
> - loop_dev = kmalloc(max_loop * sizeof(struct loop_device), GFP_KERNEL);
> - if (!loop_dev)
> - goto out_mem1;
> - memset(loop_dev, 0, max_loop * sizeof(struct loop_device));
> + lo = loop_init_one(0);
> + if (IS_ERR(lo))
> + goto out;
>
> - disks = kmalloc(max_loop * sizeof(struct gendisk *), GFP_KERNEL);
> - if (!disks)
> - goto out_mem2;
> + if (max_loop) {
> + printk(KERN_INFO "loop: the max_loop option is obsolete "
> + "and will be removed in March 2008\n");
>
> - for (i = 0; i < max_loop; i++) {
> - disks[i] = alloc_disk(1);
> - if (!disks[i])
> - goto out_mem3;
> }
> -
> - for (i = 0; i < max_loop; i++) {
> - struct loop_device *lo = &loop_dev[i];
> - struct gendisk *disk = disks[i];
> -
> - memset(lo, 0, sizeof(*lo));
> - lo->lo_queue = blk_alloc_queue(GFP_KERNEL);
> - if (!lo->lo_queue)
> - goto out_mem4;
> - mutex_init(&lo->lo_ctl_mutex);
> - lo->lo_number = i;
> - lo->lo_thread = NULL;
> - init_waitqueue_head(&lo->lo_event);
> - spin_lock_init(&lo->lo_lock);
> - disk->major = LOOP_MAJOR;
> - disk->first_minor = i;
> - disk->fops = &lo_fops;
> - sprintf(disk->disk_name, "loop%d", i);
> - disk->private_data = lo;
> - disk->queue = lo->lo_queue;
> - }
> -
> - /* We cannot fail after we call this, so another loop!*/
> - for (i = 0; i < max_loop; i++)
> - add_disk(disks[i]);
> - printk(KERN_INFO "loop: loaded (max %d devices)\n", max_loop);
> + printk(KERN_INFO "loop: module loaded\n");
> return 0;
>
> -out_mem4:
> - while (i--)
> - blk_cleanup_queue(loop_dev[i].lo_queue);
> - i = max_loop;
> -out_mem3:
> - while (i--)
> - put_disk(disks[i]);
> - kfree(disks);
> -out_mem2:
> - kfree(loop_dev);
> -out_mem1:
> +out:
> unregister_blkdev(LOOP_MAJOR, "loop");
> printk(KERN_ERR "loop: ran out of memory\n");
> return -ENOMEM;
> }
>
> -static void loop_exit(void)
> +static void __exit loop_exit(void)
> {
> - int i;
> + struct loop_device *lo, *next;
>
> - for (i = 0; i < max_loop; i++) {
> - del_gendisk(disks[i]);
> - blk_cleanup_queue(loop_dev[i].lo_queue);
> - put_disk(disks[i]);
> - }
> + list_for_each_entry_safe(lo, next, &loop_devices, lo_list)
> + loop_del_one(lo);
> +
> + blk_unregister_region(MKDEV(LOOP_MAJOR, 0), 1UL << MINORBITS);
> if (unregister_blkdev(LOOP_MAJOR, "loop"))
> printk(KERN_WARNING "loop: cannot unregister blkdev\n");
> -
> - kfree(disks);
> - kfree(loop_dev);
> }
>
> module_init(loop_init);
> diff --git a/include/linux/loop.h b/include/linux/loop.h
> index 191a595..0b99b31 100644
> --- a/include/linux/loop.h
> +++ b/include/linux/loop.h
> @@ -64,6 +64,8 @@ struct loop_device {
> wait_queue_head_t lo_event;
>
> request_queue_t *lo_queue;
> + struct gendisk *lo_disk;
> + struct list_head lo_list;
> };
>
> #endif /* __KERNEL__ */

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-04-04 12:35    [W:0.073 / U:1.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site