From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from m16.mail.163.com (m16.mail.163.com [220.197.31.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6192430F526; Mon, 5 Jan 2026 02:59:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=220.197.31.2 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767581991; cv=none; b=YOTt/WPiAqhgraqRxLYa2ESAWVCvbM4Vk/MR6v/n/qfLkCN51F7ZZeIie72YBsemn4K2Vxa4RPgiT1kxrGXBzFZJcfR/2Dhw4Dx2WpdcPOkZDEHFCe425OqQoQx5JSzkuNmFjvRdk94tNbw2OKPZAnRH1II0lk3d4NDEDfmC7OM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767581991; c=relaxed/simple; bh=c/mkKQs9bnXNk+rEOrGSqkJdIjdFyH19RvNkl4tYIHo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=SEcrknWUzssoXuar67dkDptbI13QXXIViJwtaYkphqixkelgpY7ckBOlujoH+IJbMzsO/LfT3nCiwMrTsR7dvEfltUssQz8Qd49AJJWMT8z1CJYCAaBHsJdiexMNYt3BRAoWgUPxN2aLHvPkP6J+SMF6jy+uyeBNwIjeQ2qt/LA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=163.com; spf=pass smtp.mailfrom=163.com; dkim=pass (1024-bit key) header.d=163.com header.i=@163.com header.b=WGfzxyVI; arc=none smtp.client-ip=220.197.31.2 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=163.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=163.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=163.com header.i=@163.com header.b="WGfzxyVI" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Message-ID:Date:MIME-Version:Subject:To:From: Content-Type; bh=k/wDDq/c0Y07ER+L2EOyIK/ZfCUXFlUnopGGQGTncAk=; b=WGfzxyVIt36WkMWcCbY+I+caXpjpcrYrwWIjDjkCMHHpUZIfDIruLGLRWMhHr4 OaaqV2CtWHx5eNzqXhVhN0o7VOQdRri5q4Zop4jyTix25z8FGF8nXFtDfSoN+sP9 jBZgOq4VUiLbaX1kIDIpEiSzzvljQ7/gqWO70BREAX7bQ= Received: from [10.42.20.201] (unknown []) by gzsmtp5 (Coremail) with SMTP id QCgvCgBHozHhKFtpej7pKA--.136S2; Mon, 05 Jan 2026 10:58:45 +0800 (CST) Message-ID: Date: Mon, 5 Jan 2026 10:58:41 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 9/9] exfat: support multi-cluster for exfat_get_cluster To: "Yuezhang.Mo@sony.com" Cc: "brauner@kernel.org" , "chizhiling@kylinos.cn" , "jack@suse.cz" , "linkinjeon@kernel.org" , "linux-fsdevel@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "sj1557.seo@samsung.com" , "viro@zeniv.linux.org.uk" , "willy@infradead.org" References: Content-Language: en-US From: Chi Zhiling In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-CM-TRANSID:QCgvCgBHozHhKFtpej7pKA--.136S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxCry7KF47Xr48trWfWrykKrg_yoW7Gr48pr WxKa45trs3X34xCw48tw4kZ3yS9F97tF47Jw15Jwn8Cryvqr4F9rn8trnIyF1rCw48uanF vr4Fgw17ursxA3DanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x0pEb4SnUUUUU= X-CM-SenderInfo: hfkl6xxlol0wi6rwjhhfrp/xtbC+AV0EmlbKOWwigAA3e On 1/4/26 15:56, Yuezhang.Mo@sony.com wrote: >> On 12/30/25 17:06, Yuezhang.Mo@sony.com wrote: >>>> + /* >>>> + * 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. >> > > So we also need to check the cache in the following, right? Uh, I don't think it's necessary in here, because these clusters won't exist in the cache. In the cache_lru, all exfat_cache start from non-continuous clusters. This is because exfat_get_cluster adds consecutive clusters to the cache from left to right, which means that the left side of all caches is non-continuous. For instance, if a file contains two extents, [0,30] and [31,60], then exfat_cache must start at either 0 or 31, right? When we have found a cache is [31, 45], then there won't be [41, 60] in the cache_lru. So when we already get some head clusters of a continuous extent, the tail cluster will definitely not be present in the cache. --- Here are some modifications regarding this patch (which may be reflected in the V2 version). Do you have any thoughts or suggestions on this? diff --git a/fs/exfat/cache.c b/fs/exfat/cache.c index 1ec531859944..8ff416beea3c 100644 --- a/fs/exfat/cache.c +++ b/fs/exfat/cache.c @@ -80,6 +80,10 @@ static inline void exfat_cache_update_lru(struct inode *inode, list_move(&cache->cache_list, &ei->cache_lru); } +/* + * Return fcluster of the cache which behind fclus, or + * EXFAT_EOF_CLUSTER if no cache in there. + */ static bool exfat_cache_lookup(struct inode *inode, unsigned int fclus, struct exfat_cache_id *cid, unsigned int *cached_fclus, unsigned int *cached_dclus) @@ -87,6 +91,7 @@ static bool exfat_cache_lookup(struct inode *inode, struct exfat_inode_info *ei = EXFAT_I(inode); static struct exfat_cache nohit = { .fcluster = 0, }; struct exfat_cache *hit = &nohit, *p; + unsigned int next = EXFAT_EOF_CLUSTER; unsigned int offset; spin_lock(&ei->cache_lru_lock); @@ -98,8 +103,9 @@ static bool exfat_cache_lookup(struct inode *inode, offset = hit->nr_contig; } else { offset = fclus - hit->fcluster; - break; } + } else if (p->fcluster > fclus && p->fcluster < next) { + next = p->fcluster; } } if (hit != &nohit) { @@ -114,7 +120,7 @@ static bool exfat_cache_lookup(struct inode *inode, } spin_unlock(&ei->cache_lru_lock); - return hit != &nohit; + return next; } static struct exfat_cache *exfat_cache_merge(struct inode *inode, @@ -243,7 +249,7 @@ int exfat_get_cluster(struct inode *inode, unsigned int cluster, struct exfat_inode_info *ei = EXFAT_I(inode); struct buffer_head *bh = NULL; struct exfat_cache_id cid; - unsigned int content, fclus; + unsigned int content, fclus, next; unsigned int end = cluster + *count - 1; if (ei->start_clu == EXFAT_FREE_CLUSTER) { @@ -272,14 +278,15 @@ int exfat_get_cluster(struct inode *inode, unsigned int cluster, return 0; cache_init(&cid, fclus, *dclus); - exfat_cache_lookup(inode, cluster, &cid, &fclus, dclus); + next = exfat_cache_lookup(inode, cluster, &cid, &fclus, dclus); - /* - * Return on cache hit to keep the code simple. - */ if (fclus == cluster) { - *count = cid.fcluster + cid.nr_contig - fclus + 1; - return 0; + /* The cache includes all cluster requested */ + if (cid.fcluster + cid.nr_contig >= end) + return 0; + /* No cache hole behind this cache */ + if (next == cid.fcluster + cid.nr_contig + 1) + return 0; } /* Thanks, > > ``` > /* > * Collect the remaining clusters of this contiguous extent. > */ > if (*dclus != EXFAT_EOF_CLUSTER) { > unsigned int clu = *dclus; > > /* > * Now the cid cache contains the first cluster requested, > * Advance the fclus to the last cluster of contiguous > * extent, then update the count and cid cache accordingly. > */ > while (fclus < end) { > if (exfat_ent_get(sb, clu, &content, &bh)) > goto err; > if (++clu != content) { > /* TODO: read ahead if content valid */ > break; > } > fclus++; > } > cid.nr_contig = fclus - cid.fcluster; > *count = fclus - cluster + 1; > ``` > >>>> >>>> + 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,