ext4: reset retry counter when ext4_alloc_file_blocks() makes progress
PATCH v2 1/1 ext4: reset retry counter when ext4_alloc_file_blocks() makes progress
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:
--- fs/ext4/extents.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
fs/ext4/extents.c +8 -8
@@ -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; }
Signature · 1 line
2.20.1
Quoted 16 lines · Eric Whitney
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 toext4_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.
Quoted 5 lines
v2: Silence smatch warning by initializing ret.
For smatch warning:
Reported-by kernel test robot <[email protected]>Reported-by Dan Carpenter <[email protected]>
Looks good to me. Feel free to add:
Honza
Quoted 60 lines
Signed-off-by Eric Whitney <[email protected]>--- fs/ext4/extents.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)fs/ext4/extents.c +8
-8@@ -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; }Signature · 1 line
2.20.1
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR