lkml.org 
[lkml]   [2022]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] ext4: Fix journal_ioprio mount option handling
    On 22/04/18 02:05PM, Ojaswin Mujoo wrote:
    > In __ext4_super() we always overwrote the user specified journal_ioprio
    > value with a default value, expecting parse_apply_sb_mount_options() to
    > later correctly set ctx->journal_ioprio to the user specified value.
    > However, if parse_apply_sb_mount_options() returned early because of
    > empty sbi->es_s->s_mount_opts, the correct journal_ioprio value was
    > never set.

    >
    > This patch fixes __ext4_super() to only use the default value if the
    ^^^ __ext4_fill_super
    > user has not specified any value for journal_ioprio.

    Also the problem is that ext4_parse_param() is called before
    __ext4_fill_super(). Hence when we overwrite ctx->journal_ioprio to default
    value in __ext4_fill_super(), that will end up ignoring the user passed
    journal_ioprio value via mount opts (which was passed earlier in
    ext4_parse_param()).


    >
    > Similarly, the remount behavior was to either use journal_ioprio
    > value specified during initial mount, or use the default value
    > irrespective of the journal_ioprio value specified during remount.
    > This patch modifies this to first check if a new value for ioprio
    > has been passed during remount and apply it. Incase, no new value is
    > passed, use the value specified during initial mount.

    Yup, here also ext4_parse_param() is called before __ext4_remount().
    Hence we should check if the user has passed it's value in mount opts, if not,
    only then we should use the task original ioprio.


    I tested this patch and with the patch applied, the task ioprio can be correctly
    set using "journal_ioprio" mount option.

    "Mount test"
    =============
    qemu-> sudo perf record -e probe:* -aR mount -o journal_ioprio=1 /dev/loop2 /mnt
    qemu-> ps -eaf |grep -E "jbd2|loop2"
    root 3506 2 0 07:41 ? 00:00:00 [jbd2/loop2-8]
    qemu-> sudo perf script
    mount 3504 [000] 2503.106871: probe:ext4_parse_param_L222: (ffffffff8147817f) journal_ioprio=16385 spec=32
    mount 3504 [000] 2503.106908: probe:__ext4_fill_super_L26: (ffffffff8147a650) journal_ioprio=16385 spec=32
    qemu-> ionice -p 3506
    best-effort: prio 1

    "remount test"
    =================
    qemu-> sudo perf record -e probe:* -aR mount -o remount,journal_ioprio=0 /dev/loop2 /mnt
    qemu-> sudo perf script
    mount 3519 [000] 2544.958850: probe:ext4_parse_param_L222: (ffffffff8147817f) journal_ioprio=16384 spec=32
    mount 3519 [000] 2544.958860: probe:__ext4_remount_L49: (ffffffff81479da2) journal_ioprio=16384 spec=32
    qemu-> ionice -p 3506
    best-effort: prio 0

    "remount with no mount options"
    =================================
    qemu-> sudo perf record -e probe:* -aR mount -o remount /dev/loop2 /mnt
    qemu-> ionice -p 3506
    best-effort: prio 0
    qemu-> sudo perf script
    mount 3530 [000] 2575.964048: probe:__ext4_remount_L49: (ffffffff81479da2) journal_ioprio=16384 spec=0


    >
    > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

    We should add fixes tag too. Can you please confirm if that would be this patch?
    "ext4: Completely separate options parsing and sb setup".

    With that feel free to add below -

    Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>
    Tested-by: Ritesh Harjani <riteshh@linux.ibm.com>


    -ritesh

    > ---
    > fs/ext4/super.c | 15 ++++++++++-----
    > 1 file changed, 10 insertions(+), 5 deletions(-)
    >
    > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
    > index c5a9ffbf7f4f..bfd767c51203 100644
    > --- a/fs/ext4/super.c
    > +++ b/fs/ext4/super.c
    > @@ -4427,7 +4427,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
    > int silent = fc->sb_flags & SB_SILENT;
    >
    > /* Set defaults for the variables that will be set during parsing */
    > - ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
    > + if (!(ctx->spec & EXT4_SPEC_JOURNAL_IOPRIO))
    > + ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
    >
    > sbi->s_inode_readahead_blks = EXT4_DEF_INODE_READAHEAD_BLKS;
    > sbi->s_sectors_written_start =
    > @@ -6289,7 +6290,6 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
    > char *to_free[EXT4_MAXQUOTAS];
    > #endif
    >
    > - ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
    >
    > /* Store the original options */
    > old_sb_flags = sb->s_flags;
    > @@ -6315,9 +6315,14 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
    > } else
    > old_opts.s_qf_names[i] = NULL;
    > #endif
    > - if (sbi->s_journal && sbi->s_journal->j_task->io_context)
    > - ctx->journal_ioprio =
    > - sbi->s_journal->j_task->io_context->ioprio;
    > + if (!(ctx->spec & EXT4_SPEC_JOURNAL_IOPRIO)) {
    > + if (sbi->s_journal && sbi->s_journal->j_task->io_context)
    > + ctx->journal_ioprio =
    > + sbi->s_journal->j_task->io_context->ioprio;
    > + else
    > + ctx->journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
    > +
    > + }
    >
    > ext4_apply_options(fc, sb);
    >
    > --
    > 2.27.0
    >

    \
     
     \ /
      Last update: 2022-05-11 09:50    [W:4.604 / U:1.736 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site