| Message ID | 1515359275.3044.4.camel@nixnuts.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi John,
Sorry for reply you so late since too busy these days.
Thanks for your contribution for this issue.
Thanks to the reproducer you provided, I have reproduced the crash issue you
reported.
The back trace was found.
ocfs2_mark_extent_written
ocfs2_change_extent_flag
ocfs2_split_extent
ocfs2_split_and_insert
ocfs2_grow_tree
ocfs2_add_branch
ocfs2_create_new_meta_bhs
ocfs2_claim_metadata
I will post my v3 patch today.
My direction to fix above crash issue has something similar to yours.
What the difference is I revised ocfs2_reuse_blk_from_dealloc() adding a
new parameter to it.
Thanks,
Changwei
On 2018/1/8 5:08, John Lightsey wrote:
> On Fri, 2017-12-29 at 22:36 +0000, Ge Changwei wrote:
>> Hi john,
>>
>> Thanks for your test.
>> I am on a vacation until 3, January.
>> I will investigate the issue when I am back to work.
>>
>
> From what I can tell, the order of reclaiming blocs vs allocating new
> ones just needs to be reversed for everything to work correctly.
>
> This patch on top of yours works in my testing.
>
> I'm not certain my test scenario is hitting ocfs2_add_branch() with
> new_blocks > 1 and meta_ac != NULL though.
>
>
> From 317cbd5a7c8ee5f8f6dff3844d23d3169db990a4 Mon Sep 17 00:00:00 2001
> From: John Lightsey <john@nixnuts.net>
> Date: Sun, 7 Jan 2018 14:43:21 -0600
> Subject: [PATCH] Reuse deallocated extents before claiming new extents.
>
> By reusing deallocated extents before attempting to claim new
> extents, this avoids a condition where the extent tree is truncated,
> and then an allocation requests are made for more extents than were
> originally reserved.
> ---
> fs/ocfs2/alloc.c | 26 +++++++++++++++++++-------
> fs/ocfs2/aops.c | 3 +--
> 2 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 89807ea7..dce2eaa7 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -1166,7 +1166,7 @@ static int ocfs2_add_branch(handle_t *handle,
> struct buffer_head **last_eb_bh,
> struct ocfs2_alloc_context *meta_ac)
> {
> - int status, new_blocks, i;
> + int status, new_blocks, reclaimed_blocks, i;
> u64 next_blkno, new_last_eb_blk;
> struct buffer_head *bh;
> struct buffer_head **new_eb_bhs = NULL;
> @@ -1222,8 +1222,20 @@ static int ocfs2_add_branch(handle_t *handle,
> }
>
> if (meta_ac) {
> - status = ocfs2_create_new_meta_bhs(handle, et, new_blocks,
> - meta_ac, new_eb_bhs);
> + reclaimed_blocks = 0;
> + while ( reclaimed_blocks < new_blocks && !ocfs2_is_dealloc_empty(et) ) {
> + status = ocfs2_reuse_blk_from_dealloc(handle, et,
> + new_eb_bhs + reclaimed_blocks, 1);
> + if (status < 0) {
> + mlog_errno(status);
> + goto bail;
> + }
> + reclaimed_blocks++;
> + }
> + if (reclaimed_blocks < new_blocks) {
> + status = ocfs2_create_new_meta_bhs(handle, et, new_blocks - reclaimed_blocks,
> + meta_ac, new_eb_bhs + reclaimed_blocks);
> + }
> } else if (!ocfs2_is_dealloc_empty(et)) {
> status = ocfs2_reuse_blk_from_dealloc(handle, et,
> new_eb_bhs, new_blocks);
> @@ -1362,12 +1374,12 @@ static int ocfs2_shift_tree_depth(handle_t *handle,
> struct ocfs2_extent_list *root_el;
> struct ocfs2_extent_list *eb_el;
>
> - if (meta_ac) {
> - status = ocfs2_create_new_meta_bhs(handle, et, 1, meta_ac,
> - &new_eb_bh);
> - } else if (!ocfs2_is_dealloc_empty(et)) {
> + if (!ocfs2_is_dealloc_empty(et)) {
> status = ocfs2_reuse_blk_from_dealloc(handle, et,
> &new_eb_bh, 1);
> + } else if (meta_ac) {
> + status = ocfs2_create_new_meta_bhs(handle, et, 1, meta_ac,
> + &new_eb_bh);
> } else {
> BUG();
> }
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index dcea6d5c..8ad37965 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
> @@ -1743,8 +1743,7 @@ int ocfs2_write_begin_nolock(struct address_space *mapping,
> ocfs2_init_dinode_extent_tree(&et, INODE_CACHE(inode),
> wc->w_di_bh);
> ret = ocfs2_lock_allocators(inode, &et,
> - clusters_to_alloc,
> - 2*extents_to_split,
> + clusters_to_alloc, extents_to_split,
> &data_ac, &meta_ac);
> if (ret) {
> mlog_errno(ret);
>
diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c index 89807ea7..dce2eaa7 100644 --- a/fs/ocfs2/alloc.c +++ b/fs/ocfs2/alloc.c @@ -1166,7 +1166,7 @@ static int ocfs2_add_branch(handle_t *handle, struct buffer_head **last_eb_bh, struct ocfs2_alloc_context *meta_ac) { - int status, new_blocks, i; + int status, new_blocks, reclaimed_blocks, i; u64 next_blkno, new_last_eb_blk; struct buffer_head *bh; struct buffer_head **new_eb_bhs = NULL; @@ -1222,8 +1222,20 @@ static int ocfs2_add_branch(handle_t *handle, } if (meta_ac) { - status = ocfs2_create_new_meta_bhs(handle, et, new_blocks, - meta_ac, new_eb_bhs); + reclaimed_blocks = 0; + while ( reclaimed_blocks < new_blocks && !ocfs2_is_dealloc_empty(et) ) { + status = ocfs2_reuse_blk_from_dealloc(handle, et, + new_eb_bhs + reclaimed_blocks, 1); + if (status < 0) { + mlog_errno(status); + goto bail; + } + reclaimed_blocks++; + } + if (reclaimed_blocks < new_blocks) { + status = ocfs2_create_new_meta_bhs(handle, et, new_blocks - reclaimed_blocks, + meta_ac, new_eb_bhs + reclaimed_blocks); + } } else if (!ocfs2_is_dealloc_empty(et)) { status = ocfs2_reuse_blk_from_dealloc(handle, et, new_eb_bhs, new_blocks); @@ -1362,12 +1374,12 @@ static int ocfs2_shift_tree_depth(handle_t *handle, struct ocfs2_extent_list *root_el; struct ocfs2_extent_list *eb_el; - if (meta_ac) { - status = ocfs2_create_new_meta_bhs(handle, et, 1, meta_ac, - &new_eb_bh); - } else if (!ocfs2_is_dealloc_empty(et)) { + if (!ocfs2_is_dealloc_empty(et)) { status = ocfs2_reuse_blk_from_dealloc(handle, et, &new_eb_bh, 1); + } else if (meta_ac) { + status = ocfs2_create_new_meta_bhs(handle, et, 1, meta_ac, + &new_eb_bh); } else { BUG(); } diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index dcea6d5c..8ad37965 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -1743,8 +1743,7 @@ int ocfs2_write_begin_nolock(struct address_space *mapping, ocfs2_init_dinode_extent_tree(&et, INODE_CACHE(inode), wc->w_di_bh); ret = ocfs2_lock_allocators(inode, &et, - clusters_to_alloc, - 2*extents_to_split, + clusters_to_alloc, extents_to_split, &data_ac, &meta_ac); if (ret) { mlog_errno(ret);