2011-12-22 15:00:57

by Djalal Harouni

[permalink] [raw]
Subject: [PATCH] fs/ext4: add missing ext4_resize_end

Online resize ioctls 'EXT4_IOC_GROUP_EXTEND' and 'EXT4_IOC_GROUP_ADD'
call ext4_resize_begin() to check permissions and to set the
EXT4_RESIZING bit lock, they do their work and they must finish with
ext4_resize_end() which calls clear_bit_unlock() to unlock and to
avoid -EBUSY errors for the next resize operations.

This patch adds the missing ext4_resize_end() calls on error paths.

Patch tested.

Cc: [email protected]
Signed-off-by: Djalal Harouni <[email protected]>
---
fs/ext4/ioctl.c | 28 ++++++++++++++++++----------
1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index a567968..ab25f57 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -182,19 +182,22 @@ setversion_out:
if (err)
return err;

- if (get_user(n_blocks_count, (__u32 __user *)arg))
- return -EFAULT;
+ if (get_user(n_blocks_count, (__u32 __user *)arg)) {
+ err = -EFAULT;
+ goto group_extend_out;
+ }

if (EXT4_HAS_RO_COMPAT_FEATURE(sb,
EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
ext4_msg(sb, KERN_ERR,
"Online resizing not supported with bigalloc");
- return -EOPNOTSUPP;
+ err = -EOPNOTSUPP;
+ goto group_extend_out;
}

err = mnt_want_write(filp->f_path.mnt);
if (err)
- return err;
+ goto group_extend_out;

err = ext4_group_extend(sb, EXT4_SB(sb)->s_es, n_blocks_count);
if (EXT4_SB(sb)->s_journal) {
@@ -204,9 +207,10 @@ setversion_out:
}
if (err == 0)
err = err2;
+
mnt_drop_write(filp->f_path.mnt);
+group_extend_out:
ext4_resize_end(sb);
-
return err;
}

@@ -267,19 +271,22 @@ mext_out:
return err;

if (copy_from_user(&input, (struct ext4_new_group_input __user *)arg,
- sizeof(input)))
- return -EFAULT;
+ sizeof(input))) {
+ err = -EFAULT;
+ goto group_add_out;
+ }

if (EXT4_HAS_RO_COMPAT_FEATURE(sb,
EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
ext4_msg(sb, KERN_ERR,
"Online resizing not supported with bigalloc");
- return -EOPNOTSUPP;
+ err = -EOPNOTSUPP;
+ goto group_add_out;
}

err = mnt_want_write(filp->f_path.mnt);
if (err)
- return err;
+ goto group_add_out;

err = ext4_group_add(sb, &input);
if (EXT4_SB(sb)->s_journal) {
@@ -289,9 +296,10 @@ mext_out:
}
if (err == 0)
err = err2;
+
mnt_drop_write(filp->f_path.mnt);
+group_add_out:
ext4_resize_end(sb);
-
return err;
}

--
1.7.1


2011-12-23 01:14:21

by Yongqiang Yang

[permalink] [raw]
Subject: Re: [PATCH] fs/ext4: add missing ext4_resize_end

Thanks.

The patch looks good. I have to update the new resize patch too.

Yongqiang.

On Thu, Dec 22, 2011 at 11:00 PM, Djalal Harouni <[email protected]> wrote:
> Online resize ioctls 'EXT4_IOC_GROUP_EXTEND' and 'EXT4_IOC_GROUP_ADD'
> call ext4_resize_begin() to check permissions and to set the
> EXT4_RESIZING bit lock, they do their work and they must finish with
> ext4_resize_end() which calls clear_bit_unlock() to unlock and to
> avoid -EBUSY errors for the next resize operations.
>
> This patch adds the missing ext4_resize_end() calls on error paths.
>
> Patch tested.
>
> Cc: [email protected]
> Signed-off-by: Djalal Harouni <[email protected]>
> ---
> ?fs/ext4/ioctl.c | ? 28 ++++++++++++++++++----------
> ?1 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index a567968..ab25f57 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -182,19 +182,22 @@ setversion_out:
> ? ? ? ? ? ? ? ?if (err)
> ? ? ? ? ? ? ? ? ? ? ? ?return err;
>
> - ? ? ? ? ? ? ? if (get_user(n_blocks_count, (__u32 __user *)arg))
> - ? ? ? ? ? ? ? ? ? ? ? return -EFAULT;
> + ? ? ? ? ? ? ? if (get_user(n_blocks_count, (__u32 __user *)arg)) {
> + ? ? ? ? ? ? ? ? ? ? ? err = -EFAULT;
> + ? ? ? ? ? ? ? ? ? ? ? goto group_extend_out;
> + ? ? ? ? ? ? ? }
>
> ? ? ? ? ? ? ? ?if (EXT4_HAS_RO_COMPAT_FEATURE(sb,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
> ? ? ? ? ? ? ? ? ? ? ? ?ext4_msg(sb, KERN_ERR,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Online resizing not supported with bigalloc");
> - ? ? ? ? ? ? ? ? ? ? ? return -EOPNOTSUPP;
> + ? ? ? ? ? ? ? ? ? ? ? err = -EOPNOTSUPP;
> + ? ? ? ? ? ? ? ? ? ? ? goto group_extend_out;
> ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ?err = mnt_want_write(filp->f_path.mnt);
> ? ? ? ? ? ? ? ?if (err)
> - ? ? ? ? ? ? ? ? ? ? ? return err;
> + ? ? ? ? ? ? ? ? ? ? ? goto group_extend_out;
>
> ? ? ? ? ? ? ? ?err = ext4_group_extend(sb, EXT4_SB(sb)->s_es, n_blocks_count);
> ? ? ? ? ? ? ? ?if (EXT4_SB(sb)->s_journal) {
> @@ -204,9 +207,10 @@ setversion_out:
> ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?if (err == 0)
> ? ? ? ? ? ? ? ? ? ? ? ?err = err2;
> +
> ? ? ? ? ? ? ? ?mnt_drop_write(filp->f_path.mnt);
> +group_extend_out:
> ? ? ? ? ? ? ? ?ext4_resize_end(sb);
> -
> ? ? ? ? ? ? ? ?return err;
> ? ? ? ?}
>
> @@ -267,19 +271,22 @@ mext_out:
> ? ? ? ? ? ? ? ? ? ? ? ?return err;
>
> ? ? ? ? ? ? ? ?if (copy_from_user(&input, (struct ext4_new_group_input __user *)arg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(input)))
> - ? ? ? ? ? ? ? ? ? ? ? return -EFAULT;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(input))) {
> + ? ? ? ? ? ? ? ? ? ? ? err = -EFAULT;
> + ? ? ? ? ? ? ? ? ? ? ? goto group_add_out;
> + ? ? ? ? ? ? ? }
>
> ? ? ? ? ? ? ? ?if (EXT4_HAS_RO_COMPAT_FEATURE(sb,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EXT4_FEATURE_RO_COMPAT_BIGALLOC)) {
> ? ? ? ? ? ? ? ? ? ? ? ?ext4_msg(sb, KERN_ERR,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Online resizing not supported with bigalloc");
> - ? ? ? ? ? ? ? ? ? ? ? return -EOPNOTSUPP;
> + ? ? ? ? ? ? ? ? ? ? ? err = -EOPNOTSUPP;
> + ? ? ? ? ? ? ? ? ? ? ? goto group_add_out;
> ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ?err = mnt_want_write(filp->f_path.mnt);
> ? ? ? ? ? ? ? ?if (err)
> - ? ? ? ? ? ? ? ? ? ? ? return err;
> + ? ? ? ? ? ? ? ? ? ? ? goto group_add_out;
>
> ? ? ? ? ? ? ? ?err = ext4_group_add(sb, &input);
> ? ? ? ? ? ? ? ?if (EXT4_SB(sb)->s_journal) {
> @@ -289,9 +296,10 @@ mext_out:
> ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?if (err == 0)
> ? ? ? ? ? ? ? ? ? ? ? ?err = err2;
> +
> ? ? ? ? ? ? ? ?mnt_drop_write(filp->f_path.mnt);
> +group_add_out:
> ? ? ? ? ? ? ? ?ext4_resize_end(sb);
> -
> ? ? ? ? ? ? ? ?return err;
> ? ? ? ?}
>
> --
> 1.7.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/



--
Best Wishes
Yongqiang Yang

2012-01-03 10:44:43

by Djalal Harouni

[permalink] [raw]
Subject: Re: [PATCH] fs/ext4: add missing ext4_resize_end

On Fri, Dec 23, 2011 at 09:14:20AM +0800, Yongqiang Yang wrote:
> Thanks.
>
> The patch looks good. I have to update the new resize patch too.
>
> Yongqiang.
>
> On Thu, Dec 22, 2011 at 11:00 PM, Djalal Harouni <[email protected]> wrote:
> > Online resize ioctls 'EXT4_IOC_GROUP_EXTEND' and 'EXT4_IOC_GROUP_ADD'
> > call ext4_resize_begin() to check permissions and to set the
> > EXT4_RESIZING bit lock, they do their work and they must finish with
> > ext4_resize_end() which calls clear_bit_unlock() to unlock and to
> > avoid -EBUSY errors for the next resize operations.
> >
> > This patch adds the missing ext4_resize_end() calls on error paths.
Any news on this one ?

If we call EXT4_IOC_GROUP_EXTEND ioctl and get_user() fails with -EFAULT
then any new call will return -EBUSY, unless we unmount/mount the fs.

In rare situations this can lead to a corrupt filesystem, I've experienced
this, but I'm not sure what I did. fsck fix it.

Thanks.

--
tixxdz
http://opendz.org

2012-01-05 02:23:48

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] fs/ext4: add missing ext4_resize_end

On Thu, Dec 22, 2011 at 04:00:57PM +0100, Djalal Harouni wrote:
> Online resize ioctls 'EXT4_IOC_GROUP_EXTEND' and 'EXT4_IOC_GROUP_ADD'
> call ext4_resize_begin() to check permissions and to set the
> EXT4_RESIZING bit lock, they do their work and they must finish with
> ext4_resize_end() which calls clear_bit_unlock() to unlock and to
> avoid -EBUSY errors for the next resize operations.
>
> This patch adds the missing ext4_resize_end() calls on error paths.
>
> Patch tested.
>
> Cc: [email protected]
> Signed-off-by: Djalal Harouni <[email protected]>

Applied, thanks.

- Ted