linux-ext4 - Re: ENOSPC returned during writepages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 20 Aug 2008 16:16:44 +0530
From:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To:	"Theodore Ts'o" <tytso@....edu>
Cc:	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: ENOSPC returned during writepages

On Wed, Aug 20, 2008 at 11:13:39AM +0530, Aneesh Kumar K.V wrote:
> Hi,
> 
> I am getting this even with the latest patch queue. The test program is
> a modified fsstress with fallocate support.
> 
> mpage_da_map_blocks block allocation failed for inode 377954 at logical
> offset 313 with max blocks 4 with error -28
> mpage_da_map_blocks block allocation failed for inode 336367 at logical
> offset 74 with max blocks 9 with error -28
> mpage_da_map_blocks block allocation failed for inode 345560 at logical
> offset 542 with max blocks 7 with error -28
> This should not happen.!! Data will be lost
> mpage_da_map_blocks block allocation failed for inode 355317 at logical
> offset 152 with max blocks 10 with error -28
> This should not happen.!! Data will be lost
> mpage_da_map_blocks block allocation failed for inode 395261 at logical
> offset 462 with max blocks 1 with error -28
> This should not happen.!! Data will be lost
> mpage_da_map_blocks block allocation failed for inode 323784 at logical
> offset 313 with max blocks 11 with error -28
> This should not happen.!! Data will be lost
> 

I tried this patch. There are still multiple ways we can get wrong free
block count. The patch reduced the number of errors. So we are doing
better with patch. But I guess we can't use the percpu_counter based
free block accounting with delalloc. Without delalloc it is ok even if
we find some wrong free blocks count . The actual block allocation will fail in
that case and we handle it perfectly fine. With delalloc we cannot
afford to fail the block allocation. Should we look at a free block
accounting rewrite using simple ext4_fsblk_t and and a spin lock ?

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index dfe2d4f..00934b1 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -1624,7 +1624,7 @@ ext4_fsblk_t ext4_has_free_blocks(struct ext4_sb_info *sbi,
 		(sbi->s_resgid == 0 || !in_group_p(sbi->s_resgid)))
 		root_blocks = ext4_r_blocks_count(sbi->s_es);
 #ifdef CONFIG_SMP
-	if (free_blocks - root_blocks < FBC_BATCH)
+	if (free_blocks - (nblocks + root_blocks) < FBC_BATCH)
 		free_blocks =
 			percpu_counter_sum_and_set(&sbi->s_freeblocks_counter);
 #endif
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1c289c1..f825470 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1543,7 +1543,14 @@ static int ext4_da_reserve_space(struct inode *inode, int nrblocks)
 	}
 	/* reduce fs free blocks counter */
 	percpu_counter_sub(&sbi->s_freeblocks_counter, total);
-
+	/*
+	 * Now check whether the block count has gone negative.
+	 * Some other CPU could have reserved blocks in between
+	 */
+	if (percpu_counter_read(&sbi->s_freeblocks_counter) < 0) {
+		spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
+		return -ENOSPC;
+	}
 	EXT4_I(inode)->i_reserved_data_blocks += nrblocks;
 	EXT4_I(inode)->i_reserved_meta_blocks = mdblocks;
 
--
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