[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <74dbb5f1-1565-4971-ae5c-c176d62cfa8f@kernel.org>
Date: Mon, 29 Jul 2024 10:54:46 +0800
From: Chao Yu <chao@...nel.org>
To: Jaegeuk Kim <jaegeuk@...nel.org>
Cc: linux-f2fs-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] f2fs: don't traverse directory blocks after EOF
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.
Thanks,
>
>>
>> Thanks,
>>
>>>
>>>>
>>>> Meanwhile, it uses dir_blocks() instead of variable for cleanup in
>>>> __f2fs_find_entry().
>>>>
>>>> Signed-off-by: Chao Yu <chao@...nel.org>
>>>> ---
>>>> 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
Powered by blists - more mailing lists