Skip to contentSkip to conversation

ext4: limit group search loop for non-extent files

PATCH In review Eric Sandeen 3 participants 5 messages May 3, 2013 – May 6, 2013 ext4 file system

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ext4: limit group search loop for non-extent files

From: Lachlan McIlroy <[email protected]>

In the case where we are allocating for a non-extent file, we must limit the groups we allocate from to those below 2^32 blocks, and ext4_mb_regular_allocator() attempts to do this initially by putting a cap on ngroups for the subsequent search loop.

However, the initial target group comes in from the allocation context (ac), and it may already be beyond the artificially limited ngroups. In this case, the limit

	if (group == ngroups)
		group = 0;

at the top of the loop is never true, and the loop will run away.

Catch this case inside the loop and reset the search to start at group 0.

[[email protected]: add commit msg & comments]

---

p.s. I suppose this limit could go after the assignment from ac->ac_g_ex.fe_group, depending on your sense of style...

fs/ext4/mballoc.c +5 -1
index ee6614b..f54baee 100644
@@ -1994,7 +1994,11 @@ repeat:
 		group = ac->ac_g_ex.fe_group;
 
 		for (i = 0; i < ngroups; group++, i++) {
-			if (group == ngroups)
+			/*
+			 * Artificially restricted ngroups for non-extent
+			 * files makes group > ngroups possible on first loop.
+			 */
+			if (group >= ngroups)
 				group = 0;
 
 			/* This now checks without needing the buddy page */
#1 lore raw

----- Original Message -----

Quoted 50 lines

From: Lachlan McIlroy <[email protected]>

In the case where we are allocating for a non-extent file, we must limit the groups we allocate from to those below 2^32 blocks, and ext4_mb_regular_allocator() attempts to do this initially by putting a cap on ngroups for the subsequent search loop.

However, the initial target group comes in from the allocation context (ac), and it may already be beyond the artificially limited ngroups. In this case, the limit

	if (group == ngroups)
		group = 0;

at the top of the loop is never true, and the loop will run away.

Catch this case inside the loop and reset the search to start at group 0.

[[email protected]: add commit msg & comments]

---

p.s. I suppose this limit could go after the assignment from ac->ac_g_ex.fe_group, depending on your sense of style...

fs/ext4/mballoc.c +5 -2
index ee6614b..f54baee 100644
@@ -1994,7 +1994,11 @@ repeat:
 		group = ac->ac_g_ex.fe_group;
 
 		for (i = 0; i < ngroups; group++, i++) {
-			if (group == ngroups)
+			/*
+			 * Artificially restricted ngroups for non-extent
+			 * files makes group > ngroups possible on first loop.
+			 */
+			if (group >= ngroups)
 				group = 0;
 
 			/* This now checks without needing the buddy page */
 
--
Quoted 4 lines

To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Thanks for following through with this Eric. Looks good to me.

#3 lore raw

Ack. Yes Ted that's fine with me to add my Signed-off-by. Thanks.

----- Original Message -----

Quoted 14 lines · Eric Sandeen
Quoted 1 line

From: Lachlan McIlroy <[email protected]>

Note: if it's from Lachlan, it needs his Signed-off-by. Lachlan, can you send an Ack saying it's OK for me to add a:

to the commit description?

Thanks,

- Ted

maintainer
#4 lore raw
Quoted 1 line · Lachlan McIlroy

Ack. Yes Ted that's fine with me to add my Signed-off-by. Thanks.

Thanks!

I've also tagged this with a cc:[email protected].

- Ted