diff mbox

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

Message ID 1349863655-29320-2-git-send-email-zwu.kernel@gmail.com
State Not Applicable, archived
Headers show

Commit Message

Zhiyong Wu Oct. 10, 2012, 10:07 a.m. UTC
From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>

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

Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 fs/btrfs/ctree.h |    1 +
 fs/btrfs/super.c |    7 ++++++-
 2 files changed, 7 insertions(+), 1 deletions(-)

Comments

Zhiyong Wu Oct. 10, 2012, 12:21 p.m. UTC | #1
On Wed, Oct 10, 2012 at 7:59 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> On Wed, 10 Oct 2012, zwu.kernel@gmail.com wrote:
>
>> Date: Wed, 10 Oct 2012 18:07:23 +0800
>> From: zwu.kernel@gmail.com
>> To: linux-fsdevel@vger.kernel.org
>> Cc: linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org,
>>     linux-kernel@vger.kernel.org, linuxram@linux.vnet.ibm.com,
>>     viro@zeniv.linux.org.uk, david@fromorbit.com, dave@jikos.cz,
>>     tytso@mit.edu, cmm@us.ibm.com, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> Subject: [RFC v3 01/13] btrfs: add one new mount option '-o hot_track'
>>
>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>
>> 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.

>
> -Lukas
>
>>
>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> ---
>>  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"
>>
Lukas Czerner Oct. 10, 2012, 1:11 p.m. UTC | #2
On Wed, 10 Oct 2012, Zhi Yong Wu wrote:

> Date: Wed, 10 Oct 2012 20:21:48 +0800
> From: Zhi Yong Wu <zwu.kernel@gmail.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
>     linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
>     linuxram@linux.vnet.ibm.com, viro@zeniv.linux.org.uk, david@fromorbit.com,
>     dave@jikos.cz, tytso@mit.edu, cmm@us.ibm.com,
>     Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> 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@redhat.com> wrote:
> > On Wed, 10 Oct 2012, zwu.kernel@gmail.com wrote:
> >
> >> Date: Wed, 10 Oct 2012 18:07:23 +0800
> >> From: zwu.kernel@gmail.com
> >> To: linux-fsdevel@vger.kernel.org
> >> Cc: linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org,
> >>     linux-kernel@vger.kernel.org, linuxram@linux.vnet.ibm.com,
> >>     viro@zeniv.linux.org.uk, david@fromorbit.com, dave@jikos.cz,
> >>     tytso@mit.edu, cmm@us.ibm.com, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> >> Subject: [RFC v3 01/13] btrfs: add one new mount option '-o hot_track'
> >>
> >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> >>
> >> 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@linux.vnet.ibm.com>
> >> ---
> >>  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"
> >>
> 
> 
> 
>
Zhiyong Wu Oct. 10, 2012, 1:16 p.m. UTC | #3
On Wed, Oct 10, 2012 at 9:11 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> On Wed, 10 Oct 2012, Zhi Yong Wu wrote:
>
>> Date: Wed, 10 Oct 2012 20:21:48 +0800
>> From: Zhi Yong Wu <zwu.kernel@gmail.com>
>> To: Lukáš Czerner <lczerner@redhat.com>
>> Cc: linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
>>     linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
>>     linuxram@linux.vnet.ibm.com, viro@zeniv.linux.org.uk, david@fromorbit.com,
>>     dave@jikos.cz, tytso@mit.edu, cmm@us.ibm.com,
>>     Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> 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@redhat.com> wrote:
>> > On Wed, 10 Oct 2012, zwu.kernel@gmail.com wrote:
>> >
>> >> Date: Wed, 10 Oct 2012 18:07:23 +0800
>> >> From: zwu.kernel@gmail.com
>> >> To: linux-fsdevel@vger.kernel.org
>> >> Cc: linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org,
>> >>     linux-kernel@vger.kernel.org, linuxram@linux.vnet.ibm.com,
>> >>     viro@zeniv.linux.org.uk, david@fromorbit.com, dave@jikos.cz,
>> >>     tytso@mit.edu, cmm@us.ibm.com, Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> >> Subject: [RFC v3 01/13] btrfs: add one new mount option '-o hot_track'
>> >>
>> >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> >>
>> >> 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.
OK, done, thanks. If you have comments on other patches, it will be appreciated.

>
> Thanks!
> -Lukas
>
>>
>> >
>> > -Lukas
>> >
>> >>
>> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> >> ---
>> >>  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"
>> >>
>>
>>
>>
>>
David Sterba Oct. 10, 2012, 4:28 p.m. UTC | #4
Hi,

