[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130405190136.GA20196@andromeda.usersys.redhat.com>
Date: Fri, 5 Apr 2013 16:01:37 -0300
From: Carlos Maiolino <cmaiolino@...hat.com>
To: Lukas Czerner <lczerner@...hat.com>
Cc: linux-ext4@...r.kernel.org
Subject: Re: [RFC][PATCH] ext4: introduce reserved space
Hi Lukas,
the patch is really good IMHO, but, there is one thing I think I should comment:
> if (blks < 0) {
> struct super_block *sb = mpd->inode->i_sb;
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 9379b7f..6ef160ae 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -81,6 +81,7 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly);
> static void ext4_destroy_lazyinit_thread(void);
> static void ext4_unregister_li_request(struct super_block *sb);
> static void ext4_clear_request_list(void);
> +static int ext4_reserve_clusters(struct ext4_sb_info *, ext4_fsblk_t);
>
> #if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
> static struct file_system_type ext2_fs_type = {
> @@ -2375,6 +2376,17 @@ struct ext4_attr {
> int offset;
> };
>
> +static int parse_strtoull(const char *buf,
> + unsigned long long max, unsigned long long *value)
> +{
> + int ret;
> +
> + ret = kstrtoull(skip_spaces(buf), 0, value);
> + if (!ret && *value > max)
^^^ IMHO this should be replaced by a logical OR, instead of
AND, I believe both conditions here should fail to a
-EINVAL
> + ret = -EINVAL;
> + return ret;
> +}
> +
> static int parse_strtoul(const char *buf,
> unsigned long max, unsigned long *value)
> {
> @@ -2459,6 +2471,27 @@ static ssize_t sbi_ui_store(struct ext4_attr *a,
> return count;
> }
>
> +static ssize_t reserved_clusters_show(struct ext4_attr *a,
> + struct ext4_sb_info *sbi, char *buf)
> +{
> + return snprintf(buf, PAGE_SIZE, "%lu\n",
> + atomic64_read(&sbi->s_resv_clusters));
> +}
> +
> +static ssize_t reserved_clusters_store(struct ext4_attr *a,
> + struct ext4_sb_info *sbi,
> + const char *buf, size_t count)
> +{
> + unsigned long long val;
> + int ret;
> +
> + if (parse_strtoull(buf, -1ULL, &val))
> + return -EINVAL;
> + ret = ext4_reserve_clusters(sbi, val);
> +
> + return ret ? ret : count;
> +}
> +
> static ssize_t trigger_test_error(struct ext4_attr *a,
> struct ext4_sb_info *sbi,
> const char *buf, size_t count)
> @@ -2496,6 +2529,7 @@ static struct ext4_attr ext4_attr_##name = __ATTR(name, mode, show, store)
> EXT4_RO_ATTR(delayed_allocation_blocks);
> EXT4_RO_ATTR(session_write_kbytes);
> EXT4_RO_ATTR(lifetime_write_kbytes);
> +EXT4_RW_ATTR(reserved_clusters);
> EXT4_ATTR_OFFSET(inode_readahead_blks, 0644, sbi_ui_show,
> inode_readahead_blks_store, s_inode_readahead_blks);
> EXT4_RW_ATTR_SBI_UI(inode_goal, s_inode_goal);
> @@ -2513,6 +2547,7 @@ static struct attribute *ext4_attrs[] = {
> ATTR_LIST(delayed_allocation_blocks),
> ATTR_LIST(session_write_kbytes),
> ATTR_LIST(lifetime_write_kbytes),
> + ATTR_LIST(reserved_clusters),
> ATTR_LIST(inode_readahead_blks),
> ATTR_LIST(inode_goal),
> ATTR_LIST(mb_stats),
> @@ -3188,6 +3223,40 @@ int ext4_calculate_overhead(struct super_block *sb)
> return 0;
> }
>
> +
> +static ext4_fsblk_t ext4_calculate_resv_clusters(struct ext4_sb_info *sbi)
> +{
> + ext4_fsblk_t resv_clusters;
> +
> + /*
> + * By default we reserve 2% or 4096 clusters, whichever is smaller.
> + * This should cover the situations where we can not afford to run
> + * out of space like for example punch hole, or converting
> + * uninitialized extents in delalloc path. In most cases such
> + * allocation would require 1, or 2 blocks, higher numbers are
> + * very rare.
> + */
> + resv_clusters = ext4_blocks_count(sbi->s_es) >> sbi->s_cluster_bits;
> +
> + do_div(resv_clusters, 50);
> + resv_clusters = min_t(ext4_fsblk_t, resv_clusters, 4096);
> +
> + return resv_clusters;
> +}
> +
> +
> +static int ext4_reserve_clusters(struct ext4_sb_info *sbi, ext4_fsblk_t count)
> +{
> + s64 free_clusters = percpu_counter_sum_positive(
> + &sbi->s_freeclusters_counter);
> +
> + if (count >= free_clusters)
> + return -EINVAL;
> +
> + atomic64_set(&sbi->s_resv_clusters, count);
> + return 0;
> +}
> +
> static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> {
> char *orig_data = kstrdup(data, GFP_KERNEL);
> @@ -3907,6 +3976,13 @@ no_journal:
> "available");
> }
>
> + err = ext4_reserve_clusters(sbi, ext4_calculate_resv_clusters(sbi));
> + if (err) {
> + ext4_msg(sb, KERN_ERR, "failed to reserve %llu clusters for "
> + "reserved pool", ext4_calculate_resv_clusters(sbi));
> + goto failed_mount4a;
> + }
> +
> err = ext4_setup_system_zone(sb);
> if (err) {
> ext4_msg(sb, KERN_ERR, "failed to initialize system "
> @@ -4738,9 +4814,10 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
> struct super_block *sb = dentry->d_sb;
> struct ext4_sb_info *sbi = EXT4_SB(sb);
> struct ext4_super_block *es = sbi->s_es;
> - ext4_fsblk_t overhead = 0;
> + ext4_fsblk_t overhead = 0, resv_blocks;
> u64 fsid;
> s64 bfree;
> + resv_blocks = EXT4_C2B(sbi, atomic64_read(&sbi->s_resv_clusters));
>
> if (!test_opt(sb, MINIX_DF))
> overhead = sbi->s_overhead;
> @@ -4752,8 +4829,9 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
> percpu_counter_sum_positive(&sbi->s_dirtyclusters_counter);
> /* prevent underflow in case that few free space is available */
> buf->f_bfree = EXT4_C2B(sbi, max_t(s64, bfree, 0));
> - buf->f_bavail = buf->f_bfree - ext4_r_blocks_count(es);
> - if (buf->f_bfree < ext4_r_blocks_count(es))
> + buf->f_bavail = buf->f_bfree -
> + (ext4_r_blocks_count(es) + resv_blocks);
> + if (buf->f_bfree < (ext4_r_blocks_count(es) + resv_blocks))
> buf->f_bavail = 0;
> buf->f_files = le32_to_cpu(es->s_inodes_count);
> buf->f_ffree = percpu_counter_sum_positive(&sbi->s_freeinodes_counter);
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Carlos
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists