RE: [PATCH v1 2/2] exfat: hint the empty entry which at the end of cluster chain

RE: [PATCH v1 2/2] exfat: hint the empty entry which at the end of cluster chain

[Sungjong Seo]

Hi, Yuezhang Mo,

> After traversing all directory entries, hint the empty directory
> entry no matter whether or not there are enough empty directory
> entries.
> 
> After this commit, hint the empty directory entries like this:
> 
> 1. Hint the deleted directory entries if enough;
> 2. Hint the deleted and unused directory entries which at the
>    end of the cluster chain no matter whether enough or not(Add
>    by this commit);
> 3. If no any empty directory entries, hint the empty directory
>    entries in the new cluster(Add by this commit).
> 
> This avoids repeated traversal of directory entries, reduces CPU
> usage, and improves the performance of creating files and
> directories(especially on low-performance CPUs).
> 
> Test create 5000 files in a class 4 SD card on imx6q-sabrelite
> with:
> 
> for ((i=0;i<5;i++)); do
>    sync
>    time (for ((j=1;j<=1000;j++)); do touch file$((i*1000+j)); done)
> done
> 
> The more files, the more performance improvements.
> 
>             Before   After    Improvement
>    1~1000   25.360s  22.168s  14.40%
> 1001~2000   38.242s  28.72ss  33.15%
> 2001~3000   49.134s  35.037s  40.23%
> 3001~4000   62.042s  41.624s  49.05%
> 4001~5000   73.629s  46.772s  57.42%
> 
> 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   | 26 ++++++++++++++++++++++----
>  fs/exfat/namei.c | 22 ++++++++++++++--------
>  2 files changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
> index a569f285f4fd..7600f3521246 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -936,18 +936,25 @@ struct exfat_entry_set_cache
> *exfat_get_dentry_set(struct super_block *sb,
> 
>  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)
> +		int dentry, int num_entries, int entry_type)
>  {
>  	if (ei->hint_femp.eidx == EXFAT_HINT_NONE ||
>  	    ei->hint_femp.count < num_entries ||
>  	    ei->hint_femp.eidx > dentry) {
> +		int total_entries = EXFAT_B_TO_DEN(i_size_read(&ei-
> >vfs_inode));
> +
>  		if (candi_empty->count == 0) {
>  			candi_empty->cur = *clu;
>  			candi_empty->eidx = dentry;
>  		}
> 
> -		candi_empty->count++;
> -		if (candi_empty->count == num_entries)
> +		if (entry_type == TYPE_UNUSED)
> +			candi_empty->count += total_entries - dentry;

This seems like a very good approach. Perhaps the key fix that improved
performance seems to be the handling of cases where empty space was not
found and ended with TYPE_UNUSED.

However, there are concerns about trusting and using the number of free
entries after TYPE_UNUSED calculated based on directory size. This is
because, unlike exFAT Spec., in the real world, unexpected TYPE_UNUSED
entries may exist. :( 
That's why exfat_search_empty_slot() checks if there is any valid entry
after TYPE_UNUSED. In my experience, they can be caused by a wrong FS driver
and H/W defects, and the probability of occurrence is not low.

Therefore, when the lookup ends with TYPE_UNUSED, if there are no empty
entries found yet, it would be better to set the last empty entry to
hint_femp.eidx and set hint_femp.count to 0.
If so, even if the lookup ends with TYPE_UNUSED, exfat_search_empty_slot()
can start searching from the position of the last empty entry and check
whether there are actually empty entries as many as the required
num_entries as now.

what do you think?

> +		else
> +			candi_empty->count++;
> +
> +		if (candi_empty->count == num_entries ||
> +		    candi_empty->count + candi_empty->eidx == total_entries)
>  			ei->hint_femp = *candi_empty;
>  	}
>  }
[snip]
> --
> 2.25.1

Re: [PATCH v1 2/2] exfat: hint the empty entry which at the end of cluster chain

[Namjae Jeon]

Add missing Cc: Yuezhang Mo.

2022-10-31 15:17 GMT+09:00, Sungjong Seo <sj1557.seo@samsung.com>:
> Hi, Yuezhang Mo,
>
>> After traversing all directory entries, hint the empty directory
>> entry no matter whether or not there are enough empty directory
>> entries.
>>
>> After this commit, hint the empty directory entries like this:
>>
>> 1. Hint the deleted directory entries if enough;
>> 2. Hint the deleted and unused directory entries which at the
>>    end of the cluster chain no matter whether enough or not(Add
>>    by this commit);
>> 3. If no any empty directory entries, hint the empty directory
>>    entries in the new cluster(Add by this commit).
>>
>> This avoids repeated traversal of directory entries, reduces CPU
>> usage, and improves the performance of creating files and
>> directories(especially on low-performance CPUs).
>>
>> Test create 5000 files in a class 4 SD card on imx6q-sabrelite
>> with:
>>
>> for ((i=0;i<5;i++)); do
>>    sync
>>    time (for ((j=1;j<=1000;j++)); do touch file$((i*1000+j)); done)
>> done
>>
>> The more files, the more performance improvements.
>>
>>             Before   After    Improvement
>>    1~1000   25.360s  22.168s  14.40%
>> 1001~2000   38.242s  28.72ss  33.15%
>> 2001~3000   49.134s  35.037s  40.23%
>> 3001~4000   62.042s  41.624s  49.05%
>> 4001~5000   73.629s  46.772s  57.42%
>>
>> 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   | 26 ++++++++++++++++++++++----
>>  fs/exfat/namei.c | 22 ++++++++++++++--------
>>  2 files changed, 36 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
>> index a569f285f4fd..7600f3521246 100644
>> --- a/fs/exfat/dir.c
>> +++ b/fs/exfat/dir.c
>> @@ -936,18 +936,25 @@ struct exfat_entry_set_cache
>> *exfat_get_dentry_set(struct super_block *sb,
>>
>>  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)
>> +		int dentry, int num_entries, int entry_type)
>>  {
>>  	if (ei->hint_femp.eidx == EXFAT_HINT_NONE ||
>>  	    ei->hint_femp.count < num_entries ||
>>  	    ei->hint_femp.eidx > dentry) {
>> +		int total_entries = EXFAT_B_TO_DEN(i_size_read(&ei-
>> >vfs_inode));
>> +
>>  		if (candi_empty->count == 0) {
>>  			candi_empty->cur = *clu;
>>  			candi_empty->eidx = dentry;
>>  		}
>>
>> -		candi_empty->count++;
>> -		if (candi_empty->count == num_entries)
>> +		if (entry_type == TYPE_UNUSED)
>> +			candi_empty->count += total_entries - dentry;
>
> This seems like a very good approach. Perhaps the key fix that improved
> performance seems to be the handling of cases where empty space was not
> found and ended with TYPE_UNUSED.
>
> However, there are concerns about trusting and using the number of free
> entries after TYPE_UNUSED calculated based on directory size. This is
> because, unlike exFAT Spec., in the real world, unexpected TYPE_UNUSED
> entries may exist. :(
> That's why exfat_search_empty_slot() checks if there is any valid entry
> after TYPE_UNUSED. In my experience, they can be caused by a wrong FS
> driver
> and H/W defects, and the probability of occurrence is not low.
>
> Therefore, when the lookup ends with TYPE_UNUSED, if there are no empty
> entries found yet, it would be better to set the last empty entry to
> hint_femp.eidx and set hint_femp.count to 0.
> If so, even if the lookup ends with TYPE_UNUSED, exfat_search_empty_slot()
> can start searching from the position of the last empty entry and check
> whether there are actually empty entries as many as the required
> num_entries as now.
>
> what do you think?
>
>> +		else
>> +			candi_empty->count++;
>> +
>> +		if (candi_empty->count == num_entries ||
>> +		    candi_empty->count + candi_empty->eidx == total_entries)
>>  			ei->hint_femp = *candi_empty;
>>  	}
>>  }
> [snip]
>> --
>> 2.25.1
>
>

RE: [PATCH v1 2/2] exfat: hint the empty entry which at the end of cluster chain

[Yuezhang.Mo]

> > This seems like a very good approach. Perhaps the key fix that
> > improved performance seems to be the handling of cases where empty
> > space was not found and ended with TYPE_UNUSED.
> >
> > However, there are concerns about trusting and using the number of
> > free entries after TYPE_UNUSED calculated based on directory size.
> > This is because, unlike exFAT Spec., in the real world, unexpected
> > TYPE_UNUSED entries may exist. :( That's why exfat_search_empty_slot()
> > checks if there is any valid entry after TYPE_UNUSED. In my
> > experience, they can be caused by a wrong FS driver and H/W defects,
> > and the probability of occurrence is not low.
> >
> > Therefore, when the lookup ends with TYPE_UNUSED, if there are no
> > empty entries found yet, it would be better to set the last empty
> > entry to hint_femp.eidx and set hint_femp.count to 0.
> > If so, even if the lookup ends with TYPE_UNUSED,
> > exfat_search_empty_slot() can start searching from the position of the
> > last empty entry and check whether there are actually empty entries as
> > many as the required num_entries as now.
> >
> > what do you think?

We plan to add a new helper exfat_get_empty_dentry_set(), this helper is called before
setting the entry type, it caches and then checks for empty entries(Check-on-write is safer
than checking when looking for empty directory entries).

       for (i = 0; i < es->num_entries; i++) {
               ep = exfat_get_dentry_cached(es, i);
               type = exfat_get_entry_type(ep);
               if (type == TYPE_UNUSED)
                       unused_hit = true;
               else if (type == TYPE_DELETED) {
                       if (unused_hit)
                               goto error;
               } else
                       goto error;
       }

This code is not ready, we are testing and internal reviewing.

> -----Original Message-----
> From: Namjae Jeon <linkinjeon@kernel.org>
> Sent: Monday, October 31, 2022 2:32 PM
> To: Sungjong Seo <sj1557.seo@samsung.com>; Mo, Yuezhang
> <Yuezhang.Mo@sony.com>
> Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>; linux-kernel
> <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v1 2/2] exfat: hint the empty entry which at the end of
> cluster chain
> 
> Add missing Cc: Yuezhang Mo.
> 
> 2022-10-31 15:17 GMT+09:00, Sungjong Seo <sj1557.seo@samsung.com>:
> > Hi, Yuezhang Mo,
> >
> >> After traversing all directory entries, hint the empty directory
> >> entry no matter whether or not there are enough empty directory
> >> entries.
> >>
> >> After this commit, hint the empty directory entries like this:
> >>
> >> 1. Hint the deleted directory entries if enough; 2. Hint the deleted
> >> and unused directory entries which at the
> >>    end of the cluster chain no matter whether enough or not(Add
> >>    by this commit);
> >> 3. If no any empty directory entries, hint the empty directory
> >>    entries in the new cluster(Add by this commit).
> >>
> >> This avoids repeated traversal of directory entries, reduces CPU
> >> usage, and improves the performance of creating files and
> >> directories(especially on low-performance CPUs).
> >>
> >> Test create 5000 files in a class 4 SD card on imx6q-sabrelite
> >> with:
> >>
> >> for ((i=0;i<5;i++)); do
> >>    sync
> >>    time (for ((j=1;j<=1000;j++)); do touch file$((i*1000+j)); done)
> >> done
> >>
> >> The more files, the more performance improvements.
> >>
> >>             Before   After    Improvement
> >>    1~1000   25.360s  22.168s  14.40%
> >> 1001~2000   38.242s  28.72ss  33.15%
> >> 2001~3000   49.134s  35.037s  40.23%
> >> 3001~4000   62.042s  41.624s  49.05%
> >> 4001~5000   73.629s  46.772s  57.42%
> >>
> >> 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   | 26 ++++++++++++++++++++++----
> >>  fs/exfat/namei.c | 22 ++++++++++++++--------
> >>  2 files changed, 36 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index
> >> a569f285f4fd..7600f3521246 100644
> >> --- a/fs/exfat/dir.c
> >> +++ b/fs/exfat/dir.c
> >> @@ -936,18 +936,25 @@ struct exfat_entry_set_cache
> >> *exfat_get_dentry_set(struct super_block *sb,
> >>
> >>  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)
> >> +		int dentry, int num_entries, int entry_type)
> >>  {
> >>  	if (ei->hint_femp.eidx == EXFAT_HINT_NONE ||
> >>  	    ei->hint_femp.count < num_entries ||
> >>  	    ei->hint_femp.eidx > dentry) {
> >> +		int total_entries = EXFAT_B_TO_DEN(i_size_read(&ei-
> >> >vfs_inode));
> >> +
> >>  		if (candi_empty->count == 0) {
> >>  			candi_empty->cur = *clu;
> >>  			candi_empty->eidx = dentry;
> >>  		}
> >>
> >> -		candi_empty->count++;
> >> -		if (candi_empty->count == num_entries)
> >> +		if (entry_type == TYPE_UNUSED)
> >> +			candi_empty->count += total_entries - dentry;
> >
> > This seems like a very good approach. Perhaps the key fix that
> > improved performance seems to be the handling of cases where empty
> > space was not found and ended with TYPE_UNUSED.
> >
> > However, there are concerns about trusting and using the number of
> > free entries after TYPE_UNUSED calculated based on directory size.
> > This is because, unlike exFAT Spec., in the real world, unexpected
> > TYPE_UNUSED entries may exist. :( That's why exfat_search_empty_slot()
> > checks if there is any valid entry after TYPE_UNUSED. In my
> > experience, they can be caused by a wrong FS driver and H/W defects,
> > and the probability of occurrence is not low.
> >
> > Therefore, when the lookup ends with TYPE_UNUSED, if there are no
> > empty entries found yet, it would be better to set the last empty
> > entry to hint_femp.eidx and set hint_femp.count to 0.
> > If so, even if the lookup ends with TYPE_UNUSED,
> > exfat_search_empty_slot() can start searching from the position of the
> > last empty entry and check whether there are actually empty entries as
> > many as the required num_entries as now.
> >
> > what do you think?
> >
> >> +		else
> >> +			candi_empty->count++;
> >> +
> >> +		if (candi_empty->count == num_entries ||
> >> +		    candi_empty->count + candi_empty->eidx == total_entries)
> >>  			ei->hint_femp = *candi_empty;
> >>  	}
> >>  }
> > [snip]
> >> --
> >> 2.25.1
> >
> >

RE: [PATCH v1 2/2] exfat: hint the empty entry which at the end of cluster chain

[Yuezhang.Mo]

At this stage, we can check this case in exfat_search_empty_slot() by removing the following if statement.

-               if (num_entries <= hint_femp->count) {
-                       hint_femp->eidx = EXFAT_HINT_NONE;
-                       return dentry;
-               }

What do you think?

> -----Original Message-----
> From: Mo, Yuezhang
> Sent: Monday, October 31, 2022 7:05 PM
> To: Namjae Jeon <linkinjeon@kernel.org>; Sungjong Seo
> <sj1557.seo@samsung.com>
> Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>; linux-kernel
> <linux-kernel@vger.kernel.org>
> Subject: RE: [PATCH v1 2/2] exfat: hint the empty entry which at the end of
> cluster chain
> 
> > > This seems like a very good approach. Perhaps the key fix that
> > > improved performance seems to be the handling of cases where empty
> > > space was not found and ended with TYPE_UNUSED.
> > >
> > > However, there are concerns about trusting and using the number of
> > > free entries after TYPE_UNUSED calculated based on directory size.
> > > This is because, unlike exFAT Spec., in the real world, unexpected
> > > TYPE_UNUSED entries may exist. :( That's why
> > > exfat_search_empty_slot() checks if there is any valid entry after
> > > TYPE_UNUSED. In my experience, they can be caused by a wrong FS
> > > driver and H/W defects, and the probability of occurrence is not low.
> > >
> > > Therefore, when the lookup ends with TYPE_UNUSED, if there are no
> > > empty entries found yet, it would be better to set the last empty
> > > entry to hint_femp.eidx and set hint_femp.count to 0.
> > > If so, even if the lookup ends with TYPE_UNUSED,
> > > exfat_search_empty_slot() can start searching from the position of
> > > the last empty entry and check whether there are actually empty
> > > entries as many as the required num_entries as now.
> > >
> > > what do you think?
> 
> We plan to add a new helper exfat_get_empty_dentry_set(), this helper is
> called before setting the entry type, it caches and then checks for empty
> entries(Check-on-write is safer than checking when looking for empty directory
> entries).
> 
>        for (i = 0; i < es->num_entries; i++) {
>                ep = exfat_get_dentry_cached(es, i);
>                type = exfat_get_entry_type(ep);
>                if (type == TYPE_UNUSED)
>                        unused_hit = true;
>                else if (type == TYPE_DELETED) {
>                        if (unused_hit)
>                                goto error;
>                } else
>                        goto error;
>        }
> 
> This code is not ready, we are testing and internal reviewing.
> 
> > -----Original Message-----
> > From: Namjae Jeon <linkinjeon@kernel.org>
> > Sent: Monday, October 31, 2022 2:32 PM
> > To: Sungjong Seo <sj1557.seo@samsung.com>; Mo, Yuezhang
> > <Yuezhang.Mo@sony.com>
> > Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>; linux-kernel
> > <linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH v1 2/2] exfat: hint the empty entry which at the
> > end of cluster chain
> >
> > Add missing Cc: Yuezhang Mo.
> >
> > 2022-10-31 15:17 GMT+09:00, Sungjong Seo <sj1557.seo@samsung.com>:
> > > Hi, Yuezhang Mo,
> > >
> > >> After traversing all directory entries, hint the empty directory
> > >> entry no matter whether or not there are enough empty directory
> > >> entries.
> > >>
> > >> After this commit, hint the empty directory entries like this:
> > >>
> > >> 1. Hint the deleted directory entries if enough; 2. Hint the
> > >> deleted and unused directory entries which at the
> > >>    end of the cluster chain no matter whether enough or not(Add
> > >>    by this commit);
> > >> 3. If no any empty directory entries, hint the empty directory
> > >>    entries in the new cluster(Add by this commit).
> > >>
> > >> This avoids repeated traversal of directory entries, reduces CPU
> > >> usage, and improves the performance of creating files and
> > >> directories(especially on low-performance CPUs).
> > >>
> > >> Test create 5000 files in a class 4 SD card on imx6q-sabrelite
> > >> with:
> > >>
> > >> for ((i=0;i<5;i++)); do
> > >>    sync
> > >>    time (for ((j=1;j<=1000;j++)); do touch file$((i*1000+j)); done)
> > >> done
> > >>
> > >> The more files, the more performance improvements.
> > >>
> > >>             Before   After    Improvement
> > >>    1~1000   25.360s  22.168s  14.40%
> > >> 1001~2000   38.242s  28.72ss  33.15%
> > >> 2001~3000   49.134s  35.037s  40.23%
> > >> 3001~4000   62.042s  41.624s  49.05%
> > >> 4001~5000   73.629s  46.772s  57.42%
> > >>
> > >> 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   | 26 ++++++++++++++++++++++----
> > >>  fs/exfat/namei.c | 22 ++++++++++++++--------
> > >>  2 files changed, 36 insertions(+), 12 deletions(-)
> > >>
> > >> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index
> > >> a569f285f4fd..7600f3521246 100644
> > >> --- a/fs/exfat/dir.c
> > >> +++ b/fs/exfat/dir.c
> > >> @@ -936,18 +936,25 @@ struct exfat_entry_set_cache
> > >> *exfat_get_dentry_set(struct super_block *sb,
> > >>
> > >>  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)
> > >> +		int dentry, int num_entries, int entry_type)
> > >>  {
> > >>  	if (ei->hint_femp.eidx == EXFAT_HINT_NONE ||
> > >>  	    ei->hint_femp.count < num_entries ||
> > >>  	    ei->hint_femp.eidx > dentry) {
> > >> +		int total_entries = EXFAT_B_TO_DEN(i_size_read(&ei-
> > >> >vfs_inode));
> > >> +
> > >>  		if (candi_empty->count == 0) {
> > >>  			candi_empty->cur = *clu;
> > >>  			candi_empty->eidx = dentry;
> > >>  		}
> > >>
> > >> -		candi_empty->count++;
> > >> -		if (candi_empty->count == num_entries)
> > >> +		if (entry_type == TYPE_UNUSED)
> > >> +			candi_empty->count += total_entries - dentry;
> > >
> > > This seems like a very good approach. Perhaps the key fix that
> > > improved performance seems to be the handling of cases where empty
> > > space was not found and ended with TYPE_UNUSED.
> > >
> > > However, there are concerns about trusting and using the number of
> > > free entries after TYPE_UNUSED calculated based on directory size.
> > > This is because, unlike exFAT Spec., in the real world, unexpected
> > > TYPE_UNUSED entries may exist. :( That's why
> > > exfat_search_empty_slot() checks if there is any valid entry after
> > > TYPE_UNUSED. In my experience, they can be caused by a wrong FS
> > > driver and H/W defects, and the probability of occurrence is not low.
> > >
> > > Therefore, when the lookup ends with TYPE_UNUSED, if there are no
> > > empty entries found yet, it would be better to set the last empty
> > > entry to hint_femp.eidx and set hint_femp.count to 0.
> > > If so, even if the lookup ends with TYPE_UNUSED,
> > > exfat_search_empty_slot() can start searching from the position of
> > > the last empty entry and check whether there are actually empty
> > > entries as many as the required num_entries as now.
> > >
> > > what do you think?
> > >
> > >> +		else
> > >> +			candi_empty->count++;
> > >> +
> > >> +		if (candi_empty->count == num_entries ||
> > >> +		    candi_empty->count + candi_empty->eidx == total_entries)
> > >>  			ei->hint_femp = *candi_empty;
> > >>  	}
> > >>  }
> > > [snip]
> > >> --
> > >> 2.25.1
> > >
> > >

RE: [PATCH v1 2/2] exfat: hint the empty entry which at the end of cluster chain

[Sungjong Seo]

Hi, Yuezhang,

> At this stage, we can check this case in exfat_search_empty_slot() by
> removing the following if statement.
> 
> -               if (num_entries <= hint_femp->count) {
> -                       hint_femp->eidx = EXFAT_HINT_NONE;
> -                       return dentry;
> -               }
> 
> What do you think?

Are you planning to remove the code below as well?
+              /* All entries searched but not enough empty entries */
+               if (dentry + hint_femp->count == p_dir->size * dentries_per_clu)
+                       return -ENOSPC;

Otherwise, with appropriate comments, it could be modified to:

+                /*
+                 * If hint_femp->count is enough, it is needed to check if
+                 * there are actual empty entries.
+                 * Otherwise, and if "dentry + hint_famp->count" is also equal
+                 * to "p_dir->size * dentries_per_clu", it means ENOSPC.
+                 */
+                if ((num_entries > hint_femp->count) &&
+                     (dentry + hint_femp->count ==
+                      p_dir->size * dentries_per_clu))
+                        return -ENOSPC;

As of now, it seems like the simplest and the best way.

> 
> > -----Original Message-----
> > From: Mo, Yuezhang
> > Sent: Monday, October 31, 2022 7:05 PM
> > To: Namjae Jeon <linkinjeon@kernel.org>; Sungjong Seo
> > <sj1557.seo@samsung.com>
> > Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>; linux-kernel
> > <linux-kernel@vger.kernel.org>
> > Subject: RE: [PATCH v1 2/2] exfat: hint the empty entry which at the
> > end of cluster chain
> >
> > > > This seems like a very good approach. Perhaps the key fix that
> > > > improved performance seems to be the handling of cases where empty
> > > > space was not found and ended with TYPE_UNUSED.
> > > >
> > > > However, there are concerns about trusting and using the number of
> > > > free entries after TYPE_UNUSED calculated based on directory size.
> > > > This is because, unlike exFAT Spec., in the real world, unexpected
> > > > TYPE_UNUSED entries may exist. :( That's why
> > > > exfat_search_empty_slot() checks if there is any valid entry after
> > > > TYPE_UNUSED. In my experience, they can be caused by a wrong FS
> > > > driver and H/W defects, and the probability of occurrence is not low.
> > > >
> > > > Therefore, when the lookup ends with TYPE_UNUSED, if there are no
> > > > empty entries found yet, it would be better to set the last empty
> > > > entry to hint_femp.eidx and set hint_femp.count to 0.
> > > > If so, even if the lookup ends with TYPE_UNUSED,
> > > > exfat_search_empty_slot() can start searching from the position of
> > > > the last empty entry and check whether there are actually empty
> > > > entries as many as the required num_entries as now.
> > > >
> > > > what do you think?
> >
> > We plan to add a new helper exfat_get_empty_dentry_set(), this helper
> > is called before setting the entry type, it caches and then checks for
> > empty entries(Check-on-write is safer than checking when looking for
> > empty directory entries).
> >
> >        for (i = 0; i < es->num_entries; i++) {
> >                ep = exfat_get_dentry_cached(es, i);
> >                type = exfat_get_entry_type(ep);
> >                if (type == TYPE_UNUSED)
> >                        unused_hit = true;
> >                else if (type == TYPE_DELETED) {
> >                        if (unused_hit)
> >                                goto error;
> >                } else
> >                        goto error;
> >        }
> >
> > This code is not ready, we are testing and internal reviewing.
> >
> > > -----Original Message-----
> > > From: Namjae Jeon <linkinjeon@kernel.org>
> > > Sent: Monday, October 31, 2022 2:32 PM
> > > To: Sungjong Seo <sj1557.seo@samsung.com>; Mo, Yuezhang
> > > <Yuezhang.Mo@sony.com>
> > > Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>; linux-kernel
> > > <linux-kernel@vger.kernel.org>
> > > Subject: Re: [PATCH v1 2/2] exfat: hint the empty entry which at the
> > > end of cluster chain
> > >
> > > Add missing Cc: Yuezhang Mo.
> > >
> > > 2022-10-31 15:17 GMT+09:00, Sungjong Seo <sj1557.seo@samsung.com>:
> > > > Hi, Yuezhang Mo,
> > > >
> > > >> After traversing all directory entries, hint the empty directory
> > > >> entry no matter whether or not there are enough empty directory
> > > >> entries.
> > > >>
> > > >> After this commit, hint the empty directory entries like this:
> > > >>
> > > >> 1. Hint the deleted directory entries if enough; 2. Hint the
> > > >> deleted and unused directory entries which at the
> > > >>    end of the cluster chain no matter whether enough or not(Add
> > > >>    by this commit);
> > > >> 3. If no any empty directory entries, hint the empty directory
> > > >>    entries in the new cluster(Add by this commit).
> > > >>
> > > >> This avoids repeated traversal of directory entries, reduces CPU
> > > >> usage, and improves the performance of creating files and
> > > >> directories(especially on low-performance CPUs).
> > > >>
> > > >> Test create 5000 files in a class 4 SD card on imx6q-sabrelite
> > > >> with:
> > > >>
> > > >> for ((i=0;i<5;i++)); do
> > > >>    sync
> > > >>    time (for ((j=1;j<=1000;j++)); do touch file$((i*1000+j));
> > > >> done) done
> > > >>
> > > >> The more files, the more performance improvements.
> > > >>
> > > >>             Before   After    Improvement
> > > >>    1~1000   25.360s  22.168s  14.40%
> > > >> 1001~2000   38.242s  28.72ss  33.15%
> > > >> 2001~3000   49.134s  35.037s  40.23%
> > > >> 3001~4000   62.042s  41.624s  49.05%
> > > >> 4001~5000   73.629s  46.772s  57.42%
> > > >>
> > > >> 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   | 26 ++++++++++++++++++++++----
> > > >>  fs/exfat/namei.c | 22 ++++++++++++++--------
> > > >>  2 files changed, 36 insertions(+), 12 deletions(-)
> > > >>
> > > >> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index
> > > >> a569f285f4fd..7600f3521246 100644
> > > >> --- a/fs/exfat/dir.c
> > > >> +++ b/fs/exfat/dir.c
> > > >> @@ -936,18 +936,25 @@ struct exfat_entry_set_cache
> > > >> *exfat_get_dentry_set(struct super_block *sb,
> > > >>
> > > >>  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)
> > > >> +		int dentry, int num_entries, int entry_type)
> > > >>  {
> > > >>  	if (ei->hint_femp.eidx == EXFAT_HINT_NONE ||
> > > >>  	    ei->hint_femp.count < num_entries ||
> > > >>  	    ei->hint_femp.eidx > dentry) {
> > > >> +		int total_entries = EXFAT_B_TO_DEN(i_size_read(&ei-
> > > >> >vfs_inode));
> > > >> +
> > > >>  		if (candi_empty->count == 0) {
> > > >>  			candi_empty->cur = *clu;
> > > >>  			candi_empty->eidx = dentry;
> > > >>  		}
> > > >>
> > > >> -		candi_empty->count++;
> > > >> -		if (candi_empty->count == num_entries)
> > > >> +		if (entry_type == TYPE_UNUSED)
> > > >> +			candi_empty->count += total_entries - dentry;
> > > >
> > > > This seems like a very good approach. Perhaps the key fix that
> > > > improved performance seems to be the handling of cases where empty
> > > > space was not found and ended with TYPE_UNUSED.
> > > >
> > > > However, there are concerns about trusting and using the number of
> > > > free entries after TYPE_UNUSED calculated based on directory size.
> > > > This is because, unlike exFAT Spec., in the real world, unexpected
> > > > TYPE_UNUSED entries may exist. :( That's why
> > > > exfat_search_empty_slot() checks if there is any valid entry after
> > > > TYPE_UNUSED. In my experience, they can be caused by a wrong FS
> > > > driver and H/W defects, and the probability of occurrence is not low.
> > > >
> > > > Therefore, when the lookup ends with TYPE_UNUSED, if there are no
> > > > empty entries found yet, it would be better to set the last empty
> > > > entry to hint_femp.eidx and set hint_femp.count to 0.
> > > > If so, even if the lookup ends with TYPE_UNUSED,
> > > > exfat_search_empty_slot() can start searching from the position of
> > > > the last empty entry and check whether there are actually empty
> > > > entries as many as the required num_entries as now.
> > > >
> > > > what do you think?
> > > >
> > > >> +		else
> > > >> +			candi_empty->count++;
> > > >> +
> > > >> +		if (candi_empty->count == num_entries ||
> > > >> +		    candi_empty->count + candi_empty->eidx ==
> total_entries)
> > > >>  			ei->hint_femp = *candi_empty;
> > > >>  	}
> > > >>  }
> > > > [snip]
> > > >> --
> > > >> 2.25.1
> > > >
> > > >