On Fri 19-02-21 12:25:19, Eric Whitney wrote:
> Change the retry policy in ext4_alloc_file_blocks() to allow for a full
> retry cycle whenever a portion of an allocation request has been
> fulfilled. A large allocation request often results in multiple calls
> to ext4_map_blocks(), each of which is potentially subject to a
> temporary ENOSPC condition and retry cycle. The current code only
> allows for a single retry cycle.
>
> This patch does not address a known bug or reported complaint.
> However, it should make block allocation for fallocate and zero range
> more robust.
>
> In addition, simplify the conditional controlling the allocation while
> loop, where testing len alone is sufficient. Remove the assignment to
> ret2 in the error path after the call to ext4_map_blocks() since its
> value isn't subsequently used.
>
> v2: Silence smatch warning by initializing ret.
>
> For smatch warning:
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
>
> Signed-off-by: Eric Whitney <enwlinux@xxxxxxxxx>
> ---
> fs/ext4/extents.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 3960b7ec3ab7..77c84d6f1af6 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4382,8 +4382,7 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> {
> struct inode *inode = file_inode(file);
> handle_t *handle;
> - int ret = 0;
> - int ret2 = 0, ret3 = 0;
> + int ret = 0, ret2 = 0, ret3 = 0;
> int retries = 0;
> int depth = 0;
> struct ext4_map_blocks map;
> @@ -4408,7 +4407,7 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> depth = ext_depth(inode);
>
> retry:
> - while (ret >= 0 && len) {
> + while (len) {
> /*
> * Recalculate credits when extent tree depth changes.
> */
> @@ -4430,9 +4429,13 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> inode->i_ino, map.m_lblk,
> map.m_len, ret);
> ext4_mark_inode_dirty(handle, inode);
> - ret2 = ext4_journal_stop(handle);
> + ext4_journal_stop(handle);
> break;
> }
> + /*
> + * allow a full retry cycle for any remaining allocations
> + */
> + retries = 0;
> map.m_lblk += ret;
> map.m_len = len = len - ret;
> epos = (loff_t)map.m_lblk << inode->i_blkbits;
> @@ -4450,11 +4453,8 @@ static int ext4_alloc_file_blocks(struct file *file, ext4_lblk_t offset,
> if (unlikely(ret2))
> break;
> }
> - if (ret == -ENOSPC &&
> - ext4_should_retry_alloc(inode->i_sb, &retries)) {
> - ret = 0;
> + if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
> goto retry;
> - }
>
> return ret > 0 ? ret2 : ret;
> }
> --
> 2.20.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
