2015-01-08 20:48:29

by Josh Hunt

[permalink] [raw]
Subject: [PATCH] ext4: fix warning in ext4_da_update_reserve_space()

Please add the patch below and referenced dependency (0e95c22) to stable
3.10.y. The original 7d73453 did not apply cleanly to my 3.10.y so I've
backported it here. Its dependency can be cherry-picked.

I've gotten approval to submit these to 3.10 stable from Jan and Ted, also
cc'd.
http://marc.info/?l=linux-ext4&m=142070334408249&w=2

Thanks
Josh

-----

commit 7d7345322d60edb0fa49a64a89b31360f01d09cb upstream

reaim workfile.dbase test easily triggers warning in
ext4_da_update_reserve_space():

EXT4-fs warning (device ram0): ext4_da_update_reserve_space:365:
ino 12, allocated 1 with only 0 reserved metadata blocks (releasing 1
blocks with reserved 9 data blocks)

The problem is that (one of) tests creates file and then randomly writes
to it with O_SYNC. That results in writing back pages of the file in
random order so we create extents for written blocks say 0, 2, 4, 6, 8
- this last allocation also allocates new block for extents. Then we
writeout block 1 so we have extents 0-2, 4, 6, 8 and we release
indirect extent block because extents fit in the inode again. Then we
writeout block 10 and we need to allocate indirect extent block again
which triggers the warning because we don't have the reservation
anymore.

Fix the problem by giving back freed metadata blocks resulting from
extent merging into inode's reservation pool.

