| Message ID | 1361433665-16880-1-git-send-email-lczerner@redhat.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, Feb 21, 2013 at 09:01:05AM +0100, Lukas Czerner wrote: > > Moreover when calculating number of root clusters we should be using > macro EXT4_NUM_B2C() instead of EXT4_C2B() otherwise the result will > usually be off by one. That should be "instead of EXT4_B2C()", and I don't think this is true, since the number of blocks should always be a multiple of the cluster ratio. So the use of EXT4_B2C() is a bit of an optimization (it avoids an add and mask operation), but it's probably not a measurable optimizatoin in practice. - Ted -- 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
On Fri, 22 Feb 2013, Theodore Ts'o wrote: > Date: Fri, 22 Feb 2013 00:10:48 -0500 > From: Theodore Ts'o <tytso@mit.edu> > To: Lukas Czerner <lczerner@redhat.com> > Cc: linux-ext4@vger.kernel.org > Subject: Re: [PATCH] ext4: fix free clusters calculation in bigalloc > filesystem > > On Thu, Feb 21, 2013 at 09:01:05AM +0100, Lukas Czerner wrote: > > > > Moreover when calculating number of root clusters we should be using > > macro EXT4_NUM_B2C() instead of EXT4_C2B() otherwise the result will > > usually be off by one. > > That should be "instead of EXT4_B2C()", and I don't think this is > true, since the number of blocks should always be a multiple of the > cluster ratio. So the use of EXT4_B2C() is a bit of an optimization > (it avoids an add and mask operation), but it's probably not a > measurable optimizatoin in practice. > > - Ted Oh right it was EXT4_B2C(), I'll update the description. Even though it should always be a multiple of cluster ratio it's really bad precedence for the use of EXT4_B2C() since it is confusing enough as it is with blocks/clusters. I'll resend. -Lukas -- 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
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c index cf18217..9f4302a 100644 --- a/fs/ext4/balloc.c +++ b/fs/ext4/balloc.c @@ -482,11 +482,11 @@ static int ext4_has_free_clusters(struct ext4_sb_info *sbi, free_clusters = percpu_counter_read_positive(fcc); dirty_clusters = percpu_counter_read_positive(dcc); - root_clusters = EXT4_B2C(sbi, ext4_r_blocks_count(sbi->s_es)); + root_clusters = EXT4_NUM_B2C(sbi, ext4_r_blocks_count(sbi->s_es)); if (free_clusters - (nclusters + root_clusters + dirty_clusters) < EXT4_FREECLUSTERS_WATERMARK) { - free_clusters = EXT4_C2B(sbi, percpu_counter_sum_positive(fcc)); + free_clusters = percpu_counter_sum_positive(fcc); dirty_clusters = percpu_counter_sum_positive(dcc); } /* Check whether we have space after accounting for current
ext4_has_free_clusters() should tell us whether there is enough free clusters to allocate, however number of free clusters in the file system is converted to blocks using EXT4_C2B() which is not only wrong use of the macro (we should have used EXT4_NUM_B2C) but it's also completely wrong concept since everything else is in cluster units. Moreover when calculating number of root clusters we should be using macro EXT4_NUM_B2C() instead of EXT4_C2B() otherwise the result will usually be off by one. As a result number of free clusters is much bigger than it should have been and ext4_has_free_clusters() would return 1 even if there is really not enough free clusters available. Fix this by removing the EXT4_C2B() conversion of free clusters and using EXT4_NUM_B2C() when calculating number of root clusters. This bug affects number of xfstests tests covering file system ENOSPC situation handling. With this patch most of the ENOSPC problems with bigalloc file system disappear, especially the errors caused by delayed allocation not having enough space when the actual allocation is finally requested. Signed-off-by: Lukas Czerner <lczerner@redhat.com> --- fs/ext4/balloc.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)