Re: online resizing of devices/filesystems (2.6)

Re: online resizing of devices/filesystems (2.6)

[Kevin Corry]

On August 21, 2003, Joe Thornber wrote:
> Hi,
> 
> Should genhd.h:set_capacity() find and update the i_size field of the
> inode for the device ?
> 
> The BLKGETSIZE and BLKGETSIZE64 ioctls report the size in the devices
> inode:
> 
> 	case BLKGETSIZE:
> 		if ((bdev->bd_inode->i_size >> 9) > ~0UL)
> 			return -EFBIG;
> 		return put_ulong(arg, bdev->bd_inode->i_size >> 9);
> 	case BLKGETSIZE64:
> 		return put_u64(arg, bdev->bd_inode->i_size);
> 
> Currently people have to close and reopen the device in order for a
> size change to take effect.  This is a problem if people want to do
> online resizing of a filesystem (supported by xfs and resier).

Has anyone had any thoughts about this issue?

To recap, in drivers/md/dm.c:__bind(), there is a call to set_capacity(), 
which sets the device size in the gendisk entry. But if the device is already 
open (e.g., mounted, or in use by another device (loop, raid, other 
device-mapper)), then the bdev->bd_inode->i_size field for that gendisk entry 
also needs to be updated to reflect this new size to the VFS.

My initial thoughts were to add something like this to the __bind() function 
mentioned above:

bdev = bdget_disk(md->disk, 0);
if (bdev) {
	bd_set_size(bdev, size << 9);
	bdput(bdev);
}

Of course, bd_set_size() is static to fs/block_dev.c, but it does seem to do 
exactly what we need in this situation. The only user of bd_set_size() is 
do_open(), which does quite a bit of locking before making modifications to 
the block_device entry.

Does anyone have any advice as to whether this is the correct approach, or 
what additional locking is necessary? Or is there a different approach that 
I'm overlooking?

Thanks!
-- 
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/

Re: online resizing of devices/filesystems (2.6)

[Andrew Morton]

Kevin Corry <kevcorry@us.ibm.com> wrote:
>
> On August 21, 2003, Joe Thornber wrote:
> > Hi,
> > 
> > Should genhd.h:set_capacity() find and update the i_size field of the
> > inode for the device ?
> > 
> > The BLKGETSIZE and BLKGETSIZE64 ioctls report the size in the devices
> > inode:
> > 
> > 	case BLKGETSIZE:
> > 		if ((bdev->bd_inode->i_size >> 9) > ~0UL)
> > 			return -EFBIG;
> > 		return put_ulong(arg, bdev->bd_inode->i_size >> 9);
> > 	case BLKGETSIZE64:
> > 		return put_u64(arg, bdev->bd_inode->i_size);
> > 
> > Currently people have to close and reopen the device in order for a
> > size change to take effect.  This is a problem if people want to do
> > online resizing of a filesystem (supported by xfs and resier).
> 
> Has anyone had any thoughts about this issue?

Resizing a blockdev while someone has a filesystem mounted on it is a bit
rude, but I guess that as long as it is being expanded, not much can go
wrong.

bd_set_size() isn't quite what you want because it fiddles with the
blocksize as well.

So one approach would be to do what NBD does, and just write i_size directly.

You could create a little helper library function which takes i_sem and
then writes i_size.  But the VFS tends to avoid taking i_sem on blockdevs
because it doesn't expect i_size to change ;)

Re: online resizing of devices/filesystems (2.6)

[Kevin Corry]

On Friday 17 October 2003 15:05, Andrew Morton wrote:
> Resizing a blockdev while someone has a filesystem mounted on it is a bit
> rude, but I guess that as long as it is being expanded, not much can go
> wrong.

There's no technical reason why you couldn't shrink as well. But right now we 
don't have any filesystems that support online shrink, so the tools prevent 
that scenario. Online expand is quite common, though.

> bd_set_size() isn't quite what you want because it fiddles with the
> blocksize as well.

Ok.

> So one approach would be to do what NBD does, and just write i_size
> directly.

We had considered that originally. It just seemed like too big of a hack. :) 
Plus I wasn't sure if there were locking issues with changing fields in the 
inode.

> You could create a little helper library function which takes i_sem and
> then writes i_size.  But the VFS tends to avoid taking i_sem on blockdevs
> because it doesn't expect i_size to change ;)

So...should I take i_sem or not? :)  Perhaps calling i_size_write() in 
include/linux/fs.h would be preferrable, since it seems to be doing different 
things for SMP and preempt.

Thanks for the feedback!
-- 
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/

Re: online resizing of devices/filesystems (2.6)

[Kevin Corry]

On Friday 17 October 2003 15:49, Kevin Corry wrote:
> On Friday 17 October 2003 15:05, Andrew Morton wrote:
> > So one approach would be to do what NBD does, and just write i_size
> > directly.
>
> We had considered that originally. It just seemed like too big of a hack.
> :) Plus I wasn't sure if there were locking issues with changing fields in
> the inode.
>
> > You could create a little helper library function which takes i_sem and
> > then writes i_size.  But the VFS tends to avoid taking i_sem on blockdevs
> > because it doesn't expect i_size to change ;)
>
> So...should I take i_sem or not? :)  Perhaps calling i_size_write() in
> include/linux/fs.h would be preferrable, since it seems to be doing
> different things for SMP and preempt.
>
> Thanks for the feedback!

Here's a patch that takes bdev->bd_inode->i_sem, and then calls
i_size_write() to update the inode size. My initial tests have been
successful in resizing mounted reiser and xfs filesystems on
2.6.0-test7.

-- 
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/


When setting the size of a Device-Mapper device in the gendisk entry, also try
to set the size of the corresponding block_device entry's inode. This is
necessary to allow online device/filesystem resizing to work correctly.

--- a/drivers/md/dm.c	9 Oct 2003 16:47:44 -0000
+++ b/drivers/md/dm.c	17 Oct 2003 21:06:01 -0000
@@ -726,6 +726,20 @@
 	up_write(&md->lock);
 }
 
+static void __set_size(struct gendisk *disk, sector_t size)
+{
+	struct block_device *bdev;
+
+	set_capacity(disk, size);
+	bdev = bdget_disk(disk, 0);
+	if (bdev) {
+		down(&bdev->bd_inode->i_sem);
+		i_size_write(bdev->bd_inode, size << SECTOR_SHIFT);
+		up(&bdev->bd_inode->i_sem);
+		bdput(bdev);
+	}
+}
+
 static int __bind(struct mapped_device *md, struct dm_table *t)
 {
 	request_queue_t *q = md->queue;
@@ -733,7 +747,7 @@
 	md->map = t;
 
 	size = dm_table_get_size(t);
-	set_capacity(md->disk, size);
+	__set_size(md->disk, size);
 	if (size == 0)
 		return 0;
 

online resizing of devices/filesystems (2.6)

[Joe Thornber]

Hi,

Should genhd.h:set_capacity() find and update the i_size field of the
inode for the device ?

The BLKGETSIZE and BLKGETSIZE64 ioctls report the size in the devices
inode:

	case BLKGETSIZE:
		if ((bdev->bd_inode->i_size >> 9) > ~0UL)
			return -EFBIG;
		return put_ulong(arg, bdev->bd_inode->i_size >> 9);
	case BLKGETSIZE64:
		return put_u64(arg, bdev->bd_inode->i_size);

Currently people have to close and reopen the device in order for a
size change to take effect.  This is a problem if people want to do
online resizing of a filesystem (supported by xfs and resier).

- Joe