On Tue, 9 Sep 2014 16:56:51 -0400
Jeff Layton <jlayton@xxxxxxxxxxxxxxx> wrote:
> On Tue, 9 Sep 2014 16:46:17 -0400
> "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
>
> > On Mon, Sep 08, 2014 at 10:46:48AM -0400, Jeff Layton wrote:
> > > The nfsdcltrack upcall doesn't utilize the NFSD4_CLIENT_STABLE flag,
> > > which basically results in an upcall every time we call into the client
> > > tracking ops.
> > >
> > > Change it to set this bit on a successful "check" or "create" request,
> > > and clear it on a "remove" request. Also, check to see if that bit is
> > > set before upcalling on a "check" or "remove" request, and skip
> > > upcalling appropriately, depending on its state.
> > >
> > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
> > > ---
> > > fs/nfsd/nfs4recover.c | 22 ++++++++++++++++++----
> > > 1 file changed, 18 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> > > index ae313861a6f5..5697e9d2e875 100644
> > > --- a/fs/nfsd/nfs4recover.c
> > > +++ b/fs/nfsd/nfs4recover.c
> > > @@ -1291,7 +1291,8 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp)
> > > grace_start = nfsd4_cltrack_grace_start(nn->boot_time);
> > >
> > > nfsd4_cltrack_upcall_lock(clp);
> > > - nfsd4_umh_cltrack_upcall("create", hexid, reclaim_complete, grace_start);
> > > + if (!nfsd4_umh_cltrack_upcall("create", hexid, reclaim_complete, grace_start))
> > > + set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
> >
> > Don't you also want to skip the upcall when STABLE is set?
> >
> > (Not a big deal in the 4.1 case as it's only called once per client (in
> > RECLAIM_COMPLETE) but annoying in the 4.0 case where it's called on
> > every OPEN_CONFIRM.)
> >
> > --b.
> >
>
> Ahh right, I was thinking that we'd only get one OPEN_CONFIRM per
> client, but you have to do it for each openowner...
>
> No, we can't do that. Consider the case where we might do some "check"
> ops before the "create" operation (i.e. we need to do some reclaims).
>
> The "check" will set the STABLE flag, and then the create operation
> gets skipped. That means that the grace period won't get lifted early
> even when the RECLAIM_COMPLETE comes in since the reclaim_complete
> field in the DB didn't get updated.
>
> We could (in principle) add a new flag that indicates whether a
> "create" has already been done for a client, and use that to skip
> subsequent "create" ops. Sound reasonable?
>
...or we could skip doing the upcall if the client's minorversion is 0
and the STABLE flag is set.
In the v4.0 case, there's really no difference between a "check" and
"create" operation, so there's no real benefit to doing a "create" if
you already did a "check".
> > > nfsd4_cltrack_upcall_unlock(clp);
> > >
> > > kfree(reclaim_complete);
> > > @@ -1304,6 +1305,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
> > > {
> > > char *hexid;
> > >
> > > + if (!test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
> > > + return;
> > > +
> > > hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
> > > if (!hexid) {
> > > dprintk("%s: can't allocate memory for upcall!\n", __func__);
> > > @@ -1311,7 +1315,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
> > > }
> > >
> > > nfsd4_cltrack_upcall_lock(clp);
> > > - nfsd4_umh_cltrack_upcall("remove", hexid, NULL, NULL);
> > > + if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags) &&
> > > + nfsd4_umh_cltrack_upcall("remove", hexid, NULL, NULL) == 0)
> > > + clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
> > > nfsd4_cltrack_upcall_unlock(clp);
> > >
> > > kfree(hexid);
> > > @@ -1323,6 +1329,9 @@ nfsd4_umh_cltrack_check(struct nfs4_client *clp)
> > > int ret;
> > > char *hexid, *legacy;
> > >
> > > + if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
> > > + return 0;
> > > +
> > > hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
> > > if (!hexid) {
> > > dprintk("%s: can't allocate memory for upcall!\n", __func__);
> > > @@ -1331,9 +1340,14 @@ nfsd4_umh_cltrack_check(struct nfs4_client *clp)
> > > legacy = nfsd4_cltrack_legacy_recdir(&clp->cl_name);
> > >
> > > nfsd4_cltrack_upcall_lock(clp);
> > > - ret = nfsd4_umh_cltrack_upcall("check", hexid, legacy, NULL);
> > > + if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) {
> > > + ret = 0;
> > > + } else {
> > > + ret = nfsd4_umh_cltrack_upcall("check", hexid, legacy, NULL);
> > > + if (!ret)
> > > + set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
> > > + }
> > > nfsd4_cltrack_upcall_unlock(clp);
> > > -
> > > kfree(legacy);
> > > kfree(hexid);
> > > return ret;
> > > --
> > > 1.9.3
> > >
>
>
--
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
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
