ext3: Fix ext3 block reservation early ENOSPC issue

ext3: Fix ext3 block reservation early ENOSPC issue

[Mingming Cao]

From: Mingming Cao <cmm@us.ibm.com>

ext3: Fix ext3 nomballoc allocator for ENOSPC

We could run into ENOSPC error on ext3, even when there is free blocks
on the filesystem.

The problem is triggered in the case the goal block group has 0 free blocks
, and the rest block groups are skipped due to the check of "free_blocks 
< windowsz/2". Current code could fall back to non reservation allocation
to prevent early ENOSPC after examing all the block groups with reservation on
, but this code was  bypassed if the reservation window is turned off already,
which is true in this case.

This patch fixed two issues:
1) We don't need to turn off block reservation if the goal block group has 
0 free blocks left and continue search for the rest of block groups.

Current code the intention is to turn off the block reservation if the
 goal allocation group has a few (some) free blocks left (not enough 
for make the desired reservation window),to try to allocation in the
 goal block group, to get better locality. But if the goal blocks have 
0 free blocks,  it should leave the block reservation on, and continues
search for the next block groups,rather than turn off block reservation
completely.

2) we don't need to check the window size if the block reservation is off. 

The problem was originally found and fixed in ext4.

Signed-off-by: Mingming Cao <cmm@us.ibm.com>

---
 fs/ext3/balloc.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6.27-rc3/fs/ext3/balloc.c
===================================================================
--- linux-2.6.27-rc3.orig/fs/ext3/balloc.c	2008-08-12 18:55:39.000000000 -0700
+++ linux-2.6.27-rc3/fs/ext3/balloc.c	2008-08-28 15:52:09.000000000 -0700
@@ -1547,6 +1547,7 @@
 	 * turn off reservation for this allocation
 	 */
 	if (my_rsv && (free_blocks < windowsz)
+		&& (free_blocks > 0)
 		&& (rsv_is_empty(&my_rsv->rsv_window)))
 		my_rsv = NULL;
 
@@ -1585,7 +1586,7 @@
 		 * free blocks is less than half of the reservation
 		 * window size.
 		 */
-		if (free_blocks <= (windowsz/2))
+		if (my_rsv && (free_blocks <= (windowsz/2)))
 			continue;
 
 		brelse(bitmap_bh);

Re: ext3: Fix ext3 block reservation early ENOSPC issue

[Mike Snitzer]

On Thu, Aug 28, 2008 at 7:03 PM, Mingming Cao <cmm@us.ibm.com> wrote:
> From: Mingming Cao <cmm@us.ibm.com>
>
> ext3: Fix ext3 nomballoc allocator for ENOSPC
>
> We could run into ENOSPC error on ext3, even when there is free blocks
> on the filesystem.
>
> The problem is triggered in the case the goal block group has 0 free blocks
> , and the rest block groups are skipped due to the check of "free_blocks
> < windowsz/2". Current code could fall back to non reservation allocation
> to prevent early ENOSPC after examing all the block groups with reservation on
> , but this code was  bypassed if the reservation window is turned off already,
> which is true in this case.

Looks great, nice catch!

Minor nit, the patch description should reference "free_blocks <= (windowsz/2)"


> This patch fixed two issues:
> 1) We don't need to turn off block reservation if the goal block group has
> 0 free blocks left and continue search for the rest of block groups.
>
> Current code the intention is to turn off the block reservation if the
>  goal allocation group has a few (some) free blocks left (not enough
> for make the desired reservation window),to try to allocation in the
>  goal block group, to get better locality. But if the goal blocks have
> 0 free blocks,  it should leave the block reservation on, and continues
> search for the next block groups,rather than turn off block reservation
> completely.
>
> 2) we don't need to check the window size if the block reservation is off.
>
> The problem was originally found and fixed in ext4.
>
> Signed-off-by: Mingming Cao <cmm@us.ibm.com>
>
> ---
>  fs/ext3/balloc.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.27-rc3/fs/ext3/balloc.c
> ===================================================================
> --- linux-2.6.27-rc3.orig/fs/ext3/balloc.c      2008-08-12 18:55:39.000000000 -0700
> +++ linux-2.6.27-rc3/fs/ext3/balloc.c   2008-08-28 15:52:09.000000000 -0700
> @@ -1547,6 +1547,7 @@
>         * turn off reservation for this allocation
>         */
>        if (my_rsv && (free_blocks < windowsz)
> +               && (free_blocks > 0)
>                && (rsv_is_empty(&my_rsv->rsv_window)))
>                my_rsv = NULL;

Should the comment block right above this change be adjusted with
something like?:

         /*
-         * if there is not enough free blocks to make a new resevation
-         * turn off reservation for this allocation
+         * if there is not enough free blocks to make a new reservation
+         * turn off reservation for this allocation; if the current block
+         * group has 0 free blocks keep reservation enabled and search
+         * the rest of the block groups
          */

Mike