Re: [PATCH v1 9/9] exfat: support multi-cluster for exfat_get_cluster

[Chi Zhiling]

On 12/30/25 17:06, Yuezhang.Mo@xxxxxxxx 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,

>