All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Btrfs: remove redundant btrfs_trans_release_metadata"
@ 2018-09-05  1:14 Liu Bo
  2018-09-05  5:58 ` Nikolay Borisov
  2018-09-06  5:45 ` Liu Bo
  0 siblings, 2 replies; 6+ messages in thread
From: Liu Bo @ 2018-09-05  1:14 UTC (permalink / raw)
  To: linux-btrfs

__btrfs_end_transaction() has done the metadata release twice,
probably because it used to process delayed refs in between, but now
that we don't process delayed refs any more, the 2nd release is always
a noop.

Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
---
 fs/btrfs/transaction.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index bb1b9f526e98..94b036a74d11 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -826,12 +826,6 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
 		return 0;
 	}
 
-	btrfs_trans_release_metadata(trans);
-	trans->block_rsv = NULL;
-
-	if (!list_empty(&trans->new_bgs))
-		btrfs_create_pending_block_groups(trans);
-
 	trans->delayed_ref_updates = 0;
 	if (!trans->sync) {
 		must_run_delayed_refs =
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Btrfs: remove redundant btrfs_trans_release_metadata"
  2018-09-05  1:14 [PATCH] Btrfs: remove redundant btrfs_trans_release_metadata" Liu Bo
@ 2018-09-05  5:58 ` Nikolay Borisov
  2018-09-06  5:45 ` Liu Bo
  1 sibling, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2018-09-05  5:58 UTC (permalink / raw)
  To: Liu Bo, linux-btrfs



On  5.09.2018 04:14, Liu Bo wrote:
> __btrfs_end_transaction() has done the metadata release twice,
> probably because it used to process delayed refs in between, but now
> that we don't process delayed refs any more, the 2nd release is always
> a noop.
> 
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/transaction.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index bb1b9f526e98..94b036a74d11 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -826,12 +826,6 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
>  		return 0;
>  	}
>  
> -	btrfs_trans_release_metadata(trans);
> -	trans->block_rsv = NULL;
> -
> -	if (!list_empty(&trans->new_bgs))
> -		btrfs_create_pending_block_groups(trans);
> -

The only code which can have any implications to the transaction reserve
is the btrfs_Create_pending_block_groups since it does insert items. But
at this point trans->block_rsv is already null and additionally even if
more reservations are made for this transaction further down either
btrfs_commit_transaction is called or the transaction kthread is called
which is going to commit it. So this change really seems inconsequential.

