diff mbox series

btrfs: zoned: wait for data BG to be finished on direct IO allocation

Message ID 36d4b7502a8654882718421a18472d41dbc1c83c.1697529529.git.naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series btrfs: zoned: wait for data BG to be finished on direct IO allocation | expand

Commit Message

Naohiro Aota Oct. 17, 2023, 8 a.m. UTC
Running the fio command below on a ZNS device results in "Resource
temporarily unavailable" error.

fio: io_u error on file /mnt/w.2.0: Resource temporarily unavailable: write offset=117440512, buflen=16777216
fio: io_u error on file /mnt/w.2.0: Resource temporarily unavailable: write offset=134217728, buflen=16777216
...

This happens because -EAGAIN error returned from btrfs_reserve_extent()
called from btrfs_new_extent_direct() is splling over to the userland.

btrfs_reserve_extent() returns -EAGAIN when there is no active zone
available. Then, the caller should wait for some other on-going IO to
finish a zone and retry the allocation.

This logic is already implemented for buffered write in cow_file_range(),
but it is missing for the direct IO counterpart. Implement the same logic
for it.

Fixes: 2ce543f47843 ("btrfs: zoned: wait until zone is finished when allocation didn't progress")
CC: stable@vger.kernel.org # 6.1+
Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/inode.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Johannes Thumshirn Oct. 17, 2023, 8:45 a.m. UTC | #1
On 17.10.23 10:01, Naohiro Aota wrote:
> This happens because -EAGAIN error returned from btrfs_reserve_extent()
> called from btrfs_new_extent_direct() is splling over to the userland.
                                   spilling ~^

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Shinichiro Kawasaki Oct. 17, 2023, 8:57 a.m. UTC | #2
On Oct 17, 2023 / 17:00, Naohiro Aota wrote:
> Running the fio command below on a ZNS device results in "Resource
> temporarily unavailable" error.

Thanks for the fix. The fio command is missing here. It should be like,

$ sudo fio --name=w --directory=/mnt --filesize=1GB --bs=16MB --numjobs=16 \
        --rw=write --ioengine=libaio --iodepth=128 --direct=1

> 
> fio: io_u error on file /mnt/w.2.0: Resource temporarily unavailable: write offset=117440512, buflen=16777216
> fio: io_u error on file /mnt/w.2.0: Resource temporarily unavailable: write offset=134217728, buflen=16777216
> ...
> 
> This happens because -EAGAIN error returned from btrfs_reserve_extent()
> called from btrfs_new_extent_direct() is splling over to the userland.
> 
> btrfs_reserve_extent() returns -EAGAIN when there is no active zone
> available. Then, the caller should wait for some other on-going IO to
> finish a zone and retry the allocation.
> 
> This logic is already implemented for buffered write in cow_file_range(),
> but it is missing for the direct IO counterpart. Implement the same logic
> for it.
> 
> Fixes: 2ce543f47843 ("btrfs: zoned: wait until zone is finished when allocation didn't progress")
> CC: stable@vger.kernel.org # 6.1+
> Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  fs/btrfs/inode.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index b388505c91cc..a979e964375d 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6979,8 +6979,16 @@ static struct extent_map *btrfs_new_extent_direct(struct btrfs_inode *inode,
>  	int ret;
>  
>  	alloc_hint = get_extent_allocation_hint(inode, start, len);
> +again:
>  	ret = btrfs_reserve_extent(root, len, len, fs_info->sectorsize,
>  				   0, alloc_hint, &ins, 1, 1);
> +	if (ret == -EAGAIN) {
> +		ASSERT(btrfs_is_zoned(fs_info));
> +		wait_on_bit_io(&inode->root->fs_info->flags,
> +			       BTRFS_FS_NEED_ZONE_FINISH,
> +			       TASK_UNINTERRUPTIBLE);
> +		goto again;
> +	}
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> -- 
> 2.42.0
>
David Sterba Oct. 17, 2023, 7:39 p.m. UTC | #3
On Tue, Oct 17, 2023 at 05:00:31PM +0900, Naohiro Aota wrote:
> Running the fio command below on a ZNS device results in "Resource
> temporarily unavailable" error.
> 
> fio: io_u error on file /mnt/w.2.0: Resource temporarily unavailable: write offset=117440512, buflen=16777216
> fio: io_u error on file /mnt/w.2.0: Resource temporarily unavailable: write offset=134217728, buflen=16777216
> ...
> 
> This happens because -EAGAIN error returned from btrfs_reserve_extent()
> called from btrfs_new_extent_direct() is splling over to the userland.
> 
> btrfs_reserve_extent() returns -EAGAIN when there is no active zone
> available. Then, the caller should wait for some other on-going IO to
> finish a zone and retry the allocation.
> 
> This logic is already implemented for buffered write in cow_file_range(),
> but it is missing for the direct IO counterpart. Implement the same logic
> for it.
> 
> Fixes: 2ce543f47843 ("btrfs: zoned: wait until zone is finished when allocation didn't progress")
> CC: stable@vger.kernel.org # 6.1+
> Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Tested-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>

With fixed spelling and added fio command, patch added to misc-next,
thanks.
diff mbox series

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b388505c91cc..a979e964375d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6979,8 +6979,16 @@  static struct extent_map *btrfs_new_extent_direct(struct btrfs_inode *inode,
 	int ret;
 
 	alloc_hint = get_extent_allocation_hint(inode, start, len);
+again:
 	ret = btrfs_reserve_extent(root, len, len, fs_info->sectorsize,
 				   0, alloc_hint, &ins, 1, 1);
+	if (ret == -EAGAIN) {
+		ASSERT(btrfs_is_zoned(fs_info));
+		wait_on_bit_io(&inode->root->fs_info->flags,
+			       BTRFS_FS_NEED_ZONE_FINISH,
+			       TASK_UNINTERRUPTIBLE);
+		goto again;
+	}
 	if (ret)
 		return ERR_PTR(ret);