On Thu, Jun 14, 2018 at 06:01:27AM -0400, Jeff Layton wrote:
> I know this is a "best effort" sort of thing, but should this be done
> with atomic loads and stores (READ_ONCE/WRITE_ONCE)?
We're just setting it to 1 when a reclaim comes in, and then once a
second checking the result and clearing it. So yes that's a slightly
sloppy way of checking whether something happens once a second.
I *think* the worst that could happen is something like:
read 0 from somebody_reclaimed
reclaim writes 1 to somebody_reclaimed
write 0 to somebody_reclaimed
and then a client that's reclaiming only very close to the 1-second mark
could get overlooked.
I'm assuming in any reasonable situation that a client can manage at
least 10-100 reclaims a second, and that the race window is probably
several orders of magnitude less than a second (even after taking into
account weird memory ordering).
Would READ/WRITE_ONCE do it right? I'm not sure it's worth it.
--b.
>
> >
> > dprintk("NFSD: nfsd4_open filename %.*s op_openowner %p\n",
> > (int)open->op_fname.len, open->op_fname.data,
> > @@ -424,6 +425,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > if (status)
> > goto out;
> > open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
> > + reclaim = true;
> > case NFS4_OPEN_CLAIM_FH:
> > case NFS4_OPEN_CLAIM_DELEG_CUR_FH:
> > status = do_open_fhandle(rqstp, cstate, open);
> > @@ -452,6 +454,8 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > WARN(status && open->op_created,
> > "nfsd4_process_open2 failed to open newly-created file! status=%u\n",
> > be32_to_cpu(status));
> > + if (reclaim && !status)
> > + nn->somebody_reclaimed = true;
> > out:
> > if (resfh && resfh != &cstate->current_fh) {
> > fh_dup2(&cstate->current_fh, resfh);
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 59ae65d3eec3..ffe816fe6e89 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -4689,6 +4689,28 @@ nfsd4_end_grace(struct nfsd_net *nn)
> > */
> > }
> >
> > +/*
> > + * If we've waited a lease period but there are still clients trying to
> > + * reclaim, wait a little longer to give them a chance to finish.
> > + */
> > +static bool clients_still_reclaiming(struct nfsd_net *nn)
> > +{
> > + unsigned long now = get_seconds();
> > + unsigned long double_grace_period_end = nn->boot_time +
> > + 2 * nn->nfsd4_lease;
> > +
> > + if (!nn->somebody_reclaimed)
> > + return false;
> > + nn->somebody_reclaimed = false;
> > + /*
> > + * If we've given them *two* lease times to reclaim, and they're
> > + * still not done, give up:
> > + */
> > + if (time_after(now, double_grace_period_end))
> > + return false;
> > + return true;
> > +}
> > +
> > static time_t
> > nfs4_laundromat(struct nfsd_net *nn)
> > {
> > @@ -4702,6 +4724,11 @@ nfs4_laundromat(struct nfsd_net *nn)
> > time_t t, new_timeo = nn->nfsd4_lease;
> >
> > dprintk("NFSD: laundromat service - starting\n");
> > +
> > + if (clients_still_reclaiming(nn)) {
> > + new_timeo = 0;
> > + goto out;
> > + }
> > nfsd4_end_grace(nn);
> > INIT_LIST_HEAD(&reaplist);
> > spin_lock(&nn->client_lock);
> > @@ -4799,7 +4826,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> > posix_unblock_lock(&nbl->nbl_lock);
> > free_blocked_lock(nbl);
> > }
> > -
> > +out:
> > new_timeo = max_t(time_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
> > return new_timeo;
> > }
> > @@ -6049,6 +6076,8 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > case 0: /* success! */
> > nfs4_inc_and_copy_stateid(&lock->lk_resp_stateid, &lock_stp->st_stid);
> > status = 0;
> > + if (lock->lk_reclaim)
> > + nn->somebody_reclaimed = true;
> > break;
> > case FILE_LOCK_DEFERRED:
> > nbl = NULL;
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index d107b4426f7e..5f22476cf371 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -1239,6 +1239,7 @@ static __net_init int nfsd_init_net(struct net *net)
> > goto out_idmap_error;
> > nn->nfsd4_lease = 90; /* default lease time */
> > nn->nfsd4_grace = 90;
> > + nn->somebody_reclaimed = false;
> > nn->clverifier_counter = prandom_u32();
> > nn->clientid_counter = prandom_u32();
> >
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
