JFS resize=0 problem in 2.6.0

Let me know if I'm missing the goal of the code here, but lines 261-273 of linux-2.6.0/fs/jfs/super.c are:

case Opt_resize:

{
	char *resize = args[0].from;
	if (!resize || !*resize) {    /* LINE 264 HERE */
		*newLVSize = sb->s_bdev->bd_inode->i_size >>
			sb->s_blocksize_bits;
		if (*newLVSize == 0)
			printk(KERN_ERR
			"JFS: Cannot determine volume size\n");
	} else
		*newLVSize = simple_strtoull(resize, &resize, 0);
	break;
}

It seems to me that line 264 is attempting to test for the mount paramater "resize=0", and when it comes across this, resize to the full size of the volume. However, this doesn't work. I believe it should test for the char '0' (*resize=='0'), not against literal zero.

Let me know if I'm way off base here. But the below patch does allow a

$ mount -o remount,resize=0 /mnt/test    
to resize the jfs filesystem to the full size of the volume.

Ok, I've never submitted a patch before, but here goes:

@@ -261,7 +261,7 @@

		case Opt_resize:
		{
			char *resize = args[0].from;
-			if (!resize || !*resize) {
+			if (!resize || !*resize || *resize=='0') {
				*newLVSize = sb->s_bdev->bd_inode->i_size >>
					sb->s_blocksize_bits;
				if (*newLVSize == 0)

-Elliott Bennett
[email protected]

And it won't without the patch?

What errors do you get if it fails without the patch?

It won't without the patch.

No errors...it just doesn't resize. :)

Here's a shell snippit. I have just resized /dev/vg/lv from 700M to 900M, and I'm running the ORIGINAL jfs module:

root@tesla:~# df
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/mapper/vg-lv       713500       220    713280   1% /mnt
root@tesla:~# mount
/dev/mapper/vg-lv on /mnt type jfs (rw)
root@tesla:~# mount -o remount,resize=0 /mnt
root@tesla:~# df
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/mapper/vg-lv       713500       220    713280   1% /mnt

..the resizing failed. But, switching to the patched module:

root@tesla:~# umount /mnt
root@tesla:~# rmmod jfs
root@tesla:~# cp ~/jfs_patch.ko /lib/modules/2.6.0/kernel/fs/jfs/jfs.ko
root@tesla:~# mount -t jfs /dev/vg/lv /mnt
root@tesla:~# df
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/mapper/vg-lv       713500       220    713280   1% /mnt
root@tesla:~# mount -o remount,resize=0 /mnt
root@tesla:~# df
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/mapper/vg-lv       917272       244    917028   1% /mnt

resizing works.

Oddly, now, an additional lvextend and remount/resize fails:

root@tesla:~# lvextend /dev/vg/lv -L +200M
  /dev/hdd: open failed: Read-only file system
  Extending logical volume lv to 1.07 GB
  Logical volume lv successfully resized
root@tesla:~# mount -o remount,resize=0 /mnt
root@tesla:~# df
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/mapper/vg-lv       917272       244    917028   1% /mnt
root@tesla:~# mount
/dev/mapper/vg-lv on /mnt type jfs (rw,resize=0,resize=0)

	note "resize=0,resize=0" above.
	...but unmounting it, remounting it, and then resizing it works:

root@tesla:~# umount /mnt
root@tesla:~# mount -t jfs /dev/vg/lv /mnt
root@tesla:~# df
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/mapper/vg-lv       917272       244    917028   1% /mnt
root@tesla:~# mount -o remount,resize=0 /mnt
root@tesla:~# df
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/mapper/vg-lv      1121040       272   1120768   1% /mnt
root@tesla:~# mount
/dev/mapper/vg-lv on /mnt type jfs (rw,resize=0)

It turns out that it's not so much of the fact that this is a
"second" extention during the same mount as the fact that it was extended while mounted. It seems that nomatter when I lvextend (mounted or unmounted), i must unmount (if mounted), then mount, then remount/resize for a resize to take effect. If I don't unmount after lvextending, and try to resize, I get:
jfs_extendfs: volume hasn't grown, returning

Soo...I would say that something is awry with the resizing of JFS filesystems. My patch obviously doesn't fix everything, but at least makes resizing to fill available space *possible*. :)

The code surrounding my patch change treats the same variable (resize, which is a pointer to args[0].from) as a string, so it seems pretty obvious to me it should be comparing to '0'.

-Elliott Bennett

I would be careful of DM and MD RAID in 2.6.0. There are some bugs flying around mentioning XFS->DM->MD RAID, , but also reproducable with Ext3->DM.

So if you're using DM, you might want to do some extra consistancy checks in your tests, and don't use it with important data.

Am Fr, den 02.01.2004 schrieb Mike Fedyk um 22:24:

Only DM or DM->RAID? There is one bug on bugzilla mentioning XFS problems on a >2TB LVM volume dating back to 2.5.73. DM uses sector_t everywhere, so perhaps it might be a different problem.

DM has a workaround included since one of the later test kernels so that it should not break on top of the raid code. Also someone posted bugfixes not too long ago that fixed some hard to trigger bugs with bi_idx not always starting at zero (which should have affected raid and dm code under rare circumstances). These bugfixes are in 2.6.1-pre1 I think.

There is also one bug that isn't fixed in 2.6.0 (but in the -mm kernels for some time and also in 2.6.1-pre1), it concerns online resizing of mounted filesystems.

At least I think the core DM code should be safe under normal usage. I bombed it with all kinds of BIOs, not only the page sized ones most filesystems and swap code create, using an IDE disk. I'm using it on all my systems for a long time now.

Elliot's problem is a different though, the jfs option parser doesn't do what he wants. ;)

Am So, den 28.12.2003 schrieb [email protected] um 16:30:

literal zero is the end of the string.

So the code checks for resize= and not resize=0.

I think your fix is wrong because it would also recognize resize=0123 because it only tests the first character.

-           if (!resize || !*resize) {
+           if (!resize || !*resize || *resize=='0')

It should probably be

+ if (!resize || !*resize || (*resize=='0' && !resize[1]))

Or better: check the integer value that is returned by simple_strtoull.

But did you test what resize= does?

Good catch! I'm embarrassed that I didn't see that.

As for "resize=", it seems to fail. Probably because it doesn't match the "resize=%u" pattern:

root@tesla:~# mount -o remount,resize= /mnt
mount: /mnt not mounted already, or bad option

Someone posted a patch the other day that supposedly handles both "resize=0" and "resize" alone..I believe by simply adding it to the pattern list.

-Elliott

Sorry for not replying sooner. I was on vacation.

Originally, the code accepted "resize" or "resize=" as an indication that it should resize to the block device size. Recently, the parsing code was cleaned up to use the match_token() function, and "resize" without an argument is no longer accepted.

The following patch should allow the resize option without an argument, as well as allowing resize=0 to indicate the same thing.

diff -ur linux-2.6.0/fs/jfs/super.c linux/fs/jfs/super.c
@@ -203,7 +203,7 @@
 
 enum {
 	Opt_integrity, Opt_nointegrity, Opt_iocharset, Opt_resize,
-	Opt_errors, Opt_ignore, Opt_err,
+	Opt_resize_nosize, Opt_errors, Opt_ignore, Opt_err,
 };
 
 static match_table_t tokens = {

@@ -211,6 +211,7 @@
 	{Opt_nointegrity, "nointegrity"},
 	{Opt_iocharset, "iocharset=%s"},
 	{Opt_resize, "resize=%u"},
+	{Opt_resize_nosize, "resize"},
 	{Opt_errors, "errors=%s"},
 	{Opt_ignore, "noquota"},
 	{Opt_ignore, "quota"},

@@ -261,7 +262,7 @@
 		case Opt_resize:
 		{
 			char *resize = args[0].from;
-			if (!resize || !*resize) {
+			if (!resize || !*resize || resize == 0) {
 				*newLVSize = sb->s_bdev->bd_inode->i_size >>
 					sb->s_blocksize_bits;
 				if (*newLVSize == 0)

@@ -271,6 +272,15 @@
 				*newLVSize = simple_strtoull(resize, &resize, 0);
 			break;
 		}
+		case Opt_resize_nosize:
+		{
+			*newLVSize = sb->s_bdev->bd_inode->i_size >>
+				sb->s_blocksize_bits;
+			if (*newLVSize == 0)
+				printk(KERN_ERR
+				       "JFS: Cannot determine volume size\n");
+			break;
+		}
 		case Opt_errors:
 		{
 			char *errors = args[0].from;

Thanks,
Shaggy

David Kleikamp
IBM Linux Technology Center