[PATCH RFC] btrfs: no need to transition to rdonly for ENOSPC error

[PATCH RFC] btrfs: no need to transition to rdonly for ENOSPC error

[Anand Jain]

In the current kernel both scrub and balance fails due to ENOSPC,
however there is no reason that it should be transitioned to the
RDONLY and making free spaces difficult.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 12d7d3be7cd4..8c1b858f55c4 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -172,6 +172,9 @@ void __btrfs_handle_fs_error(struct btrfs_fs_info *fs_info, const char *function
 	if (sb_rdonly(sb))
 		return;
 
+	if (errno == -ENOSPC)
+		return;
+
 	btrfs_discard_stop(fs_info);
 
 	/* btrfs handle error by forcing the filesystem readonly */
-- 
2.18.4

Re: [PATCH RFC] btrfs: no need to transition to rdonly for ENOSPC error

[Filipe Manana]

On Tue, Jan 12, 2021 at 1:01 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> In the current kernel both scrub and balance fails due to ENOSPC,
> however there is no reason that it should be transitioned to the
> RDONLY and making free spaces difficult.

No way.

There are plenty of places where ignoring an ENOSPC would leave the
filesystem in an inconsistent state.
Like anything that needs to do a series of btree modifications, we
want to make sure no one is able to commit the transaction if we do
the first update but fail the second for example.
For example, btrfs_finish_ordered_io() (delete and insert new extent
in subvolume tree, add checksums, etc).

Thanks.

>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/super.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 12d7d3be7cd4..8c1b858f55c4 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -172,6 +172,9 @@ void __btrfs_handle_fs_error(struct btrfs_fs_info *fs_info, const char *function
>         if (sb_rdonly(sb))
>                 return;
>
> +       if (errno == -ENOSPC)
> +               return;
> +
>         btrfs_discard_stop(fs_info);
>
>         /* btrfs handle error by forcing the filesystem readonly */
> --
> 2.18.4
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH RFC] btrfs: no need to transition to rdonly for ENOSPC error

[Qu Wenruo]

On 2021/1/12 下午6:34, Anand Jain wrote:
> In the current kernel both scrub and balance fails due to ENOSPC,
> however there is no reason that it should be transitioned to the
> RDONLY and making free spaces difficult.

This just masks the real problem.

 From all your reports, the problem is the same, we exhausted the
metadata space, but the GlobalRSV is only 100+MiB.

It looks like the problem Josef is working on to make GlobalRSV work as
usual.

Definitely not the proper way to fix it.

Thanks,
Qu
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>   fs/btrfs/super.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 12d7d3be7cd4..8c1b858f55c4 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -172,6 +172,9 @@ void __btrfs_handle_fs_error(struct btrfs_fs_info *fs_info, const char *function
>   	if (sb_rdonly(sb))
>   		return;
>
> +	if (errno == -ENOSPC)
> +		return;
> +
>   	btrfs_discard_stop(fs_info);
>
>   	/* btrfs handle error by forcing the filesystem readonly */
>

Re: [PATCH RFC] btrfs: no need to transition to rdonly for ENOSPC error

[Josef Bacik]

On 1/12/21 5:34 AM, Anand Jain wrote:
> In the current kernel both scrub and balance fails due to ENOSPC,
> however there is no reason that it should be transitioned to the
> RDONLY and making free spaces difficult.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>   fs/btrfs/super.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 12d7d3be7cd4..8c1b858f55c4 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -172,6 +172,9 @@ void __btrfs_handle_fs_error(struct btrfs_fs_info *fs_info, const char *function
>   	if (sb_rdonly(sb))
>   		return;
>   
> +	if (errno == -ENOSPC)
> +		return;
> +
>   	btrfs_discard_stop(fs_info);
>   
>   	/* btrfs handle error by forcing the filesystem readonly */
> 

Filipe and Qu are right, but there are cases where we probably shouldn't flip 
read only on ENOSPC, and we should address those at the specific spots where 
this happens.  Do you have an example?  If it's happening at btrfs_search_slot() 
time then we're not reserving enough space in whatever reserve we use, but if 
it's happening somewhere else, for example trying to mark a block group as read 
only and getting an ENOSPC there, we could probably skip flipping read only for 
that sort of case.  We need to address the specific scenarios you're hitting 
rather than blanket skipping the abort for ENOSPC, otherwise we'll get corrupt 
file systems.  Thanks,

Josef

Re: [PATCH RFC] btrfs: no need to transition to rdonly for ENOSPC error

[Anand Jain]

On 12/1/21 10:41 pm, Josef Bacik wrote:
> On 1/12/21 5:34 AM, Anand Jain wrote:
>> In the current kernel both scrub and balance fails due to ENOSPC,
>> however there is no reason that it should be transitioned to the
>> RDONLY and making free spaces difficult.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/super.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 12d7d3be7cd4..8c1b858f55c4 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -172,6 +172,9 @@ void __btrfs_handle_fs_error(struct btrfs_fs_info 
>> *fs_info, const char *function
>>       if (sb_rdonly(sb))
>>           return;
>> +    if (errno == -ENOSPC)
>> +        return;
>> +
>>       btrfs_discard_stop(fs_info);
>>       /* btrfs handle error by forcing the filesystem readonly */
>>
> 
> Filipe and Qu are right, but there are cases where we probably shouldn't 
> flip read only on ENOSPC, and we should address those at the specific 
> spots where this happens.  Do you have an example?  If it's happening at 
> btrfs_search_slot() time then we're not reserving enough space in 
> whatever reserve we use, but if it's happening somewhere else, for 
> example trying to mark a block group as read only and getting an ENOSPC 
> there, we could probably skip flipping read only for that sort of case.  
> We need to address the specific scenarios you're hitting rather than 
> blanket skipping the abort for ENOSPC, otherwise we'll get corrupt file 
> systems.  Thanks,
> 

Ok. It makes sense to me.

Unfortunately, I had to restore my laptop, my only source of production.

Per the stack trace below, ENOSPC is from the search.

[ 2460.219094] BTRFS info (device sdb6): balance: start -f 
-dconvert=single -mconvert=single -sconvert=single
[ 2460.219224] BTRFS info (device sdb6): relocating block group 
194572713984 flags data
[ 2460.241299] ------------[ cut here ]------------
[ 2460.241302] BTRFS: Transaction aborted (error -28)
[ 2460.241328] WARNING: CPU: 3 PID: 4135 at fs/btrfs/volumes.c:3003 
btrfs_remove_chunk+0x60e/0x6c0 [btrfs]
::
[ 2460.241447] RIP: 0010:btrfs_remove_chunk+0x60e/0x6c0 [btrfs]
::
[ 2460.241497] Call Trace:
[ 2460.241500]  btrfs_relocate_chunk+0x9d/0xd0 [btrfs]
[ 2460.241533]  __btrfs_balance+0x413/0xa30 [btrfs]
[ 2460.241566]  btrfs_balance+0x436/0x4d0 [btrfs]
[ 2460.241597]  btrfs_ioctl_balance+0x2d5/0x380 [btrfs]
[ 2460.241647]  btrfs_ioctl+0x565/0x22b0 [btrfs]
[ 2460.241693]  ? selinux_file_ioctl+0x174/0x220
[ 2460.241698]  ? handle_mm_fault+0xd7/0x2b0
[ 2460.241702]  __x64_sys_ioctl+0x91/0xc0
[ 2460.241705]  ? __x64_sys_ioctl+0x91/0xc0
[ 2460.241707]  do_syscall_64+0x38/0x50
[ 2460.241711]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
::
[ 2460.241735] ---[ end trace 95b8b7b84503a477 ]---
[ 2460.241738] BTRFS: error (device sdb6) in btrfs_remove_chunk:3003: 
errno=-28 No space left
[ 2460.241743] BTRFS info (device sdb6): forced readonly
[ 2460.242646] BTRFS info (device sdb6): 1 enospc errors during balance
[ 2460.242649] BTRFS info (device sdb6): balance: ended with status: -30


btrfs fi us /Volumes/
Overall:
     Device size:         154.97GiB
     Device allocated:         154.97GiB
     Device unallocated:           2.00MiB
     Device missing:             0.00B
     Used:             147.91GiB
     Free (estimated):           2.75GiB    (min: 2.75GiB)
     Data ratio:                  1.99
     Metadata ratio:              1.52
     Global reserve:         183.72MiB    (used: 0.00B)

Data,single: Size:894.00MiB, Used:0.00B (0.00%)
    /dev/sdb6     894.00MiB

Data,RAID1: Size:75.53GiB, Used:73.23GiB (96.96%)
    /dev/sdb6      75.53GiB
    /dev/sdc2      75.53GiB

Metadata,single: Size:964.00MiB, Used:0.00B (0.00%)
    /dev/sdb6     964.00MiB

Metadata,RAID1: Size:1.00GiB, Used:737.25MiB (72.00%)
    /dev/sdb6       1.00GiB
    /dev/sdc2       1.00GiB

System,single: Size:32.00MiB, Used:0.00B (0.00%)
    /dev/sdb6      32.00MiB

System,RAID1: Size:32.00MiB, Used:16.00KiB (0.05%)
    /dev/sdb6      32.00MiB
    /dev/sdc2      32.00MiB

Unallocated:
    /dev/sdb6       1.00MiB
    /dev/sdc2       1.00MiB
--------


But below is another ENOSPC leading rdonlyfs. Here it was during the 
scrub. And while trying to finish the ordered extents.


[ 2365.314375] ------------[ cut here ]------------
[ 2365.314385] BTRFS: Transaction aborted (error -28)
[ 2365.314470] WARNING: CPU: 2 PID: 4672 at fs/btrfs/inode.c:2823 
btrfs_finish_ordered_io+0x8e6/0x950 [btrfs]
<snap>
[ 2365.314921] CPU: 2 PID: 4672 Comm: kworker/u8:11 Not tainted 
5.11.0-rc2+ #3
[ 2365.314933] Hardware name: LENOVO 20FAS6PN00/20FAS6PN00, BIOS 
N1CET47W (1.15 ) 08/08/2016
[ 2365.314939] Workqueue: btrfs-endio-write btrfs_work_helper [btrfs]
[ 2365.315093] RIP: 0010:btrfs_finish_ordered_io+0x8e6/0x950 [btrfs]
<snap>
[ 2365.315290] Call Trace:
[ 2365.315309] BTRFS warning (device sda6): 
btrfs_finish_ordered_io:2823: Aborting unused transaction(No space left).
[ 2365.315305]  finish_ordered_fn+0x15/0x20 [btrfs]
[ 2365.315532]  btrfs_work_helper+0xcc/0x2f0 [btrfs]
[ 2365.315763]  process_one_work+0x1ee/0x390
[ 2365.315781]  worker_thread+0x50/0x3d0
[ 2365.315797]  kthread+0x114/0x150
[ 2365.315811]  ? process_one_work+0x390/0x390
[ 2365.315824]  ? kthread_park+0x90/0x90
[ 2365.315836]  ret_from_fork+0x22/0x30
[ 2365.315861] ---[ end trace b0e289a2c3d424e8 ]---
[ 2365.315871] BTRFS warning (device sda6): 
btrfs_finish_ordered_io:2823: Aborting unused transaction(No space left).
[ 2365.316713] BTRFS warning (device sda6): 
btrfs_finish_ordered_io:2823: Aborting unused transaction(No space left).
[ 2365.316796] BTRFS warning (device sda6): Skipping commit of aborted 
transaction.
[ 2365.316809] BTRFS: error (device sda6) in cleanup_transaction:1938: 
errno=-30 Readonly filesystem
[ 2365.316824] BTRFS info (device sda6): forced readonly
[ 2365.359195] BTRFS info (device sda6): scrub: not finished on devid 2 
with status: -125

-Anand

> Josef