2008-12-10 13:41:47

by Aneesh Kumar K.V

[permalink] [raw]
Subject: [PATCH] ext4: Don't overwrite allocation_context ac_status

We can call ext4_mb_check_limits even after successfull
allocation. Make sure we don't overwrite ac_status.
This fix the below lockdep warning

=============================================
[ INFO: possible recursive locking detected ]
2.6.28-rc6-autokern1 #1
---------------------------------------------
fsstress/11948 is trying to acquire lock:
(&meta_group_info[i]->alloc_sem){----}, at: [<c04d9a49>] ext4_mb_load_buddy+0x9f/0x278
.....

stack backtrace:
.....
[<c04db974>] ext4_mb_regular_allocator+0xbb5/0xd44
.....

but task is already holding lock:
(&meta_group_info[i]->alloc_sem){----}, at: [<c04d9a49>] ext4_mb_load_buddy+0x9f/0x278

Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/ext4/mballoc.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 22d31c3..8ce2f19 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -1403,6 +1403,8 @@ static void ext4_mb_check_limits(struct ext4_allocation_context *ac,
struct ext4_free_extent ex;
int max;

+ if (ac->ac_status == AC_STATUS_FOUND)
+ return;
/*
* We don't want to scan for a whole year
*/
--
1.6.1.rc1.56.g2dd62



2008-12-12 17:59:27

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: Don't overwrite allocation_context ac_status

On Wed, Dec 10, 2008 at 06:37:43PM +0530, Aneesh Kumar K.V wrote:
> We can call ext4_mb_check_limits even after successfull
> allocation. Make sure we don't overwrite ac_status.
> This fix the below lockdep warning

So the ext4_mb_check_limits() function isn't that well documented, but
one of the things it is supposed to do is to make sure that blocks
comprising the extent that was found is in fact still available. This
check patches this out. Are we sure this does the right thing?

- Ted

2008-12-17 09:14:52

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: [PATCH] ext4: Don't overwrite allocation_context ac_status

On Fri, Dec 12, 2008 at 12:59:22PM -0500, Theodore Tso wrote:
> On Wed, Dec 10, 2008 at 06:37:43PM +0530, Aneesh Kumar K.V wrote:
> > We can call ext4_mb_check_limits even after successfull
> > allocation. Make sure we don't overwrite ac_status.
> > This fix the below lockdep warning
>
> So the ext4_mb_check_limits() function isn't that well documented, but
> one of the things it is supposed to do is to make sure that blocks
> comprising the extent that was found is in fact still available. This
> check patches this out. Are we sure this does the right thing?

ext4_mb_check_limits does allocation in complex_scan. In complex scan
we loop through available free blocks and try to find the best extent.
We also want to make sure we doesn't loop too much. So
ext4_mb_check_limits and ext4_mb_measure_extent does multiple things.


Now what we do in complex_scan is

for blocks in group:
ext4_mb_find_extent();
ext4_mb_measure_extents()

ext4_mb_check_limits(..., 1);

Now ext4_mb_measure_extents() can result in successfull
allocation. So we need to make sure we don't do further allocation in
ext4_mb_check_limits when we call ext4_mb_check_limits later with
finish_group = 1;


-aneesh


2009-01-02 05:23:14

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: Don't overwrite allocation_context ac_status

Thanks, I've queued this patch into the ext4 patch queue.

- Ted