Message ID | 20230725185106.2147273-11-shikemeng@huaweicloud.com |
---|---|
State | Superseded |
Headers | show |
Series | A few fixes and cleanups to mballoc | expand |
On Wed, Jul 26, 2023 at 02:51:06AM +0800, Kemeng Shi wrote: > We named criteria with CR_XXX, correct stale comment to criteria with > raw number. Hi Kemeng, Thanks for the cleanups. > > Fixes: f52f3d2b9fba ("ext4: Give symbolic names to mballoc criterias") > Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> > Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > --- > fs/ext4/mballoc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > index 36eea63eaace..de5da76e6748 100644 > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c > @@ -2777,8 +2777,8 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac) > > /* > * ac->ac_2order is set only if the fe_len is a power of 2 > - * if ac->ac_2order is set we also set criteria to 0 so that we > - * try exact allocation using buddy. > + * if ac->ac_2order is set we also set criteria to CR_POWER2_ALIGNED > + * so that we try exact allocation using buddy. > */ > i = fls(ac->ac_g_ex.fe_len); > ac->ac_2order = 0; > @@ -2835,8 +2835,8 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac) > /* > * Batch reads of the block allocation bitmaps > * to get multiple READs in flight; limit > - * prefetching at cr=0/1, otherwise mballoc can > - * spend a lot of time loading imperfect groups > + * prefetching at cr below CR_FAST, otherwise mballoc One of my earlier patchset has replaced the CR_FAST macro with ext4_mb_cr_expensive() so maybe we can account for that here: https://lore.kernel.org/linux-ext4/20230630085927.140137-1-ojaswin@linux.ibm.com/ Regards, ojaswin > + * can spend a lot of time loading imperfect groups > */ > if ((prefetch_grp == group) && > (cr >= CR_FAST || > -- > 2.30.0 >
on 7/26/2023 10:50 PM, Ojaswin Mujoo wrote: > On Wed, Jul 26, 2023 at 02:51:06AM +0800, Kemeng Shi wrote: >> We named criteria with CR_XXX, correct stale comment to criteria with >> raw number. > > Hi Kemeng, > > Thanks for the cleanups. > >> >> Fixes: f52f3d2b9fba ("ext4: Give symbolic names to mballoc criterias") >> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> >> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> >> --- >> fs/ext4/mballoc.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c >> index 36eea63eaace..de5da76e6748 100644 >> --- a/fs/ext4/mballoc.c >> +++ b/fs/ext4/mballoc.c >> @@ -2777,8 +2777,8 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac) >> >> /* >> * ac->ac_2order is set only if the fe_len is a power of 2 >> - * if ac->ac_2order is set we also set criteria to 0 so that we >> - * try exact allocation using buddy. >> + * if ac->ac_2order is set we also set criteria to CR_POWER2_ALIGNED >> + * so that we try exact allocation using buddy. >> */ >> i = fls(ac->ac_g_ex.fe_len); >> ac->ac_2order = 0; >> @@ -2835,8 +2835,8 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac) >> /* >> * Batch reads of the block allocation bitmaps >> * to get multiple READs in flight; limit >> - * prefetching at cr=0/1, otherwise mballoc can >> - * spend a lot of time loading imperfect groups >> + * prefetching at cr below CR_FAST, otherwise mballoc > > One of my earlier patchset has replaced the CR_FAST macro with > ext4_mb_cr_expensive() so maybe we can account for that here: > > https://lore.kernel.org/linux-ext4/20230630085927.140137-1-ojaswin@linux.ibm.com/ > Hi Ojaswin, sorry for missing this. I still could not find the comment update of stale comment "limit prefetching at cr=0/1" in that patch. Maybe we could update comment to "prefetching at inexpensive CR, otherwise ...". What do you think. Or did I miss anything.
On Thu, Jul 27, 2023 at 09:29:11AM +0800, Kemeng Shi wrote: > > > on 7/26/2023 10:50 PM, Ojaswin Mujoo wrote: > > On Wed, Jul 26, 2023 at 02:51:06AM +0800, Kemeng Shi wrote: > >> We named criteria with CR_XXX, correct stale comment to criteria with > >> raw number. > > > > Hi Kemeng, > > > > Thanks for the cleanups. > > > >> > >> Fixes: f52f3d2b9fba ("ext4: Give symbolic names to mballoc criterias") > >> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> > >> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > >> --- > >> fs/ext4/mballoc.c | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > >> index 36eea63eaace..de5da76e6748 100644 > >> --- a/fs/ext4/mballoc.c > >> +++ b/fs/ext4/mballoc.c > >> @@ -2777,8 +2777,8 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac) > >> > >> /* > >> * ac->ac_2order is set only if the fe_len is a power of 2 > >> - * if ac->ac_2order is set we also set criteria to 0 so that we > >> - * try exact allocation using buddy. > >> + * if ac->ac_2order is set we also set criteria to CR_POWER2_ALIGNED > >> + * so that we try exact allocation using buddy. > >> */ > >> i = fls(ac->ac_g_ex.fe_len); > >> ac->ac_2order = 0; > >> @@ -2835,8 +2835,8 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac) > >> /* > >> * Batch reads of the block allocation bitmaps > >> * to get multiple READs in flight; limit > >> - * prefetching at cr=0/1, otherwise mballoc can > >> - * spend a lot of time loading imperfect groups > >> + * prefetching at cr below CR_FAST, otherwise mballoc > > > > One of my earlier patchset has replaced the CR_FAST macro with > > ext4_mb_cr_expensive() so maybe we can account for that here: > > > > https://lore.kernel.org/linux-ext4/20230630085927.140137-1-ojaswin@linux.ibm.com/ > > > Hi Ojaswin, sorry for missing this. I still could not find the comment update > of stale comment "limit prefetching at cr=0/1" in that patch. Maybe we could > update comment to "prefetching at inexpensive CR, otherwise ...". What do > you think. Or did I miss anything. Hey Kemeng, That's right I missed the update but just wanted to let you know that CR_FAST would be removed. "prefetching at inexpensive CRs, ..." sounds good to me. Regards, ojaswin > > -- > Best wishes > Kemeng Shi > > Regards, > > ojaswin > > > >> + * can spend a lot of time loading imperfect groups > > > >> */ > >> if ((prefetch_grp == group) && > >> (cr >= CR_FAST || > >> -- > >> 2.30.0 > >> > > >
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 36eea63eaace..de5da76e6748 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -2777,8 +2777,8 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac) /* * ac->ac_2order is set only if the fe_len is a power of 2 - * if ac->ac_2order is set we also set criteria to 0 so that we - * try exact allocation using buddy. + * if ac->ac_2order is set we also set criteria to CR_POWER2_ALIGNED + * so that we try exact allocation using buddy. */ i = fls(ac->ac_g_ex.fe_len); ac->ac_2order = 0; @@ -2835,8 +2835,8 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac) /* * Batch reads of the block allocation bitmaps * to get multiple READs in flight; limit - * prefetching at cr=0/1, otherwise mballoc can - * spend a lot of time loading imperfect groups + * prefetching at cr below CR_FAST, otherwise mballoc + * can spend a lot of time loading imperfect groups */ if ((prefetch_grp == group) && (cr >= CR_FAST ||