| Message ID | 20220528110017.354175-3-libaokun1@huawei.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | ext4: fix two bugs in ext4_mb_normalize_request | expand |
On 22/05/28 07:00PM, Baokun Li wrote: > ext4_mb_normalize_request() can move logical start of allocated blocks > to reduce fragmentation and better utilize preallocation. However logical > block requested as a start of allocation (ac->ac_o_ex.fe_logical) should > always be covered by allocated blocks so we should check that by > modifying and to or in the assertion. > > Signed-off-by: Baokun Li <libaokun1@huawei.com> Changes looks good to me as we discussed. Feel free to add - Reviewed-by: Ritesh Harjani <ritesh.list@gmail.com> > --- > V1->V2: > Change Fixes from dfe076c106f6 to c9de560ded61. > V2->V3: > Delete Fixes tag. > Add more comments and commit logs to make the code easier to understand. > > fs/ext4/mballoc.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > index 4d3740fdff90..9e06334771a3 100644 > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c > @@ -4185,7 +4185,22 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac, > } > rcu_read_unlock(); > > - if (start + size <= ac->ac_o_ex.fe_logical && > + /* > + * In this function "start" and "size" are normalized for better > + * alignment and length such that we could preallocate more blocks. > + * This normalization is done such that original request of > + * ac->ac_o_ex.fe_logical & fe_len should always lie within "start" and > + * "size" boundaries. > + * (Note fe_len can be relaxed since FS block allocation API does not > + * provide gurantee on number of contiguous blocks allocation since that > + * depends upon free space left, etc). > + * In case of inode pa, later we use the allocated blocks > + * [pa_start + fe_logical - pa_lstart, fe_len/size] from the preallocated > + * range of goal/best blocks [start, size] to put it at the > + * ac_o_ex.fe_logical extent of this inode. > + * (See ext4_mb_use_inode_pa() for more details) > + */ > + if (start + size <= ac->ac_o_ex.fe_logical || > start > ac->ac_o_ex.fe_logical) { > ext4_msg(ac->ac_sb, KERN_ERR, > "start %lu, size %lu, fe_logical %lu", > -- > 2.31.1 >
在 2022/5/28 23:12, Ritesh Harjani 写道: > On 22/05/28 07:00PM, Baokun Li wrote: >> ext4_mb_normalize_request() can move logical start of allocated blocks >> to reduce fragmentation and better utilize preallocation. However logical >> block requested as a start of allocation (ac->ac_o_ex.fe_logical) should >> always be covered by allocated blocks so we should check that by >> modifying and to or in the assertion. >> >> Signed-off-by: Baokun Li <libaokun1@huawei.com> > Changes looks good to me as we discussed. Feel free to add - > > Reviewed-by: Ritesh Harjani <ritesh.list@gmail.com> > >> --- >> V1->V2: >> Change Fixes from dfe076c106f6 to c9de560ded61. >> V2->V3: >> Delete Fixes tag. >> Add more comments and commit logs to make the code easier to understand. >> >> fs/ext4/mballoc.c | 17 ++++++++++++++++- >> 1 file changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c >> index 4d3740fdff90..9e06334771a3 100644 >> --- a/fs/ext4/mballoc.c >> +++ b/fs/ext4/mballoc.c >> @@ -4185,7 +4185,22 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac, >> } >> rcu_read_unlock(); >> >> - if (start + size <= ac->ac_o_ex.fe_logical && >> + /* >> + * In this function "start" and "size" are normalized for better >> + * alignment and length such that we could preallocate more blocks. >> + * This normalization is done such that original request of >> + * ac->ac_o_ex.fe_logical & fe_len should always lie within "start" and >> + * "size" boundaries. >> + * (Note fe_len can be relaxed since FS block allocation API does not >> + * provide gurantee on number of contiguous blocks allocation since that >> + * depends upon free space left, etc). >> + * In case of inode pa, later we use the allocated blocks >> + * [pa_start + fe_logical - pa_lstart, fe_len/size] from the preallocated >> + * range of goal/best blocks [start, size] to put it at the >> + * ac_o_ex.fe_logical extent of this inode. >> + * (See ext4_mb_use_inode_pa() for more details) >> + */ >> + if (start + size <= ac->ac_o_ex.fe_logical || >> start > ac->ac_o_ex.fe_logical) { >> ext4_msg(ac->ac_sb, KERN_ERR, >> "start %lu, size %lu, fe_logical %lu", >> -- >> 2.31.1 >> > . Thank you for your review!
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 4d3740fdff90..9e06334771a3 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -4185,7 +4185,22 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac, } rcu_read_unlock(); - if (start + size <= ac->ac_o_ex.fe_logical && + /* + * In this function "start" and "size" are normalized for better + * alignment and length such that we could preallocate more blocks. + * This normalization is done such that original request of + * ac->ac_o_ex.fe_logical & fe_len should always lie within "start" and + * "size" boundaries. + * (Note fe_len can be relaxed since FS block allocation API does not + * provide gurantee on number of contiguous blocks allocation since that + * depends upon free space left, etc). + * In case of inode pa, later we use the allocated blocks + * [pa_start + fe_logical - pa_lstart, fe_len/size] from the preallocated + * range of goal/best blocks [start, size] to put it at the + * ac_o_ex.fe_logical extent of this inode. + * (See ext4_mb_use_inode_pa() for more details) + */ + if (start + size <= ac->ac_o_ex.fe_logical || start > ac->ac_o_ex.fe_logical) { ext4_msg(ac->ac_sb, KERN_ERR, "start %lu, size %lu, fe_logical %lu",
ext4_mb_normalize_request() can move logical start of allocated blocks to reduce fragmentation and better utilize preallocation. However logical block requested as a start of allocation (ac->ac_o_ex.fe_logical) should always be covered by allocated blocks so we should check that by modifying and to or in the assertion. Signed-off-by: Baokun Li <libaokun1@huawei.com> --- V1->V2: Change Fixes from dfe076c106f6 to c9de560ded61. V2->V3: Delete Fixes tag. Add more comments and commit logs to make the code easier to understand. fs/ext4/mballoc.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)