Re: [Jfs-discussion] [PATCH] jfs: Several bugs in jfs_freeze() and jfs_unfreeze()

[Vahram M. <vma...@gm...>]

Hi, Dave.

This patch looks much better. We have no objections. Thanks.

Vahram

On Sat, May 25, 2013 at 2:01 AM, Dave Kleikamp <dav...@or...>wrote:

> Vahram,
> This is your first patch modified to be a little more robust. If there is
> no objection, I'll push it to linux-next along with Gu's nointegrity
> patch.
>
> From: Vahram Martirosyan <vma...@gm...>
> Date: Fri, 24 May 2013 13:57:12 +0500
>
> The mentioned functions do not pay attention to the error codes returned
> by the functions updateSuper(), lmLogInit() and lmLogShutdown(). It brings
> to system crash later when writing to log.
>
> The patch adds corresponding code to check and return the error codes
> and to print correct error messages in case of errors.
>
> Found by Linux File System Verification project (linuxtesting.org).
>
> Signed-off-by: Vahram Martirosyan <vah...@li...>
> Reviewed-by: Gu Zheng <guz...@cn...>
> Signed-off-by: Dave Kleikamp <dav...@or...>
> ---
>  fs/jfs/super.c | 38 ++++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index 2003e83..788e0a9 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -611,11 +611,28 @@ static int jfs_freeze(struct super_block *sb)
>  {
>         struct jfs_sb_info *sbi = JFS_SBI(sb);
>         struct jfs_log *log = sbi->log;
> +       int rc = 0;
>
>         if (!(sb->s_flags & MS_RDONLY)) {
>                 txQuiesce(sb);
> -               lmLogShutdown(log);
> -               updateSuper(sb, FM_CLEAN);
> +               rc = lmLogShutdown(log);
> +               if (rc) {
> +                       jfs_error(sb, "jfs_freeze: lmLogShutdown failed");
> +
> +                       /* let operations fail rather than hang */
> +                       txResume(sb);
> +
> +                       return rc;
> +               }
> +               rc = updateSuper(sb, FM_CLEAN);
> +               if (rc) {
> +                       jfs_err("jfs_freeze: updateSuper failed\n");
> +                       /*
> +                        * Don't fail here. Everything succeeded except
> +                        * marking the superblock clean, so there's really
> +                        * no harm in leaving it frozen for now.
> +                        */
> +               }
>         }
>         return 0;
>  }
> @@ -627,13 +644,18 @@ static int jfs_unfreeze(struct super_block *sb)
>         int rc = 0;
>
>         if (!(sb->s_flags & MS_RDONLY)) {
> -               updateSuper(sb, FM_MOUNT);
> -               if ((rc = lmLogInit(log)))
> -                       jfs_err("jfs_unlock failed with return code %d",
> rc);
> -               else
> -                       txResume(sb);
> +               rc = updateSuper(sb, FM_MOUNT);
> +               if (rc) {
> +                       jfs_error(sb, "jfs_unfreeze: updateSuper failed");
> +                       goto out;
> +               }
> +               rc = lmLogInit(log);
> +               if (rc)
> +                       jfs_error(sb, "jfs_unfreeze: lmLogInit failed");
> +out:
> +               txResume(sb);
>         }
> -       return 0;
> +       return rc;
>  }
>
>  static struct dentry *jfs_do_mount(struct file_system_type *fs_type,
> --
> 1.8.2.3
>