Re: [PATCH v6 24/28] btrfs: enable relocation in HMZONED mode

Naohiro Aota
2019-12-18T10:49:25Z

On Tue, Dec 17, 2019 at 04:32:04PM -0500, Josef Bacik wrote:
>On 12/12/19 11:09 PM, Naohiro Aota wrote:
>>To serialize allocation and submit_bio, we introduced mutex around them. As
>>a result, preallocation must be completely disabled to avoid a deadlock.
>>
>>Since current relocation process relies on preallocation to move file data
>>extents, it must be handled in another way. In HMZONED mode, we just
>>truncate the inode to the size that we wanted to pre-allocate. Then, we
>>flush dirty pages on the file before finishing relocation process.
>>run_delalloc_hmzoned() will handle all the allocation and submit IOs to
>>the underlying layers.
>>
>>Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
>>---
>> fs/btrfs/relocation.c | 39 +++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 37 insertions(+), 2 deletions(-)
>>
>>diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>>index d897a8e5e430..2d17b7566df4 100644
>>--- a/fs/btrfs/relocation.c
>>+++ b/fs/btrfs/relocation.c
>>@@ -3159,6 +3159,34 @@ int prealloc_file_extent_cluster(struct inode *inode,
>> if (ret)
>> goto out;
>>+ /*
>>+ * In HMZONED, we cannot preallocate the file region. Instead,
>>+ * we dirty and fiemap_write the region.
>>+ */
>>+
>>+ if (btrfs_fs_incompat(btrfs_sb(inode->i_sb), HMZONED)) {
>>+ struct btrfs_root *root = BTRFS_I(inode)->root;
>>+ struct btrfs_trans_handle *trans;
>>+
>>+ end = cluster->end - offset + 1;
>>+ trans = btrfs_start_transaction(root, 1);
>>+ if (IS_ERR(trans))
>>+ return PTR_ERR(trans);
>>+
>>+ inode->i_ctime = current_time(inode);
>>+ i_size_write(inode, end);
>>+ btrfs_ordered_update_i_size(inode, end, NULL);
>>+ ret = btrfs_update_inode(trans, root, inode);
>>+ if (ret) {
>>+ btrfs_abort_transaction(trans, ret);
>>+ btrfs_end_transaction(trans);
>>+ return ret;
>>+ }
>>+ ret = btrfs_end_transaction(trans);
>>+
>>+ goto out;
>>+ }
>>+
>
>Why are we arbitrarily extending the i_size here? If we don't need
>prealloc we don't need to jack up the i_size either.

We need to extend i_size to read data from the relocating block
group. If not, btrfs_readpage() in relocate_file_extent_cluster()
always reads zero filled page because the read position is beyond the
file size.

>> cur_offset = prealloc_start;
>> while (nr < cluster->nr) {
>> start = cluster->boundary[nr] - offset;
>>@@ -3346,6 +3374,10 @@ static int relocate_file_extent_cluster(struct inode *inode,
>> btrfs_throttle(fs_info);
>> }
>> WARN_ON(nr != cluster->nr);
>>+ if (btrfs_fs_incompat(fs_info, HMZONED) && !ret) {
>>+ ret = btrfs_wait_ordered_range(inode, 0, (u64)-1);
>>+ WARN_ON(ret);
>
>Do not WAR_ON() when this could happen due to IO errors. Thanks,
>
>Josef

Sure. We can just drop it.