On Fri, Feb 5, 2021 at 9:26 AM Naohiro Aota <naohiro.aota@xxxxxxx> 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. We cannot wait for the range
> to be written, because it will cause a deadlock. So, let's bail out to a
> full commit in this case.
>
> Cc: Filipe Manana <fdmanana@xxxxxxxxx>
> Signed-off-by: Naohiro Aota <naohiro.aota@xxxxxxx>
> ---
> fs/btrfs/tree-log.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> This patch solves a regression introduced by fixing patch 40. I'm
> sorry for the confusing patch numbering.
Hum, how does patch 40 can cause this?
And is it before the fixup or after?
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 4e72794342c0..629e605cd62d 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -3120,6 +3120,14 @@ 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);
> + /*
> + * There is a hole writing out the extents and cannot proceed it on
> + * zoned filesystem, which require sequential writing. We can
require -> requires
> + * ignore the error for now, since we don't wait for completion for
> + * now.
So why can we ignore the error for now?
Why not just bail out here and mark the log for full commit? (without
a transaction abort)
> + */
> + if (ret == -EAGAIN)
> + ret = 0;
> if (ret) {
> blk_finish_plug(&plug);
> btrfs_abort_transaction(trans, ret);
> @@ -3229,7 +3237,16 @@ 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) {
> + /*
> + * There is a hole in the extents, and failed to sequential write
> + * on zoned filesystem. We cannot wait for this write outs, sinc it
this -> these
> + * cause a deadlock. Bail out to the full commit, instead.
> + */
> + if (ret == -EAGAIN) {
> + btrfs_wait_tree_log_extents(log, mark);
> + mutex_unlock(&log_root_tree->log_mutex);
> + goto out_wake_log_root;
Must also call btrfs_set_log_full_commit(trans);
Thanks.
> + } 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.”
