* [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree
@ 2020-07-09 8:33 Qu Wenruo
2020-07-09 8:33 ` [PATCH v2 2/2] btrfs: relocation: review the call sites which can be interruped by signal Qu Wenruo
2020-07-09 11:05 ` [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree David Sterba
0 siblings, 2 replies; 8+ messages in thread
From: Qu Wenruo @ 2020-07-09 8:33 UTC (permalink / raw)
To: linux-btrfs; +Cc: Josef Bacik
[BUG]
There is a bug report about bad signal timing could lead to read-only
fs during balance:
BTRFS info (device xvdb): balance: start -d -m -s
BTRFS info (device xvdb): relocating block group 73001861120 flags metadata
BTRFS info (device xvdb): found 12236 extents, stage: move data extents
BTRFS info (device xvdb): relocating block group 71928119296 flags data
BTRFS info (device xvdb): found 3 extents, stage: move data extents
BTRFS info (device xvdb): found 3 extents, stage: update data pointers
BTRFS info (device xvdb): relocating block group 60922265600 flags metadata
BTRFS: error (device xvdb) in btrfs_drop_snapshot:5505: errno=-4 unknown
BTRFS info (device xvdb): forced readonly
BTRFS info (device xvdb): balance: ended with status: -4
[CAUSE]
The direct cause is the -EINTR from the following call chain when a
fatal signal is pending:
relocate_block_group()
|- clean_dirty_subvols()
|- btrfs_drop_snapshot()
|- btrfs_start_transaction()
|- btrfs_delayed_refs_rsv_refill()
|- btrfs_reserve_metadata_bytes()
|- __reserve_metadata_bytes()
|- wait_reserve_ticket()
|- prepare_to_wait_event();
|- ticket->error = -EINTR;
Normally this behavior is fine for most btrfs_start_transaction()
callers, as they need to catch the fatal signal and exit asap.
However for balance, especially for the clean_dirty_subvols() case, we're
already doing cleanup works, such -EINTR from btrfs_drop_snapshot()
could cause a lot of unexpected problems.
From the mentioned forced read-only, to later balance error due to half
dropped reloc trees.
[FIX]
Fix this problem by using btrfs_join_transaction() if
btrfs_drop_snapshot() is called from relocation context.
As btrfs_join_transaction() won't wait full tickets, it won't get
interrupted from signal.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
---
fs/btrfs/extent-tree.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index c0bc35f932bf..d8ef48a807d1 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -5298,7 +5298,10 @@ int btrfs_drop_snapshot(struct btrfs_root *root, int update_ref, int for_reloc)
goto out;
}
- trans = btrfs_start_transaction(tree_root, 0);
+ if (for_reloc)
+ trans = btrfs_join_transaction(tree_root);
+ else
+ trans = btrfs_start_transaction(tree_root, 0);
if (IS_ERR(trans)) {
err = PTR_ERR(trans);
goto out_free;
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] btrfs: relocation: review the call sites which can be interruped by signal
2020-07-09 8:33 [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree Qu Wenruo
@ 2020-07-09 8:33 ` Qu Wenruo
2020-07-09 9:54 ` David Sterba
2020-07-09 11:05 ` [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree David Sterba
1 sibling, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2020-07-09 8:33 UTC (permalink / raw)
To: linux-btrfs
Since most metadata reservation calls can return -EINTR when get
interruped by fatal signal, we need to review the all the metadata
reservation call sites.
In relocation code, the metadata reservation happens in the following
sites:
- btrfs_block_rsv_refill() in merge_reloc_root()
merge_reloc_root() is a pretty critial section, we don't want get
interrupted by signal, so change the flush status to
BTRFS_RESERVE_FLUSH_LIMIT, so it won't get interrupted by signal.
Since such change can be ENPSPC-prone, also shrink the amount of
metadata to reserve a little to avoid deadly ENOSPC there.
- btrfs_block_rsv_refill() in reserve_metadata_space()
It calls with BTRFS_RESERVE_FLUSH_LIMIT, which won't get interrupred
by signal.
- btrfs_block_rsv_refill() in prepare_to_relocate()
- btrfs_block_rsv_add() in prepare_to_relocate()
- btrfs_block_rsv_refill() in relocate_block_group()
- btrfs_delalloc_reserve_metadata() in relocate_file_extent_cluster()
- btrfs_start_transaction() in relocate_block_group()
- btrfs_start_transaction() in create_reloc_inode()
Can be interruped by fatal signal and we can handle it easily.
For these call sites, just catch the -EINTR value in btrfs_balance()
and count them as canceled.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/relocation.c | 13 +++++++++++--
fs/btrfs/volumes.c | 2 +-
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 2b869fb2e62c..23914edd4710 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1686,12 +1686,21 @@ static noinline_for_stack int merge_reloc_root(struct reloc_control *rc,
btrfs_unlock_up_safe(path, 0);
}
- min_reserved = fs_info->nodesize * (BTRFS_MAX_LEVEL - 1) * 2;
+ /*
+ * In merge_reloc_root(), we modify the upper level pointer to swap
+ * the tree blocks between reloc tree and subvolume tree.
+ * Thus for tree block COW, we COW at most from level 1 to root level
+ * for each tree.
+ *
+ * Thus the needed metadata space is at most root_level * nodesize,
+ * and * 2 since we have two trees to COW.
+ */
+ min_reserved = fs_info->nodesize * btrfs_root_level(root_item) * 2;
memset(&next_key, 0, sizeof(next_key));
while (1) {
ret = btrfs_block_rsv_refill(root, rc->block_rsv, min_reserved,
- BTRFS_RESERVE_FLUSH_ALL);
+ BTRFS_RESERVE_FLUSH_LIMIT);
if (ret) {
err = ret;
goto out;
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index aabc6c922e04..d60df30bdc47 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4135,7 +4135,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
mutex_lock(&fs_info->balance_mutex);
if (ret == -ECANCELED && atomic_read(&fs_info->balance_pause_req))
btrfs_info(fs_info, "balance: paused");
- else if (ret == -ECANCELED && atomic_read(&fs_info->balance_cancel_req))
+ else if (ret == -ECANCELED || ret == -EINTR)
btrfs_info(fs_info, "balance: canceled");
else
btrfs_info(fs_info, "balance: ended with status: %d", ret);
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] btrfs: relocation: review the call sites which can be interruped by signal
2020-07-09 8:33 ` [PATCH v2 2/2] btrfs: relocation: review the call sites which can be interruped by signal Qu Wenruo
@ 2020-07-09 9:54 ` David Sterba
2020-07-09 10:15 ` Qu Wenruo
2020-07-09 10:46 ` Qu Wenruo
0 siblings, 2 replies; 8+ messages in thread
From: David Sterba @ 2020-07-09 9:54 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Thu, Jul 09, 2020 at 04:33:33PM +0800, Qu Wenruo wrote:
> Since most metadata reservation calls can return -EINTR when get
> interruped by fatal signal, we need to review the all the metadata
> reservation call sites.
>
> In relocation code, the metadata reservation happens in the following
> sites:
> - btrfs_block_rsv_refill() in merge_reloc_root()
> merge_reloc_root() is a pretty critial section, we don't want get
> interrupted by signal, so change the flush status to
> BTRFS_RESERVE_FLUSH_LIMIT, so it won't get interrupted by signal.
> Since such change can be ENPSPC-prone, also shrink the amount of
> metadata to reserve a little to avoid deadly ENOSPC there.
>
> - btrfs_block_rsv_refill() in reserve_metadata_space()
> It calls with BTRFS_RESERVE_FLUSH_LIMIT, which won't get interrupred
> by signal.
This semantics of BTRFS_RESERVE_FLUSH_LIMIT regarding signals should be
documented, right now there's a comment but says something about avoidig
deadlocks.
> - btrfs_block_rsv_refill() in prepare_to_relocate()
> - btrfs_block_rsv_add() in prepare_to_relocate()
> - btrfs_block_rsv_refill() in relocate_block_group()
> - btrfs_delalloc_reserve_metadata() in relocate_file_extent_cluster()
> - btrfs_start_transaction() in relocate_block_group()
> - btrfs_start_transaction() in create_reloc_inode()
> Can be interruped by fatal signal and we can handle it easily.
> For these call sites, just catch the -EINTR value in btrfs_balance()
> and count them as canceled.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/relocation.c | 13 +++++++++++--
> fs/btrfs/volumes.c | 2 +-
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 2b869fb2e62c..23914edd4710 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1686,12 +1686,21 @@ static noinline_for_stack int merge_reloc_root(struct reloc_control *rc,
> btrfs_unlock_up_safe(path, 0);
> }
>
> - min_reserved = fs_info->nodesize * (BTRFS_MAX_LEVEL - 1) * 2;
> + /*
> + * In merge_reloc_root(), we modify the upper level pointer to swap
> + * the tree blocks between reloc tree and subvolume tree.
> + * Thus for tree block COW, we COW at most from level 1 to root level
> + * for each tree.
> + *
> + * Thus the needed metadata space is at most root_level * nodesize,
> + * and * 2 since we have two trees to COW.
> + */
> + min_reserved = fs_info->nodesize * btrfs_root_level(root_item) * 2;
> memset(&next_key, 0, sizeof(next_key));
>
> while (1) {
> ret = btrfs_block_rsv_refill(root, rc->block_rsv, min_reserved,
> - BTRFS_RESERVE_FLUSH_ALL);
> + BTRFS_RESERVE_FLUSH_LIMIT);
> if (ret) {
> err = ret;
> goto out;
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index aabc6c922e04..d60df30bdc47 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4135,7 +4135,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
> mutex_lock(&fs_info->balance_mutex);
> if (ret == -ECANCELED && atomic_read(&fs_info->balance_pause_req))
> btrfs_info(fs_info, "balance: paused");
> - else if (ret == -ECANCELED && atomic_read(&fs_info->balance_cancel_req))
> + else if (ret == -ECANCELED || ret == -EINTR)
Why do you remove atomic_read(&fs_info->balance_cancel_req) ?
> btrfs_info(fs_info, "balance: canceled");
I'm not sure if it would be useful to print the reason, like
- 'canceled: user request'
- 'canceled: interrupted'
> else
> btrfs_info(fs_info, "balance: ended with status: %d", ret);
> --
> 2.27.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] btrfs: relocation: review the call sites which can be interruped by signal
2020-07-09 9:54 ` David Sterba
@ 2020-07-09 10:15 ` Qu Wenruo
2020-07-09 10:25 ` David Sterba
2020-07-09 10:46 ` Qu Wenruo
1 sibling, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2020-07-09 10:15 UTC (permalink / raw)
To: dsterba, linux-btrfs
On 2020/7/9 下午5:54, David Sterba wrote:
> On Thu, Jul 09, 2020 at 04:33:33PM +0800, Qu Wenruo wrote:
>> Since most metadata reservation calls can return -EINTR when get
>> interruped by fatal signal, we need to review the all the metadata
>> reservation call sites.
>>
>> In relocation code, the metadata reservation happens in the following
>> sites:
>> - btrfs_block_rsv_refill() in merge_reloc_root()
>> merge_reloc_root() is a pretty critial section, we don't want get
>> interrupted by signal, so change the flush status to
>> BTRFS_RESERVE_FLUSH_LIMIT, so it won't get interrupted by signal.
>> Since such change can be ENPSPC-prone, also shrink the amount of
>> metadata to reserve a little to avoid deadly ENOSPC there.
>>
>> - btrfs_block_rsv_refill() in reserve_metadata_space()
>> It calls with BTRFS_RESERVE_FLUSH_LIMIT, which won't get interrupred
>> by signal.
>
> This semantics of BTRFS_RESERVE_FLUSH_LIMIT regarding signals should be
> documented, right now there's a comment but says something about avoidig
> deadlocks.
>
>> - btrfs_block_rsv_refill() in prepare_to_relocate()
>> - btrfs_block_rsv_add() in prepare_to_relocate()
>> - btrfs_block_rsv_refill() in relocate_block_group()
>> - btrfs_delalloc_reserve_metadata() in relocate_file_extent_cluster()
>> - btrfs_start_transaction() in relocate_block_group()
>> - btrfs_start_transaction() in create_reloc_inode()
>> Can be interruped by fatal signal and we can handle it easily.
>> For these call sites, just catch the -EINTR value in btrfs_balance()
>> and count them as canceled.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/relocation.c | 13 +++++++++++--
>> fs/btrfs/volumes.c | 2 +-
>> 2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index 2b869fb2e62c..23914edd4710 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -1686,12 +1686,21 @@ static noinline_for_stack int merge_reloc_root(struct reloc_control *rc,
>> btrfs_unlock_up_safe(path, 0);
>> }
>>
>> - min_reserved = fs_info->nodesize * (BTRFS_MAX_LEVEL - 1) * 2;
>> + /*
>> + * In merge_reloc_root(), we modify the upper level pointer to swap
>> + * the tree blocks between reloc tree and subvolume tree.
>> + * Thus for tree block COW, we COW at most from level 1 to root level
>> + * for each tree.
>> + *
>> + * Thus the needed metadata space is at most root_level * nodesize,
>> + * and * 2 since we have two trees to COW.
>> + */
>> + min_reserved = fs_info->nodesize * btrfs_root_level(root_item) * 2;
>> memset(&next_key, 0, sizeof(next_key));
>>
>> while (1) {
>> ret = btrfs_block_rsv_refill(root, rc->block_rsv, min_reserved,
>> - BTRFS_RESERVE_FLUSH_ALL);
>> + BTRFS_RESERVE_FLUSH_LIMIT);
>> if (ret) {
>> err = ret;
>> goto out;
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index aabc6c922e04..d60df30bdc47 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -4135,7 +4135,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>> mutex_lock(&fs_info->balance_mutex);
>> if (ret == -ECANCELED && atomic_read(&fs_info->balance_pause_req))
>> btrfs_info(fs_info, "balance: paused");
>> - else if (ret == -ECANCELED && atomic_read(&fs_info->balance_cancel_req))
>> + else if (ret == -ECANCELED || ret == -EINTR)
>
> Why do you remove atomic_read(&fs_info->balance_cancel_req) ?
Because now btrfs_should_cancel_balance() can return ECANCELED without
balance_cancel_req increased due to pending fatal signal.
>
>> btrfs_info(fs_info, "balance: canceled");
>
> I'm not sure if it would be useful to print the reason, like
>
> - 'canceled: user request'
> - 'canceled: interrupted'
To me, if user interrupt the balance progress, it's obvious they want to
cancel it.
Thus no need to distinguish btrfs balance cancel and signal cancel.
Thanks,
Qu
>
>> else
>> btrfs_info(fs_info, "balance: ended with status: %d", ret);
>> --
>> 2.27.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] btrfs: relocation: review the call sites which can be interruped by signal
2020-07-09 10:15 ` Qu Wenruo
@ 2020-07-09 10:25 ` David Sterba
0 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2020-07-09 10:25 UTC (permalink / raw)
To: Qu Wenruo; +Cc: dsterba, linux-btrfs
On Thu, Jul 09, 2020 at 06:15:09PM +0800, Qu Wenruo wrote:
> On 2020/7/9 下午5:54, David Sterba wrote:
> > On Thu, Jul 09, 2020 at 04:33:33PM +0800, Qu Wenruo wrote:
> >> @@ -4135,7 +4135,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
> >> mutex_lock(&fs_info->balance_mutex);
> >> if (ret == -ECANCELED && atomic_read(&fs_info->balance_pause_req))
> >> btrfs_info(fs_info, "balance: paused");
> >> - else if (ret == -ECANCELED && atomic_read(&fs_info->balance_cancel_req))
> >> + else if (ret == -ECANCELED || ret == -EINTR)
> >
> > Why do you remove atomic_read(&fs_info->balance_cancel_req) ?
>
> Because now btrfs_should_cancel_balance() can return ECANCELED without
> balance_cancel_req increased due to pending fatal signal.
Ah right, I misread it as || which would remove one reason for
cancellation.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] btrfs: relocation: review the call sites which can be interruped by signal
2020-07-09 9:54 ` David Sterba
2020-07-09 10:15 ` Qu Wenruo
@ 2020-07-09 10:46 ` Qu Wenruo
1 sibling, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2020-07-09 10:46 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 4125 bytes --]
On 2020/7/9 下午5:54, David Sterba wrote:
> On Thu, Jul 09, 2020 at 04:33:33PM +0800, Qu Wenruo wrote:
>> Since most metadata reservation calls can return -EINTR when get
>> interruped by fatal signal, we need to review the all the metadata
>> reservation call sites.
>>
>> In relocation code, the metadata reservation happens in the following
>> sites:
>> - btrfs_block_rsv_refill() in merge_reloc_root()
>> merge_reloc_root() is a pretty critial section, we don't want get
>> interrupted by signal, so change the flush status to
>> BTRFS_RESERVE_FLUSH_LIMIT, so it won't get interrupted by signal.
>> Since such change can be ENPSPC-prone, also shrink the amount of
>> metadata to reserve a little to avoid deadly ENOSPC there.
>>
>> - btrfs_block_rsv_refill() in reserve_metadata_space()
>> It calls with BTRFS_RESERVE_FLUSH_LIMIT, which won't get interrupred
>> by signal.
>
> This semantics of BTRFS_RESERVE_FLUSH_LIMIT regarding signals should be
> documented, right now there's a comment but says something about avoidig
> deadlocks.
For this, I tend to add one or more patches to add some comment for all
FLUSH enums, and all their callers.
I hate when some infrastructure hit me by surprise, and since ticketing
system is hidden from a lot of functions, we also need to mention that
-EINTR case.
Thanks,
Qu
>
>> - btrfs_block_rsv_refill() in prepare_to_relocate()
>> - btrfs_block_rsv_add() in prepare_to_relocate()
>> - btrfs_block_rsv_refill() in relocate_block_group()
>> - btrfs_delalloc_reserve_metadata() in relocate_file_extent_cluster()
>> - btrfs_start_transaction() in relocate_block_group()
>> - btrfs_start_transaction() in create_reloc_inode()
>> Can be interruped by fatal signal and we can handle it easily.
>> For these call sites, just catch the -EINTR value in btrfs_balance()
>> and count them as canceled.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/relocation.c | 13 +++++++++++--
>> fs/btrfs/volumes.c | 2 +-
>> 2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index 2b869fb2e62c..23914edd4710 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -1686,12 +1686,21 @@ static noinline_for_stack int merge_reloc_root(struct reloc_control *rc,
>> btrfs_unlock_up_safe(path, 0);
>> }
>>
>> - min_reserved = fs_info->nodesize * (BTRFS_MAX_LEVEL - 1) * 2;
>> + /*
>> + * In merge_reloc_root(), we modify the upper level pointer to swap
>> + * the tree blocks between reloc tree and subvolume tree.
>> + * Thus for tree block COW, we COW at most from level 1 to root level
>> + * for each tree.
>> + *
>> + * Thus the needed metadata space is at most root_level * nodesize,
>> + * and * 2 since we have two trees to COW.
>> + */
>> + min_reserved = fs_info->nodesize * btrfs_root_level(root_item) * 2;
>> memset(&next_key, 0, sizeof(next_key));
>>
>> while (1) {
>> ret = btrfs_block_rsv_refill(root, rc->block_rsv, min_reserved,
>> - BTRFS_RESERVE_FLUSH_ALL);
>> + BTRFS_RESERVE_FLUSH_LIMIT);
>> if (ret) {
>> err = ret;
>> goto out;
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index aabc6c922e04..d60df30bdc47 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -4135,7 +4135,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>> mutex_lock(&fs_info->balance_mutex);
>> if (ret == -ECANCELED && atomic_read(&fs_info->balance_pause_req))
>> btrfs_info(fs_info, "balance: paused");
>> - else if (ret == -ECANCELED && atomic_read(&fs_info->balance_cancel_req))
>> + else if (ret == -ECANCELED || ret == -EINTR)
>
> Why do you remove atomic_read(&fs_info->balance_cancel_req) ?
>
>> btrfs_info(fs_info, "balance: canceled");
>
> I'm not sure if it would be useful to print the reason, like
>
> - 'canceled: user request'
> - 'canceled: interrupted'
>
>> else
>> btrfs_info(fs_info, "balance: ended with status: %d", ret);
>> --
>> 2.27.0
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree
2020-07-09 8:33 [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree Qu Wenruo
2020-07-09 8:33 ` [PATCH v2 2/2] btrfs: relocation: review the call sites which can be interruped by signal Qu Wenruo
@ 2020-07-09 11:05 ` David Sterba
2020-07-09 12:11 ` Qu Wenruo
1 sibling, 1 reply; 8+ messages in thread
From: David Sterba @ 2020-07-09 11:05 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Josef Bacik
On Thu, Jul 09, 2020 at 04:33:32PM +0800, Qu Wenruo wrote:
> From the mentioned forced read-only, to later balance error due to half
> dropped reloc trees.
>
> [FIX]
> Fix this problem by using btrfs_join_transaction() if
> btrfs_drop_snapshot() is called from relocation context.
>
> As btrfs_join_transaction() won't wait full tickets, it won't get
> interrupted from signal.
Could you please rephrase the text above?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree
2020-07-09 11:05 ` [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree David Sterba
@ 2020-07-09 12:11 ` Qu Wenruo
0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2020-07-09 12:11 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs, Josef Bacik
[-- Attachment #1.1: Type: text/plain, Size: 648 bytes --]
On 2020/7/9 下午7:05, David Sterba wrote:
> On Thu, Jul 09, 2020 at 04:33:32PM +0800, Qu Wenruo wrote:
>
>> From the mentioned forced read-only, to later balance error due to half
>> dropped reloc trees.
>>
>> [FIX]
>> Fix this problem by using btrfs_join_transaction() if
>> btrfs_drop_snapshot() is called from relocation context.
>>
>> As btrfs_join_transaction() won't wait full tickets, it won't get
>> interrupted from signal.
>
> Could you please rephrase the text above?
>
How about this:
As btrfs_join_transaction() won't reserve new metadata space, it won't
get interrupted by signal at all.
Thanks,
Qu
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-09 12:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 8:33 [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree Qu Wenruo
2020-07-09 8:33 ` [PATCH v2 2/2] btrfs: relocation: review the call sites which can be interruped by signal Qu Wenruo
2020-07-09 9:54 ` David Sterba
2020-07-09 10:15 ` Qu Wenruo
2020-07-09 10:25 ` David Sterba
2020-07-09 10:46 ` Qu Wenruo
2020-07-09 11:05 ` [PATCH v2 1/2] btrfs: avoid possible signal interruption for btrfs_drop_snapshot() on relocation tree David Sterba
2020-07-09 12:11 ` Qu Wenruo
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.