On Tue, 24 Feb 2015 20:04:16 +0100, Andreas Rohner wrote:
> This patch adds a new feature flag NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS,
> which allows the user to enable and disable the tracking of live
> blocks. The flag can be set at file system creation time with mkfs or
> at any later time with nilfs-tune.
> 
> Additionally a new option NILFS_OPT_TRACK_LIVE_BLKS is added to be
> used by the GC. It is set to the same value as
> NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS at startup. It is mainly used to
> easily and efficiently check for the feature at runtime and to disable
> it if the kernel doesn't support it.
> 
> It is fully backwards compatible, because
> NILFS_FEATURE_COMPAT_SUFILE_EXTENSION also is backwards compatible and
> it basically only tells the kernel to update a counter for every
> segment in the SUFILE. If the kernel doesn't support it, the counter
> won't be updated and the GC policies depending on that information
> will work less efficient, but they would still work.
> 
> Signed-off-by: Andreas Rohner <[email protected]>
> ---
>  include/nilfs.h              | 30 +++++++++++++++++++++++++++---
>  include/nilfs2_fs.h          |  4 +++-
>  lib/feature.c                |  2 ++
>  lib/nilfs.c                  | 32 ++++----------------------------
>  man/mkfs.nilfs2.8            |  6 ++++++
>  sbin/mkfs/mkfs.c             |  3 ++-
>  sbin/nilfs-tune/nilfs-tune.c |  4 ++--
>  7 files changed, 46 insertions(+), 35 deletions(-)
> 
> diff --git a/include/nilfs.h b/include/nilfs.h
> index f695f48..22a9190 100644
> --- a/include/nilfs.h
> +++ b/include/nilfs.h
> @@ -130,6 +130,7 @@ struct nilfs {
>  
>  #define NILFS_OPT_MMAP               0x01
>  #define NILFS_OPT_SET_SUINFO 0x02
> +#define NILFS_OPT_TRACK_LIVE_BLKS    0x04
>  
>  
>  struct nilfs *nilfs_open(const char *, const char *, int);
> @@ -141,9 +142,25 @@ void nilfs_opt_clear_mmap(struct nilfs *);
>  int nilfs_opt_set_mmap(struct nilfs *);
>  int nilfs_opt_test_mmap(struct nilfs *);
>  
> -void nilfs_opt_clear_set_suinfo(struct nilfs *);
> -int nilfs_opt_set_set_suinfo(struct nilfs *);
> -int nilfs_opt_test_set_suinfo(struct nilfs *);
> +#define NILFS_OPT_FLAG(flag, name)                                   \
> +static inline void                                                   \
> +nilfs_opt_set_##name(struct nilfs *nilfs)                    \
> +{                                                                    \
> +     nilfs->n_opts |= NILFS_OPT_##flag;              \
> +}                                                                    \
> +static inline void                                                   \
> +nilfs_opt_clear_##name(struct nilfs *nilfs)                  \
> +{                                                                    \
> +     nilfs->n_opts &= ~NILFS_OPT_##flag;             \
> +}                                                                    \
> +static inline int                                                    \
> +nilfs_opt_test_##name(const struct nilfs *nilfs)                     \
> +{                                                                    \
> +     return !!(nilfs->n_opts & NILFS_OPT_##flag);    \
> +}

Don't break library compatibility by inlining.  Al least this should
be done in a separate patch.

I think we should do the opposite thing.  I mean that the
implementation of nilfs structure should be hidden in nilfs library
sometime when we will bump up the library version because it will
break the library compatibility.

> +
> +NILFS_OPT_FLAG(SET_SUINFO, set_suinfo);
> +NILFS_OPT_FLAG(TRACK_LIVE_BLKS, track_live_blks);
>  
>  nilfs_cno_t nilfs_get_oldest_cno(struct nilfs *);
>  
> @@ -326,4 +343,11 @@ static inline __u32 nilfs_get_blocks_per_segment(const 
> struct nilfs *nilfs)
>       return le32_to_cpu(nilfs->n_sb->s_blocks_per_segment);
>  }
>  
> +static inline int nilfs_feature_track_live_blks(const struct nilfs *nilfs)
> +{
> +     __u64 fc = le64_to_cpu(nilfs->n_sb->s_feature_compat);
> +     return (fc & NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS) &&
> +             (fc & NILFS_FEATURE_COMPAT_SUFILE_EXTENSION);
> +}
> +
>  #endif       /* NILFS_H */
> diff --git a/include/nilfs2_fs.h b/include/nilfs2_fs.h
> index d01a924..427ca53 100644
> --- a/include/nilfs2_fs.h
> +++ b/include/nilfs2_fs.h
> @@ -220,10 +220,12 @@ struct nilfs_super_block {
>   * doesn't know about, it should refuse to mount the filesystem.
>   */
>  #define NILFS_FEATURE_COMPAT_SUFILE_EXTENSION                (1ULL << 0)
> +#define NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS         (1ULL << 1)
>  
>  #define NILFS_FEATURE_COMPAT_RO_BLOCK_COUNT          (1ULL << 0)
>  
> -#define NILFS_FEATURE_COMPAT_SUPP    NILFS_FEATURE_COMPAT_SUFILE_EXTENSION
> +#define NILFS_FEATURE_COMPAT_SUPP    (NILFS_FEATURE_COMPAT_SUFILE_EXTENSION \
> +                             | NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS)
>  #define NILFS_FEATURE_COMPAT_RO_SUPP NILFS_FEATURE_COMPAT_RO_BLOCK_COUNT
>  #define NILFS_FEATURE_INCOMPAT_SUPP  0ULL
>  
> diff --git a/lib/feature.c b/lib/feature.c
> index d954cda..ebe8c3f 100644
> --- a/lib/feature.c
> +++ b/lib/feature.c
> @@ -57,6 +57,8 @@ static const struct nilfs_feature features[] = {
>       /* Compat features */
>       { NILFS_FEATURE_TYPE_COMPAT,
>         NILFS_FEATURE_COMPAT_SUFILE_EXTENSION, "sufile_ext" },
> +     { NILFS_FEATURE_TYPE_COMPAT,
> +       NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS, "track_live_blks" },
>       /* Read-only compat features */
>       { NILFS_FEATURE_TYPE_COMPAT_RO,
>         NILFS_FEATURE_COMPAT_RO_BLOCK_COUNT, "block_count" },
> diff --git a/lib/nilfs.c b/lib/nilfs.c
> index 30db654..2067fc0 100644
> --- a/lib/nilfs.c
> +++ b/lib/nilfs.c
> @@ -290,34 +290,6 @@ int nilfs_opt_test_mmap(struct nilfs *nilfs)
>       return !!(nilfs->n_opts & NILFS_OPT_MMAP);
>  }
>  
> -/**
> - * nilfs_opt_set_set_suinfo - set set_suinfo option
> - * @nilfs: nilfs object
> - */
> -int nilfs_opt_set_set_suinfo(struct nilfs *nilfs)
> -{
> -     nilfs->n_opts |= NILFS_OPT_SET_SUINFO;
> -     return 0;
> -}
> -
> -/**
> - * nilfs_opt_clear_set_suinfo - clear set_suinfo option
> - * @nilfs: nilfs object
> - */
> -void nilfs_opt_clear_set_suinfo(struct nilfs *nilfs)
> -{
> -     nilfs->n_opts &= ~NILFS_OPT_SET_SUINFO;
> -}
> -
> -/**
> - * nilfs_opt_test_set_suinfo - test whether set_suinfo option is set or not
> - * @nilfs: nilfs object
> - */
> -int nilfs_opt_test_set_suinfo(struct nilfs *nilfs)
> -{
> -     return !!(nilfs->n_opts & NILFS_OPT_SET_SUINFO);
> -}
> -
>  static int nilfs_open_sem(struct nilfs *nilfs)
>  {
>       char semnambuf[NAME_MAX - 4];
> @@ -382,6 +354,7 @@ struct nilfs *nilfs_open(const char *dev, const char 
> *dir, int flags)
>       nilfs->n_dev = NULL;
>       nilfs->n_ioc = NULL;
>       nilfs->n_mincno = NILFS_CNO_MIN;
> +     nilfs->n_opts = 0;

Please fix this as a separate patch.  This is a leak bug even though
it doesn't really matters.

Regards,
Ryusuke Konishi

>       memset(nilfs->n_sems, 0, sizeof(nilfs->n_sems));
>  
>       if (flags & NILFS_OPEN_RAW) {
> @@ -405,6 +378,9 @@ struct nilfs *nilfs_open(const char *dev, const char 
> *dir, int flags)
>                       errno = ENOTSUP;
>                       goto out_fd;
>               }
> +
> +             if (nilfs_feature_track_live_blks(nilfs))
> +                     nilfs_opt_set_track_live_blks(nilfs);
>       }
>  
>       if (flags &
> diff --git a/man/mkfs.nilfs2.8 b/man/mkfs.nilfs2.8
> index 6c9a644..2431ac9 100644
> --- a/man/mkfs.nilfs2.8
> +++ b/man/mkfs.nilfs2.8
> @@ -176,6 +176,12 @@ cannot be disabled, because it changes the ondisk 
> format. Nevertheless it
>  is fully compatible with older versions of the file system. This feature
>  is on by default, because it is fully backwards compatible and can only
>  be set at file system creation time.
> +.TP
> +.B track_live_blks
> +Enables the tracking of live blocks, which might improve the effectiveness of
> +garbage collection, but entails a small runtime overhead. It is important to
> +note, that this feature depends on sufile_ext, which can only be set
> +at file system creation time.
>  .RE
>  .TP
>  .B \-q
> diff --git a/sbin/mkfs/mkfs.c b/sbin/mkfs/mkfs.c
> index 3985262..680311c 100644
> --- a/sbin/mkfs/mkfs.c
> +++ b/sbin/mkfs/mkfs.c
> @@ -1082,7 +1082,8 @@ static inline void check_ctime(time_t ctime)
>  
>  static const __u64 ok_features[NILFS_MAX_FEATURE_TYPES] = {
>       /* Compat */
> -     NILFS_FEATURE_COMPAT_SUFILE_EXTENSION,
> +     NILFS_FEATURE_COMPAT_SUFILE_EXTENSION |
> +     NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS,
>       /* Read-only compat */
>       NILFS_FEATURE_COMPAT_RO_BLOCK_COUNT,
>       /* Incompat */
> diff --git a/sbin/nilfs-tune/nilfs-tune.c b/sbin/nilfs-tune/nilfs-tune.c
> index 60f1d39..7889310 100644
> --- a/sbin/nilfs-tune/nilfs-tune.c
> +++ b/sbin/nilfs-tune/nilfs-tune.c
> @@ -84,7 +84,7 @@ static void nilfs_tune_usage(void)
>  
>  static const __u64 ok_features[NILFS_MAX_FEATURE_TYPES] = {
>       /* Compat */
> -     0,
> +     NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS,
>       /* Read-only compat */
>       NILFS_FEATURE_COMPAT_RO_BLOCK_COUNT,
>       /* Incompat */
> @@ -93,7 +93,7 @@ static const __u64 ok_features[NILFS_MAX_FEATURE_TYPES] = {
>  
>  static const __u64 clear_ok_features[NILFS_MAX_FEATURE_TYPES] = {
>       /* Compat */
> -     0,
> +     NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS,
>       /* Read-only compat */
>       NILFS_FEATURE_COMPAT_RO_BLOCK_COUNT,
>       /* Incompat */
> -- 
> 2.3.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to