On 2025/6/23 23:47, Dan Carpenter wrote:
On Mon, Jun 23, 2025 at 08:22:27PM +0800, Su Hui wrote:
Using guard() to replace *unlock* label. guard() makes lock/unlock code
more clear. Change the order of the code to let all lock code in the
same scope. No functional changes.
Signed-off-by: Su Hui <suhui@xxxxxxxxxxxx>
---
fs/nfsd/nfscache.c | 99 ++++++++++++++++++++++------------------------
1 file changed, 48 insertions(+), 51 deletions(-)
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index ba9d326b3de6..2d92adf3e6b0 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -489,7 +489,7 @@ int nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
if (type == RC_NOCACHE) {
nfsd_stats_rc_nocache_inc(nn);
- goto out;
+ return rtn;
}
csum = nfsd_cache_csum(&rqstp->rq_arg, start, len);
@@ -500,64 +500,61 @@ int nfsd_cache_lookup(struct svc_rqst *rqstp, unsigned int start,
*/
rp = nfsd_cacherep_alloc(rqstp, csum, nn);
if (!rp)
- goto out;
+ return rtn;
b = nfsd_cache_bucket_find(rqstp->rq_xid, nn);
- spin_lock(&b->cache_lock);
- found = nfsd_cache_insert(b, rp, nn);
- if (found != rp)
- goto found_entry;
- *cacherep = rp;
- rp->c_state = RC_INPROG;
- nfsd_prune_bucket_locked(nn, b, 3, &dispose);
- spin_unlock(&b->cache_lock);
+ scoped_guard(spinlock, &b->cache_lock) {
+ found = nfsd_cache_insert(b, rp, nn);
+ if (found == rp) {
+ *cacherep = rp;
+ rp->c_state = RC_INPROG;
+ nfsd_prune_bucket_locked(nn, b, 3, &dispose);
+ goto out;
It took me a while to figure out why we've added a goto here. In the
original code this "goto out;" was a "spin_unlock(&b->cache_lock);".
The spin_unlock() is more readable because you can immediately see that
it's trying to drop the lock where a "goto out;" is less obvious about
the intention.
Does "break;" be better in this place? Meaning Break this lock guard scope.
But as NeillBrown suggestion[1], this patch will be replaced by several
patches.
No matter what, this "goto out;" will be removed in the next v2 patchset.
I think this patch works fine, but I'm not sure it's an improvement.
Got it, thanks for your suggestions!
[1]
https://lore.kernel.org/all/175072435698.2280845.12079422273351211469@xxxxxxxxxxxxxxxxxxxxx/
regards,
Su Hui
