On 07/29, Chao Yu wrote:
> On 2024/7/27 11:27, Jaegeuk Kim wrote:
> > On 07/26, Chao Yu wrote:
> > > On 2024/7/26 0:55, Jaegeuk Kim wrote:
> > > > On 07/12, Chao Yu wrote:
> > > > > All directory blocks are within the scope of i_size, so let's limit
> > > > > the end_block to just check valid dirent blocks.
> > > > 
> > > > Do we really need this?
> > > 
> > > f2fs_readdir() and f2fs_empty_dir() uses dir_blocks() for upper boundary,
> > > this patch aligns find_in_level() w/ them.
> > > 
> > > Also, it can avoid grabbing never used page cache across EOF.
> > > 
> > > So, we can consider taking this patch?
> > 
> > I'm wondering whether the current code has a bug or not.
> 
> I think it's not a bug, but, in corner case it may waste memory and
> cause a bit performance reduction.

Well, I don't think it's really a problem.

> 
> Thanks,
> 
> > 
> > > 
> > > Thanks,
> > > 
> > > > 
> > > > > 
> > > > > Meanwhile, it uses dir_blocks() instead of variable for cleanup in
> > > > > __f2fs_find_entry().
> > > > > 
> > > > > Signed-off-by: Chao Yu <[email protected]>
> > > > > ---
> > > > >    fs/f2fs/dir.c | 6 ++++--
> > > > >    1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> > > > > index 02c9355176d3..d4591c215f07 100644
> > > > > --- a/fs/f2fs/dir.c
> > > > > +++ b/fs/f2fs/dir.c
> > > > > @@ -305,18 +305,21 @@ static struct f2fs_dir_entry 
> > > > > *find_in_level(struct inode *dir,
> > > > >       int s = GET_DENTRY_SLOTS(fname->disk_name.len);
> > > > >       unsigned int nbucket, nblock;
> > > > >       unsigned int bidx, end_block;
> > > > > +     unsigned long last_block;
> > > > >       struct page *dentry_page;
> > > > >       struct f2fs_dir_entry *de = NULL;
> > > > >       pgoff_t next_pgofs;
> > > > >       bool room = false;
> > > > >       int max_slots;
> > > > > +     last_block = dir_blocks(dir);
> > > > >       nbucket = dir_buckets(level, F2FS_I(dir)->i_dir_level);
> > > > >       nblock = bucket_blocks(level);
> > > > >       bidx = dir_block_index(level, F2FS_I(dir)->i_dir_level,
> > > > >                              le32_to_cpu(fname->hash) % nbucket);
> > > > >       end_block = bidx + nblock;
> > > > > +     end_block = min_t(unsigned int, end_block, last_block);
> > > > >       while (bidx < end_block) {
> > > > >               /* no need to allocate new dentry pages to all the 
> > > > > indices */
> > > > > @@ -361,7 +364,6 @@ struct f2fs_dir_entry *__f2fs_find_entry(struct 
> > > > > inode *dir,
> > > > >                                        const struct f2fs_filename 
> > > > > *fname,
> > > > >                                        struct page **res_page)
> > > > >    {
> > > > > -     unsigned long npages = dir_blocks(dir);
> > > > >       struct f2fs_dir_entry *de = NULL;
> > > > >       unsigned int max_depth;
> > > > >       unsigned int level;
> > > > > @@ -373,7 +375,7 @@ struct f2fs_dir_entry *__f2fs_find_entry(struct 
> > > > > inode *dir,
> > > > >               goto out;
> > > > >       }
> > > > > -     if (npages == 0)
> > > > > +     if (dir_blocks(dir) == 0)
> > > > >               goto out;
> > > > >       max_depth = F2FS_I(dir)->i_current_depth;
> > > > > -- 
> > > > > 2.40.1


_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to