Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 2/2] exfat: unify name extraction
To:     Sungjong Seo <sj1557.seo@samsung.com>
Cc:     kohada.tetsuhiro@dc.mitsubishielectric.co.jp,
        mori.takahiro@ab.mitsubishielectric.co.jp,
        motai.hirotaka@aj.mitsubishielectric.co.jp,
        'Namjae Jeon' <namjae.jeon@samsung.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20200806055653.9329-1-kohada.t2@gmail.com>
 <CGME20200806055726epcas1p2f36810983abf14d3aa27f8a102bbbc4d@epcas1p2.samsung.com>
 <20200806055653.9329-2-kohada.t2@gmail.com>
 <000201d66da8$07a2c750$16e855f0$@samsung.com>
From:   Tetsuhiro Kohada <kohada.t2@gmail.com>
Message-ID: <bbd9355c-cd48-b961-0a91-771a702c03df@gmail.com>
Date:   Wed, 12 Aug 2020 15:02:17 +0900
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <000201d66da8$07a2c750$16e855f0$@samsung.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.
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".

Should I add this to the commit-message?


BR
---
Tetsuhiro Kohada <kohada.t2@gmail.com>