Re: [RFC v3 01/13] btrfs: add one new mount option '-o hot_track'

[Lukáš Czerner]

On Wed, 10 Oct 2012, Zhi Yong Wu wrote:

> Date: Wed, 10 Oct 2012 20:21:48 +0800
> From: Zhi Yong Wu <zwu.kernel@xxxxxxxxx>
> To: LukÃÅ Czerner <lczerner@xxxxxxxxxx>
> Cc: linux-fsdevel@xxxxxxxxxxxxxxx, linux-ext4@xxxxxxxxxxxxxxx,
>     linux-btrfs@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx,
>     linuxram@xxxxxxxxxxxxxxxxxx, viro@xxxxxxxxxxxxxxxxxx, david@xxxxxxxxxxxxx,
>     dave@xxxxxxxx, tytso@xxxxxxx, cmm@xxxxxxxxxx,
>     Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
> Subject: Re: [RFC v3 01/13] btrfs: add one new mount option '-o hot_track'
> 
> On Wed, Oct 10, 2012 at 7:59 PM, LukÃÅ Czerner <lczerner@xxxxxxxxxx> wrote:
> > On Wed, 10 Oct 2012, zwu.kernel@xxxxxxxxx wrote:
> >
> >> Date: Wed, 10 Oct 2012 18:07:23 +0800
> >> From: zwu.kernel@xxxxxxxxx
> >> To: linux-fsdevel@xxxxxxxxxxxxxxx
> >> Cc: linux-ext4@xxxxxxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx,
> >>     linux-kernel@xxxxxxxxxxxxxxx, linuxram@xxxxxxxxxxxxxxxxxx,
> >>     viro@xxxxxxxxxxxxxxxxxx, david@xxxxxxxxxxxxx, dave@xxxxxxxx,
> >>     tytso@xxxxxxx, cmm@xxxxxxxxxx, Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
> >> Subject: [RFC v3 01/13] btrfs: add one new mount option '-o hot_track'
> >>
> >> From: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
> >>
> >> Introduce one new mount option '-o hot_track',
> >> and add its parsing support.
> >>   Its usage looks like:
> >>    mount -o hot_track
> >>    mount -o nouser,hot_track
> >>    mount -o nouser,hot_track,loop
> >>    mount -o hot_track,nouser
> >
> > This patch should probably be at the end of the series.
> Can you let me know your reason? I think that it is not necessary to
> be at the end of the series.

Simply because you're adding the mount option which does not do
anything yet. Moreover you change the implementation of the hot track
as you go. You should enable this once it is ready to use, not the other
way around. So, please move this at the end of the patch set when
the feature is supposed to be ready to use.

Thanks!
-Lukas

> 
> >
> > -Lukas
> >
> >>
> >> Signed-off-by: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
> >> ---
> >>  fs/btrfs/ctree.h |    1 +
> >>  fs/btrfs/super.c |    7 ++++++-
> >>  2 files changed, 7 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> >> index 9821b67..094bec6 100644
> >> --- a/fs/btrfs/ctree.h
> >> +++ b/fs/btrfs/ctree.h
> >> @@ -1726,6 +1726,7 @@ struct btrfs_ioctl_defrag_range_args {
> >>  #define BTRFS_MOUNT_CHECK_INTEGRITY  (1 << 20)
> >>  #define BTRFS_MOUNT_CHECK_INTEGRITY_INCLUDING_EXTENT_DATA (1 << 21)
> >>  #define BTRFS_MOUNT_PANIC_ON_FATAL_ERROR     (1 << 22)
> >> +#define BTRFS_MOUNT_HOT_TRACK                (1 << 23)
> >>
> >>  #define btrfs_clear_opt(o, opt)              ((o) &= ~BTRFS_MOUNT_##opt)
> >>  #define btrfs_set_opt(o, opt)                ((o) |= BTRFS_MOUNT_##opt)
> >> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> >> index 83d6f9f..00be9e3 100644
> >> --- a/fs/btrfs/super.c
> >> +++ b/fs/btrfs/super.c
> >> @@ -41,6 +41,7 @@
> >>  #include <linux/slab.h>
> >>  #include <linux/cleancache.h>
> >>  #include <linux/ratelimit.h>
> >> +#include <linux/hot_tracking.h>
> >>  #include "compat.h"
> >>  #include "delayed-inode.h"
> >>  #include "ctree.h"
> >> @@ -303,7 +304,7 @@ enum {
> >>       Opt_notreelog, Opt_ratio, Opt_flushoncommit, Opt_discard,
> >>       Opt_space_cache, Opt_clear_cache, Opt_user_subvol_rm_allowed,
> >>       Opt_enospc_debug, Opt_subvolrootid, Opt_defrag, Opt_inode_cache,
> >> -     Opt_no_space_cache, Opt_recovery, Opt_skip_balance,
> >> +     Opt_no_space_cache, Opt_recovery, Opt_skip_balance, Opt_hot_track,
> >>       Opt_check_integrity, Opt_check_integrity_including_extent_data,
> >>       Opt_check_integrity_print_mask, Opt_fatal_errors,
> >>       Opt_err,
> >> @@ -342,6 +343,7 @@ static match_table_t tokens = {
> >>       {Opt_no_space_cache, "nospace_cache"},
> >>       {Opt_recovery, "recovery"},
> >>       {Opt_skip_balance, "skip_balance"},
> >> +     {Opt_hot_track, "hot_track"},
> >>       {Opt_check_integrity, "check_int"},
> >>       {Opt_check_integrity_including_extent_data, "check_int_data"},
> >>       {Opt_check_integrity_print_mask, "check_int_print_mask=%d"},
> >> @@ -553,6 +555,9 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
> >>               case Opt_skip_balance:
> >>                       btrfs_set_opt(info->mount_opt, SKIP_BALANCE);
> >>                       break;
> >> +             case Opt_hot_track:
> >> +                     btrfs_set_opt(info->mount_opt, HOT_TRACK);
> >> +                     break;
> >>  #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
> >>               case Opt_check_integrity_including_extent_data:
> >>                       printk(KERN_INFO "btrfs: enabling check integrity"
> >>
> 
> 
> 
>