| Message ID | 20210205145836.gtty4r6a4ftolehj@naota-xeon (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Fri, Feb 5, 2021 at 2:58 PM Naohiro Aota <naohiro.aota@wdc.com> 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 <naohiro.aota@wdc.com> Reviewed-by: Filipe Manana <fdmanana@suse.com> 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 >
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 <naohiro.aota@wdc.com> Inserted before the last patch, thanks.
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);
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 <naohiro.aota@wdc.com> --- fs/btrfs/tree-log.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)