Cc: <[email protected]> # 3.10.x: 0e95c22: quota: provide interface for readding allocated space into reserved space
Cc: <[email protected]> # 3.10.x
Signed-off-by: Jan Kara <[email protected]>
---
fs/ext4/ext4.h | 1 +
fs/ext4/extents.c | 3 ++-
fs/ext4/mballoc.c | 21 +++++++++++++++++----
3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index e4c4ac0..1a487fb 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -589,6 +589,7 @@ enum {
#define EXT4_FREE_BLOCKS_NO_QUOT_UPDATE 0x0008
#define EXT4_FREE_BLOCKS_NOFREE_FIRST_CLUSTER 0x0010
#define EXT4_FREE_BLOCKS_NOFREE_LAST_CLUSTER 0x0020
+#define EXT4_FREE_BLOCKS_RESERVE 0x0040

/*
* Flags used by ext4_discard_partial_page_buffers
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 84d817b..7fbd1c5 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -1722,7 +1722,8 @@ static void ext4_ext_try_to_merge_up(handle_t *handle,

brelse(path[1].p_bh);
ext4_free_blocks(handle, inode, NULL, blk, 1,
- EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
+ EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET |
+ EXT4_FREE_BLOCKS_RESERVE);
}

/*
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 162b80d..df5050f 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4610,6 +4610,7 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
struct buffer_head *gd_bh;
ext4_group_t block_group;
struct ext4_sb_info *sbi;
+ struct ext4_inode_info *ei = EXT4_I(inode);
struct ext4_buddy e4b;
unsigned int count_clusters;
int err = 0;
@@ -4808,7 +4809,6 @@ do_more:
ext4_block_bitmap_csum_set(sb, block_group, gdp, bitmap_bh);
ext4_group_desc_csum_set(sb, block_group, gdp);
ext4_unlock_group(sb, block_group);
- percpu_counter_add(&sbi->s_freeclusters_counter, count_clusters);

if (sbi->s_log_groups_per_flex) {
ext4_group_t flex_group = ext4_flex_group(sbi, block_group);
@@ -4816,10 +4816,23 @@ do_more:
&sbi->s_flex_groups[flex_group].free_clusters);
}

- ext4_mb_unload_buddy(&e4b);


2015-01-22 03:31:13

by Josh Hunt

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix warning in ext4_da_update_reserve_space()

On 01/08/2015 02:48 PM, Josh Hunt wrote:
> Please add the patch below and referenced dependency (0e95c22) to stable
> 3.10.y. The original 7d73453 did not apply cleanly to my 3.10.y so I've
> backported it here. Its dependency can be cherry-picked.
>
> I've gotten approval to submit these to 3.10 stable from Jan and Ted, also
> cc'd.
> http://marc.info/?l=linux-ext4&m=142070334408249&w=2
>
> Thanks
> Josh
>
> -----
>
> commit 7d7345322d60edb0fa49a64a89b31360f01d09cb upstream
>
> reaim workfile.dbase test easily triggers warning in
> ext4_da_update_reserve_space():
>
> EXT4-fs warning (device ram0): ext4_da_update_reserve_space:365:
> ino 12, allocated 1 with only 0 reserved metadata blocks (releasing 1
> blocks with reserved 9 data blocks)
>
> The problem is that (one of) tests creates file and then randomly writes
> to it with O_SYNC. That results in writing back pages of the file in
> random order so we create extents for written blocks say 0, 2, 4, 6, 8
> - this last allocation also allocates new block for extents. Then we
> writeout block 1 so we have extents 0-2, 4, 6, 8 and we release
> indirect extent block because extents fit in the inode again. Then we
> writeout block 10 and we need to allocate indirect extent block again
> which triggers the warning because we don't have the reservation
> anymore.
>
> Fix the problem by giving back freed metadata blocks resulting from
> extent merging into inode's reservation pool.
>
> Cc: <[email protected]> # 3.10.x: 0e95c22: quota: provide interface for readding allocated space into reserved space
> Cc: <[email protected]> # 3.10.x
> Signed-off-by: Jan Kara <[email protected]>

I've just realized the patch dependency I referenced above, 0e95c22, is
incorrect. It should have been 1c8924eb106. Sorry for any confusion.

Let me know if I you'd like me to respin the patch with the proper hash
information.

Thanks
Josh

2015-01-28 00:12:37

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix warning in ext4_da_update_reserve_space()

On Thu, Jan 08, 2015 at 03:48:28PM -0500, Josh Hunt wrote:
> Please add the patch below and referenced dependency (0e95c22) to stable
> 3.10.y. The original 7d73453 did not apply cleanly to my 3.10.y so I've
> backported it here. Its dependency can be cherry-picked.

The dependancy was 1c8924eb106c1ac755d5d35ce9b3ff42e89e2511, there is
no "0e95c22" in Linus's tree :(

2015-01-28 00:17:18

by Josh Hunt

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix warning in ext4_da_update_reserve_space()

On 01/27/2015 06:12 PM, Greg KH wrote:
> On Thu, Jan 08, 2015 at 03:48:28PM -0500, Josh Hunt wrote:
>> Please add the patch below and referenced dependency (0e95c22) to stable
>> 3.10.y. The original 7d73453 did not apply cleanly to my 3.10.y so I've
>> backported it here. Its dependency can be cherry-picked.
>
> The dependancy was 1c8924eb106c1ac755d5d35ce9b3ff42e89e2511, there is
> no "0e95c22" in Linus's tree :(
>

Yeah, sorry. I sent a correction, but maybe it slipped through the cracks.

Josh

2015-02-03 22:53:43

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix warning in ext4_da_update_reserve_space()

On Thu, Jan 08, 2015 at 03:48:28PM -0500, Josh Hunt wrote:
> Please add the patch below and referenced dependency (0e95c22) to stable
> 3.10.y.

There is no such commit "0e95c22" in Linus's tree, do you mean
"1c8924eb106c1ac755d5d35ce9b3ff42e89e2511"? If so, that doesn't apply
to 3.10 at all, so I can't apply that on, and so, can't apply the patch
below either :(

Care to provide proper backports of both of them?

thanks,

greg k-h

2015-02-03 22:57:24

by Josh Hunt

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix warning in ext4_da_update_reserve_space()

On 02/03/2015 04:53 PM, Greg KH wrote:
> On Thu, Jan 08, 2015 at 03:48:28PM -0500, Josh Hunt wrote:
>> Please add the patch below and referenced dependency (0e95c22) to stable
>> 3.10.y.
>
> There is no such commit "0e95c22" in Linus's tree, do you mean
> "1c8924eb106c1ac755d5d35ce9b3ff42e89e2511"? If so, that doesn't apply
> to 3.10 at all, so I can't apply that on, and so, can't apply the patch
> below either :(
>
> Care to provide proper backports of both of them?
>
> thanks,
>
> greg k-h
>

Hey Greg

I thought we discussed this already. You've already applied these
patches to 3.10.y:

https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-3.10.y&id=23d5efc071f5f47257e5b33c41534a30b1099cc3

https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-3.10.y&id=4e9eb2afbc494a56e35bf6c1a621f579eb1199b2

I messed up the commit id reference in my original submission to you,
but sent a followup. Given that both patches are in 3.10.y now I don't
think there's anything else to do.

Let me know if you need something else from me.

Thanks
Josh

2015-02-03 23:13:27

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix warning in ext4_da_update_reserve_space()

On Tue, Feb 03, 2015 at 04:57:22PM -0600, Josh Hunt wrote:
> On 02/03/2015 04:53 PM, Greg KH wrote:
> >On Thu, Jan 08, 2015 at 03:48:28PM -0500, Josh Hunt wrote:
> >> Please add the patch below and referenced dependency (0e95c22) to stable
> >> 3.10.y.
> >
> >There is no such commit "0e95c22" in Linus's tree, do you mean
> >"1c8924eb106c1ac755d5d35ce9b3ff42e89e2511"? If so, that doesn't apply
> >to 3.10 at all, so I can't apply that on, and so, can't apply the patch
> >below either :(
> >
> >Care to provide proper backports of both of them?
> >
> >thanks,
> >
> >greg k-h
> >
>
> Hey Greg
>
> I thought we discussed this already. You've already applied these patches to
> 3.10.y:
>
> https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-3.10.y&id=23d5efc071f5f47257e5b33c41534a30b1099cc3
>
> https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-3.10.y&id=4e9eb2afbc494a56e35bf6c1a621f579eb1199b2
>
> I messed up the commit id reference in my original submission to you, but
> sent a followup. Given that both patches are in 3.10.y now I don't think
> there's anything else to do.

Ok, that's even better, I like it when I already did something I needed
to do :)

> Let me know if you need something else from me.

As long as it's all ok, I'm happy,

greg k-h

2015-02-03 23:15:36

by Josh Hunt

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix warning in ext4_da_update_reserve_space()

On 02/03/2015 05:13 PM, Greg KH wrote:
> On Tue, Feb 03, 2015 at 04:57:22PM -0600, Josh Hunt wrote:
>> On 02/03/2015 04:53 PM, Greg KH wrote:
>>> On Thu, Jan 08, 2015 at 03:48:28PM -0500, Josh Hunt wrote:
>>>> Please add the patch below and referenced dependency (0e95c22) to stable
>>>> 3.10.y.
>>>
>>> There is no such commit "0e95c22" in Linus's tree, do you mean
>>> "1c8924eb106c1ac755d5d35ce9b3ff42e89e2511"? If so, that doesn't apply
>>> to 3.10 at all, so I can't apply that on, and so, can't apply the patch
>>> below either :(
>>>
>>> Care to provide proper backports of both of them?
>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>
>> Hey Greg
>>
>> I thought we discussed this already. You've already applied these patches to
>> 3.10.y:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-3.10.y&id=23d5efc071f5f47257e5b33c41534a30b1099cc3
>>
>> https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-3.10.y&id=4e9eb2afbc494a56e35bf6c1a621f579eb1199b2
>>
>> I messed up the commit id reference in my original submission to you, but
>> sent a followup. Given that both patches are in 3.10.y now I don't think
>> there's anything else to do.
>
> Ok, that's even better, I like it when I already did something I needed
> to do :)
>
>> Let me know if you need something else from me.
>
> As long as it's all ok, I'm happy,

I'm happy too :D Thanks!

Josh