On Wed, Oct 10, 2012 at 06:07:23PM +0800, zwu.kernel@gmail.com wrote:
> --- 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)

Please don't forget to add new options to btrfs_show_options(),
otherwise we can't tell what filesystems have hot tracking enabled.

> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -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,

Please add the new option to the end.

>  	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"},

(also this one)

>  	{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:

It's a common patter in the surrounding code that a message is printed
when enabling options, but the vfs prints its own, so I'm not sure if
it's needed here as well. Just thinking, leave it as it is now.

> +			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"

david
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhiyong Wu Oct. 11, 2012, 1:41 p.m. UTC | #5
On Thu, Oct 11, 2012 at 12:28 AM, David Sterba <dave@jikos.cz> wrote:
> Hi,
>
> On Wed, Oct 10, 2012 at 06:07:23PM +0800, zwu.kernel@gmail.com wrote:
>> --- 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)
>
> Please don't forget to add new options to btrfs_show_options(),
> otherwise we can't tell what filesystems have hot tracking enabled.
Great catch, thanks.
>
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -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,
>
> Please add the new option to the end.
OK.
>
>>       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"},
>
> (also this one)
ditto.
>
>>       {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:
>
> It's a common patter in the surrounding code that a message is printed
> when enabling options, but the vfs prints its own, so I'm not sure if
> it's needed here as well. Just thinking, leave it as it is now.
OK
>
>> +                     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"
>
> david
Zhiyong Wu Oct. 11, 2012, 2:35 p.m. UTC | #6
On Thu, Oct 11, 2012 at 12:28 AM, David Sterba <dave@jikos.cz> wrote:
> Hi,
>
> On Wed, Oct 10, 2012 at 06:07:23PM +0800, zwu.kernel@gmail.com wrote:
>> --- 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)
>
> Please don't forget to add new options to btrfs_show_options(),
> otherwise we can't tell what filesystems have hot tracking enabled.
>
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -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,
>
> Please add the new option to the end.
To be honest, it can't be added to the end, if you check Opt_err's
pattern value, you will find it is NULL, it will cause match_one()
return 1. So if we add Opt_hot_track to the end of this array, it will
be covered by match_token(), so i prefer to add it to
Opt_fatal_errors. Do you think of it?
>
>>       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"},
>
> (also this one)
>
>>       {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:
>
> It's a common patter in the surrounding code that a message is printed
> when enabling options, but the vfs prints its own, so I'm not sure if
> it's needed here as well. Just thinking, leave it as it is now.
>
>> +                     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"
>
> david
David Sterba Oct. 11, 2012, 2:41 p.m. UTC | #7
On Thu, Oct 11, 2012 at 10:35:28PM +0800, Zhi Yong Wu wrote:
> >> --- a/fs/btrfs/super.c
> >> +++ b/fs/btrfs/super.c
> >> @@ -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,
> >
> > Please add the new option to the end.
> To be honest, it can't be added to the end, if you check Opt_err's
> pattern value, you will find it is NULL, it will cause match_one()
> return 1. So if we add Opt_hot_track to the end of this array, it will
> be covered by match_token(), so i prefer to add it to
> Opt_fatal_errors. Do you think of it?

Ah, sorry, I was not clear what the 'end' means here. The Opt_err is a
end-of-the-list token, so yes please add hot_track between
Opt_fatal_errors and Opt_err.

david
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhiyong Wu Oct. 11, 2012, 2:46 p.m. UTC | #8
On Thu, Oct 11, 2012 at 10:41 PM, David Sterba <dave@jikos.cz> wrote:
> On Thu, Oct 11, 2012 at 10:35:28PM +0800, Zhi Yong Wu wrote:
>> >> --- a/fs/btrfs/super.c
>> >> +++ b/fs/btrfs/super.c
>> >> @@ -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,
>> >
>> > Please add the new option to the end.
>> To be honest, it can't be added to the end, if you check Opt_err's
>> pattern value, you will find it is NULL, it will cause match_one()
>> return 1. So if we add Opt_hot_track to the end of this array, it will
>> be covered by match_token(), so i prefer to add it to
>> Opt_fatal_errors. Do you think of it?
>
> Ah, sorry, I was not clear what the 'end' means here. The Opt_err is a
> end-of-the-list token, so yes please add hot_track between
> Opt_fatal_errors and Opt_err.
Done, thanks. It will be appreciated if you can make comments on other
patches of this series.:)

>
> david
diff mbox

Patch

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"