On Tue, Jun 24, 2025 at 09:45:27AM +0800, Su Hui wrote:
> 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.
>
Yeah, probably break is better.
regards,
dan carpenter
