Re: [PATCH v15.1 43/43] btrfs: zoned: deal with holes writing out tree-log pages

2021-02-08 Thread David Sterba
On Fri, Feb 05, 2021 at 11:58:36PM +0900, Naohiro Aota wrote:
> Since the zoned filesystem requires sequential write out of metadata, we
> cannot proceed with a hole in tree-log pages. When such a hole exists,
> btree_write_cache_pages() will return -EAGAIN. This happens when someone,
> e.g., a concurrent transaction commit, writes a dirty extent in this
> tree-log commit.
> 
> If we are not going to wait for the extents, we can hope the concurrent
> writing fills the hole for us. So, we can ignore the error in this case and
> hope the next write will succeed.
> 
> If we want to wait for them and got the error, we cannot wait for them
> because it will cause a deadlock. So, let's bail out to a full commit in
> this case.
> 
> Signed-off-by: Naohiro Aota 

Inserted before the last patch, thanks.


Re: [PATCH v15.1 43/43] btrfs: zoned: deal with holes writing out tree-log pages

2021-02-05 Thread Filipe Manana
On Fri, Feb 5, 2021 at 2:58 PM Naohiro Aota  wrote:
>
> Since the zoned filesystem requires sequential write out of metadata, we
> cannot proceed with a hole in tree-log pages. When such a hole exists,
> btree_write_cache_pages() will return -EAGAIN. This happens when someone,
> e.g., a concurrent transaction commit, writes a dirty extent in this
> tree-log commit.
>
> If we are not going to wait for the extents, we can hope the concurrent
> writing fills the hole for us. So, we can ignore the error in this case and
> hope the next write will succeed.
>
> If we want to wait for them and got the error, we cannot wait for them
> because it will cause a deadlock. So, let's bail out to a full commit in
> this case.
>
> Signed-off-by: Naohiro Aota 

Reviewed-by: Filipe Manana 

It looks good now, thanks!

> ---
>  fs/btrfs/tree-log.c | 23 ++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index fc04625cbbd1..d90695c1ab6c 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -3120,6 +3120,17 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
>  */
> blk_start_plug(&plug);
> ret = btrfs_write_marked_extents(fs_info, &log->dirty_log_pages, 
> mark);
> +   /*
> +* -EAGAIN happens when someone, e.g., a concurrent transaction
> +*  commit, writes a dirty extent in this tree-log commit. This
> +*  concurrent write will create a hole writing out the extents,
> +*  and we cannot proceed on a zoned filesystem, requiring
> +*  sequential writing. While we can bail out to a full commit
> +*  here, but we can continue hoping the concurrent writing fills
> +*  the hole.
> +*/
> +   if (ret == -EAGAIN && btrfs_is_zoned(fs_info))
> +   ret = 0;
> if (ret) {
> blk_finish_plug(&plug);
> btrfs_abort_transaction(trans, ret);
> @@ -3242,7 +3253,17 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
>  &log_root_tree->dirty_log_pages,
>  EXTENT_DIRTY | EXTENT_NEW);
> blk_finish_plug(&plug);
> -   if (ret) {
> +   /*
> +* As described above, -EAGAIN indicates a hole in the extents. We
> +* cannot wait for these write outs since the waiting cause a
> +* deadlock. Bail out to the full commit instead.
> +*/
> +   if (ret == -EAGAIN && btrfs_is_zoned(fs_info)) {
> +   btrfs_set_log_full_commit(trans);
> +   btrfs_wait_tree_log_extents(log, mark);
> +   mutex_unlock(&log_root_tree->log_mutex);
> +   goto out_wake_log_root;
> +   } else if (ret) {
> btrfs_set_log_full_commit(trans);
> btrfs_abort_transaction(trans, ret);
> mutex_unlock(&log_root_tree->log_mutex);
> --
> 2.30.0
>


-- 
Filipe David Manana,

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