Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
In-Reply-To: <000001d8ece8$0241bca0$06c535e0$@samsung.com>
References: <CGME20221019072850epcas1p459b27e0d44eb0cc36ec09e9a734dcf60@epcas1p4.samsung.com>
 <PUZPR04MB6316EBE97C82DFBEFE3CCDAF812B9@PUZPR04MB6316.apcprd04.prod.outlook.com>
 <000001d8ece8$0241bca0$06c535e0$@samsung.com>
From:   Namjae Jeon <linkinjeon@kernel.org>
Date:   Mon, 31 Oct 2022 15:30:58 +0900
Message-ID: <CAKYAXd__ypbjLpnNVDxf3UE4M+au2QwYYe2PeY8QsKZCBaO54w@mail.gmail.com>
Subject: Re: [PATCH v1 1/2] exfat: simplify empty entry hint
To:     Sungjong Seo <sj1557.seo@samsung.com>,
        Yuezhang Mo <Yuezhang.Mo@sony.com>
Cc:     linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Add missing Cc: Yuezhang Mo.

2022-10-31 14:16 GMT+09:00, Sungjong Seo <sj1557.seo@samsung.com>:
> Hello, Yuezhang Mo,
>
>> This commit adds exfat_hint_empty_entry() to reduce code complexity and
>> make code more readable.
>>
>> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
>> Reviewed-by: Andy Wu <Andy.Wu@sony.com>
>> Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
>> ---
>>  fs/exfat/dir.c | 56 ++++++++++++++++++++++++++++----------------------
>>  1 file changed, 32 insertions(+), 24 deletions(-)
>>
>> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index
>> 7b648b6662f0..a569f285f4fd 100644
>> --- a/fs/exfat/dir.c
>> +++ b/fs/exfat/dir.c
>> @@ -934,6 +934,24 @@ struct exfat_entry_set_cache
>> *exfat_get_dentry_set(struct super_block *sb,
>>  	return NULL;
>>  }
>>
>> +static inline void exfat_hint_empty_entry(struct exfat_inode_info *ei,
>> +		struct exfat_hint_femp *candi_empty, struct exfat_chain *clu,
>> +		int dentry, int num_entries)
>> +{
>> +	if (ei->hint_femp.eidx == EXFAT_HINT_NONE ||
>> +	    ei->hint_femp.count < num_entries ||
>
> It seems like a good approach.
> BTW, ei->hint_femp.count was already reset at the beginning of
> exfat_find_dir_entry(). So condition-check above could be removed.
> Is there any scenario I'm missing?
>
>> +	    ei->hint_femp.eidx > dentry) {
>> +		if (candi_empty->count == 0) {
>> +			candi_empty->cur = *clu;
>> +			candi_empty->eidx = dentry;
>> +		}
>> +
>> +		candi_empty->count++;
>> +		if (candi_empty->count == num_entries)
>> +			ei->hint_femp = *candi_empty;
>> +	}
>> +}
>> +
>>  enum {
>>  	DIRENT_STEP_FILE,
>>  	DIRENT_STEP_STRM,
>> @@ -958,7 +976,7 @@ int exfat_find_dir_entry(struct super_block *sb,
>> struct exfat_inode_info *ei,  {
>>  	int i, rewind = 0, dentry = 0, end_eidx = 0, num_ext = 0, len;
>>  	int order, step, name_len = 0;
>> -	int dentries_per_clu, num_empty = 0;
>> +	int dentries_per_clu;
>>  	unsigned int entry_type;
>>  	unsigned short *uniname = NULL;
>>  	struct exfat_chain clu;
>> @@ -976,7 +994,15 @@ int exfat_find_dir_entry(struct super_block *sb,
>> struct exfat_inode_info *ei,
>>  		end_eidx = dentry;
>>  	}
>>
>> -	candi_empty.eidx = EXFAT_HINT_NONE;
>> +	if (ei->hint_femp.eidx != EXFAT_HINT_NONE &&
>> +	    ei->hint_femp.count < num_entries)
>> +		ei->hint_femp.eidx = EXFAT_HINT_NONE;
>> +
>> +	if (ei->hint_femp.eidx == EXFAT_HINT_NONE)
>> +		ei->hint_femp.count = 0;
>> +
>> +	candi_empty = ei->hint_femp;
>> +
>
> It would be nice to make the code block above a static inline function as
> well.
>
>>  rewind:
>>  	order = 0;
>>  	step = DIRENT_STEP_FILE;
> [snip]
>> --
>> 2.25.1
>
>