> Thanks for your reply.
>
> On 2020/08/09 2:19, Sungjong Seo wrote:
> > [snip]
> >> @@ -963,80 +942,38 @@ int exfat_find_dir_entry(struct super_block
> >> *sb, struct exfat_inode_info *ei,
> >> num_empty = 0;
> >> candi_empty.eidx = EXFAT_HINT_NONE;
> >>
> > [snip]
> >>
> >> - if (entry_type &
> >> - (TYPE_CRITICAL_SEC |
> > TYPE_BENIGN_SEC)) {
> >> - if (step == DIRENT_STEP_SECD) {
> >> - if (++order == num_ext)
> >> - goto found;
> >> - continue;
> >> - }
> >> + exfat_get_uniname_from_name_entries(es, &uni_name);
> >
> > It is needed to check a return value.
>
> I'll fix it in v2.
>
>
> >> + exfat_free_dentry_set(es, false);
> >> +
> >> + if (!exfat_uniname_ncmp(sb,
> >> + p_uniname->name,
> >> + uni_name.name,
> >> + name_len)) {
> >> + /* set the last used position as hint */
> >> + hint_stat->clu = clu.dir;
> >> + hint_stat->eidx = dentry;
> >
> > eidx and clu of hint_stat should have one for the next entry we'll
> > start looking for.
> > Did you intentionally change the concept?
>
> Yes, this is intentional.
> Essentially, the "Hint" concept is to reduce the next seek cost with
> minimal cost.
> There is a difference in the position of the hint, but the concept is the
> same.
> As you can see, the patched code strategy doesn't move from current
> position.
> Basically, the original code strategy is advancing only one dentry.(It's
> the "minimum cost") However, when it reaches the cluster boundary, it gets
> the next cluster and error handling.
I didn't get exactly what "original code" is.
Do you mean whole code lines for exfat_find_dir_entry()?
Or just only for handling the hint in it?
The strategy of original code for hint is advancing not one dentry but one dentry_set.
If a hint position is not moved to next like the patched code,
caller have to start at old dentry_set that could be already loaded on dentry cache.
Let's think the case of searching through all files sequentially.
The patched code should check twice per a file.
No better than the original policy.
> Getting the next cluster The error handling already exists at the end of
> the while loop, so the code is duplicated.
> These costs should be paid next time and are no longer the "minimum cost".
I agree with your words, "These costs should be paid next time".
If so, how about moving the cluster handling for a hint dentry to
the beginning of the function while keeping the original policy?
BTW, this patch is not related to the hint code.
I think it would be better to keep the original code in this patch and improve it with a separate patch.
> Should I add this to the commit-message?
>
>
> BR
> ---
> Tetsuhiro Kohada <kohada.t2@xxxxxxxxx>