>  	trans->delayed_ref_updates = 0;
>  	if (!trans->sync) {
>  		must_run_delayed_refs =
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Btrfs: remove redundant btrfs_trans_release_metadata"
  2018-09-05  1:14 [PATCH] Btrfs: remove redundant btrfs_trans_release_metadata" Liu Bo
  2018-09-05  5:58 ` Nikolay Borisov
@ 2018-09-06  5:45 ` Liu Bo
  2018-09-06  6:47   ` Liu Bo
  1 sibling, 1 reply; 6+ messages in thread
From: Liu Bo @ 2018-09-06  5:45 UTC (permalink / raw)
  To: Liu Bo; +Cc: linux-btrfs

Somehow this ends up with crash in btrfs/124, I'm trying to figure out
what went wrong.

thanks,
liubo


On Tue, Sep 4, 2018 at 6:14 PM, Liu Bo <bo.liu@linux.alibaba.com> wrote:
> __btrfs_end_transaction() has done the metadata release twice,
> probably because it used to process delayed refs in between, but now
> that we don't process delayed refs any more, the 2nd release is always
> a noop.
>
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> ---
>  fs/btrfs/transaction.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index bb1b9f526e98..94b036a74d11 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -826,12 +826,6 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
>                 return 0;
>         }
>
> -       btrfs_trans_release_metadata(trans);
> -       trans->block_rsv = NULL;
> -
> -       if (!list_empty(&trans->new_bgs))
> -               btrfs_create_pending_block_groups(trans);
> -
>         trans->delayed_ref_updates = 0;
>         if (!trans->sync) {
>                 must_run_delayed_refs =
> --
> 1.8.3.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Btrfs: remove redundant btrfs_trans_release_metadata"
  2018-09-06  5:45 ` Liu Bo
@ 2018-09-06  6:47   ` Liu Bo
  2018-09-06 18:50     ` Nikolay Borisov
  0 siblings, 1 reply; 6+ messages in thread
From: Liu Bo @ 2018-09-06  6:47 UTC (permalink / raw)
  To: Liu Bo; +Cc: linux-btrfs

On Wed, Sep 5, 2018 at 10:45 PM, Liu Bo <obuil.liubo@gmail.com> wrote:
> Somehow this ends up with crash in btrfs/124, I'm trying to figure out
> what went wrong.
>

It revealed the problem addressed in Josef's patch[1], so with it,
this patch works just fine.

[1] btrfs: make sure we create all new bgs

thanks,
liubo

>
> On Tue, Sep 4, 2018 at 6:14 PM, Liu Bo <bo.liu@linux.alibaba.com> wrote:
>> __btrfs_end_transaction() has done the metadata release twice,
>> probably because it used to process delayed refs in between, but now
>> that we don't process delayed refs any more, the 2nd release is always
>> a noop.
>>
>> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
>> ---
>>  fs/btrfs/transaction.c | 6 ------
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index bb1b9f526e98..94b036a74d11 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -826,12 +826,6 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
>>                 return 0;
>>         }
>>
>> -       btrfs_trans_release_metadata(trans);
>> -       trans->block_rsv = NULL;
>> -
>> -       if (!list_empty(&trans->new_bgs))
>> -               btrfs_create_pending_block_groups(trans);
>> -
>>         trans->delayed_ref_updates = 0;
>>         if (!trans->sync) {
>>                 must_run_delayed_refs =
>> --
>> 1.8.3.1
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Btrfs: remove redundant btrfs_trans_release_metadata"
  2018-09-06  6:47   ` Liu Bo
@ 2018-09-06 18:50     ` Nikolay Borisov
  2018-09-06 20:25       ` Liu Bo
  0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Borisov @ 2018-09-06 18:50 UTC (permalink / raw)
  To: Liu Bo, Liu Bo; +Cc: linux-btrfs



On  6.09.2018 09:47, Liu Bo wrote:
> On Wed, Sep 5, 2018 at 10:45 PM, Liu Bo <obuil.liubo@gmail.com> wrote:
>> Somehow this ends up with crash in btrfs/124, I'm trying to figure out
>> what went wrong.
>>
> 
> It revealed the problem addressed in Josef's patch[1], so with it,
> this patch works just fine.

What exactly was the crash ?

> 
> [1] btrfs: make sure we create all new bgs
> 
> thanks,
> liubo
> 
>>
>> On Tue, Sep 4, 2018 at 6:14 PM, Liu Bo <bo.liu@linux.alibaba.com> wrote:
>>> __btrfs_end_transaction() has done the metadata release twice,
>>> probably because it used to process delayed refs in between, but now
>>> that we don't process delayed refs any more, the 2nd release is always
>>> a noop.
>>>
>>> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
>>> ---
>>>  fs/btrfs/transaction.c | 6 ------
>>>  1 file changed, 6 deletions(-)
>>>
>>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>>> index bb1b9f526e98..94b036a74d11 100644
>>> --- a/fs/btrfs/transaction.c
>>> +++ b/fs/btrfs/transaction.c
>>> @@ -826,12 +826,6 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
>>>                 return 0;
>>>         }
>>>
>>> -       btrfs_trans_release_metadata(trans);
>>> -       trans->block_rsv = NULL;
>>> -
>>> -       if (!list_empty(&trans->new_bgs))
>>> -               btrfs_create_pending_block_groups(trans);
>>> -
>>>         trans->delayed_ref_updates = 0;
>>>         if (!trans->sync) {
>>>                 must_run_delayed_refs =
>>> --
>>> 1.8.3.1
>>>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Btrfs: remove redundant btrfs_trans_release_metadata"
  2018-09-06 18:50     ` Nikolay Borisov
@ 2018-09-06 20:25       ` Liu Bo
  0 siblings, 0 replies; 6+ messages in thread
From: Liu Bo @ 2018-09-06 20:25 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: Liu Bo, linux-btrfs

On Thu, Sep 6, 2018 at 11:50 AM, Nikolay Borisov <nborisov@suse.com> wrote:
>
>
> On  6.09.2018 09:47, Liu Bo wrote:
>> On Wed, Sep 5, 2018 at 10:45 PM, Liu Bo <obuil.liubo@gmail.com> wrote:
>>> Somehow this ends up with crash in btrfs/124, I'm trying to figure out
>>> what went wrong.
>>>
>>
>> It revealed the problem addressed in Josef's patch[1], so with it,
>> this patch works just fine.
>
> What exactly was the crash ?
>

assertion failed: list_empty(&block_group->bg_list), file:
fs/btrfs/extent-tree.c,

kernel BUG at fs/btrfs/ctree.h:3427!
...
close_ctree+0x142/0x310 [btrfs]

thanks,
liubo



>>
>> [1] btrfs: make sure we create all new bgs
>>
>> thanks,
>> liubo
>>
>>>
>>> On Tue, Sep 4, 2018 at 6:14 PM, Liu Bo <bo.liu@linux.alibaba.com> wrote:
>>>> __btrfs_end_transaction() has done the metadata release twice,
>>>> probably because it used to process delayed refs in between, but now
>>>> that we don't process delayed refs any more, the 2nd release is always
>>>> a noop.
>>>>
>>>> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
>>>> ---
>>>>  fs/btrfs/transaction.c | 6 ------
>>>>  1 file changed, 6 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>>>> index bb1b9f526e98..94b036a74d11 100644
>>>> --- a/fs/btrfs/transaction.c
>>>> +++ b/fs/btrfs/transaction.c
>>>> @@ -826,12 +826,6 @@ static int __btrfs_end_transaction(struct btrfs_trans_handle *trans,
>>>>                 return 0;
>>>>         }
>>>>
>>>> -       btrfs_trans_release_metadata(trans);
>>>> -       trans->block_rsv = NULL;
>>>> -
>>>> -       if (!list_empty(&trans->new_bgs))
>>>> -               btrfs_create_pending_block_groups(trans);
>>>> -
>>>>         trans->delayed_ref_updates = 0;
>>>>         if (!trans->sync) {
>>>>                 must_run_delayed_refs =
>>>> --
>>>> 1.8.3.1
>>>>
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-09-07  1:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-05  1:14 [PATCH] Btrfs: remove redundant btrfs_trans_release_metadata" Liu Bo
2018-09-05  5:58 ` Nikolay Borisov
2018-09-06  5:45 ` Liu Bo
2018-09-06  6:47   ` Liu Bo
2018-09-06 18:50     ` Nikolay Borisov
2018-09-06 20:25       ` Liu Bo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.