RE: [PATCH v1 1/2] exfat: simplify empty entry hint

[Sungjong Seo]

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@xxxxxxxx>
> Reviewed-by: Andy Wu <Andy.Wu@xxxxxxxx>
> Reviewed-by: Aoyama Wataru <wataru.aoyama@xxxxxxxx>
> ---
>  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