linux-kernel - Re: [PATCH v1 9/9] exfat: support multi-cluster for exfat_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d832364d-02e6-458d-9eb2-442e1452a0f9@163.com>
Date: Wed, 31 Dec 2025 10:35:11 +0800
From: Chi Zhiling <chizhiling@....com>
To: "Yuezhang.Mo@...y.com" <Yuezhang.Mo@...y.com>
Cc: "brauner@...nel.org" <brauner@...nel.org>,
 "chizhiling@...inos.cn" <chizhiling@...inos.cn>, "jack@...e.cz"
 <jack@...e.cz>, "linkinjeon@...nel.org" <linkinjeon@...nel.org>,
 "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "sj1557.seo@...sung.com" <sj1557.seo@...sung.com>,
 "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
 "willy@...radead.org" <willy@...radead.org>
Subject: Re: [PATCH v1 9/9] exfat: support multi-cluster for exfat_get_cluster

On 12/30/25 17:06, Yuezhang.Mo@...y.com wrote:
>> @@ -243,6 +244,7 @@ int exfat_get_cluster(struct inode *inode, unsigned int cluster,
>>        struct buffer_head *bh = NULL;
>>        struct exfat_cache_id cid;
>>        unsigned int content, fclus;
>> +     unsigned int end = (*count <= 1) ? cluster : cluster + *count - 1;
> 
> In exfat_get_block,  count is set as follows:
> 
> count = EXFAT_B_TO_CLU_ROUND_UP(bh_result->b_size, sbi);
> 
> So '*count' is always greater than or equal to 1, the above condition seems unnecessary.


Yes, it is unnecessary.

> 
>> --- a/fs/exfat/inode.c
>> +++ b/fs/exfat/inode.c
>> @@ -134,6 +134,7 @@ static int exfat_map_cluster(struct inode *inode, unsigned int clu_offset,
>>        struct exfat_inode_info *ei = EXFAT_I(inode);
>>        unsigned int local_clu_offset = clu_offset;
>>        unsigned int num_to_be_allocated = 0, num_clusters;
>> +     unsigned int hint_count = max(*count, 1);
> 
> Same as above, hint_count seems unnecessary.
> 
>> +     /*
>> +      * Return on cache hit to keep the code simple.
>> +      */
>> +     if (fclus == cluster) {
>> +             *count = cid.fcluster + cid.nr_contig - fclus + 1;
>>                return 0;
> 
> If 'cid.fcluster + cid.nr_contig - fclus + 1 < *count', how about continuing to collect clusters?
> The following clusters may be continuous.

I'm glad you noticed this detail. It is necessary to explain this and 
update it in the code comments.

The main reason why I didn't continue the collection was that the 
subsequent clusters might also exist in the cache. This requires us to 
search the cache again to confirm this, and this action might introduce 
additional performance overhead.

I think we can continue to collect, but we need to check the cache 
before doing so.

> 
>> +             while (fclus < end) {
>> +                     if (exfat_ent_get(sb, clu, &content, &bh))
>> +                             goto err;
>> +                     if (++clu != content) {
>> +                             /* TODO: read ahead if content valid */
>> +                             break;
> 
> The next cluster index has been read and will definitely be used.
> How about add it to the cache?

Good idea!
will add it in v2,

thanks,

>