Date:   Thu, 28 Mar 2019 17:17:26 -0400
From:   "J. Bruce Fields" <bfields@fieldses.org>
To:     Scott Mayhew <smayhew@redhat.com>
Cc:     jlayton@kernel.org, linux-nfs@vger.kernel.org
Subject: Re: [PATCH RFC v3 0/5] un-deprecate nfsdcld
Message-ID: <20190328211726.GA31938@fieldses.org>
References: <20190326220630.2911-1-smayhew@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190326220630.2911-1-smayhew@redhat.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

Thanks!  It looks good to me on a first skim, but I probably won't get a
proper look at it till next week.

You say "RFC"--are there still known issue that you're working on?

--b.

On Tue, Mar 26, 2019 at 06:06:25PM -0400, Scott Mayhew wrote:
> When nfsdcld was released, it was quickly deprecated in favor of the
> nfsdcltrack usermodehelper, so as to not require another running daemon.
> That prevents NFSv4 clients from reclaiming locks from nfsd's running in
> containers, since neither nfsdcltrack nor the legacy client tracking
> code work in containers.  These patches un-deprecate the use of nfsdcld
> for NFSv4 client tracking.
> 
> These patches are intended to go alongside some nfs-utils patches that
> introduce an enhancement that allows nfsd to "slurp" up the client
> records during client tracking initialization and store them internally
> in hash table.  This enables nfsd to check whether an NFSv4 client is
> allowed to reclaim without having to do an upcall to nfsdcld.  It also
> allows nfsd to decide to end the v4 grace period early if the number of
> RECLAIM_COMPLETE operations it has received from "known" clients is
> equal to the number of entries in the hash table.  It also allows nfsd
> to skip the v4 grace period altogether if it knows there are no clients
> allowed to reclaim.
> 
> There is a fallback to allow nfsd to continue to work with older nfsdcld
> daemons in the event that any are out in the wild (unlikely).
> Everything should work fine except nfsd will not be able to exit the
> grace period early or skip the grace period altogether.
> 
> v3:
> - nfs4_state_start_net() now calls nfsd4_end_grace() instead of
>   open-coding it
> - Removed some unnecessary initializations of nr_reclaim_complete
> - Removed dec_reclaim_complete() altogether.  If we're calling
>   expire_client() as a result of receiving a DESTROY_CLIENTID or
>   SETCLIENTID_CONFIRM then we wouldn't want to decrement the count.  And
>   laundromat thread never expires clients due to loss of lease until the
>   grace period is over (the laundromat gets rescheduled by 1 second as
>   long as clients are reclaiming during that 1 second, for up to 2x the
>   original grace period in total).  So dec_reclaim_complete() is
>   unnecessary.
> - Changed the preference order of client tracking methods.  The new
>   order is 1) new nfdscld, 2) old nfsdcld, 3) nfsdcltrack, 4) legacy
>   v4recovery directory.
> - Added some special handling of legacy records sent by nfsdcld in the
>   GraceStart downcalls.  nfsdcld will do a one-time "upgrade" from old
>   tracking methods.  No changes are needed to accomodate nfsdcltrack
>   records, but legacy records will be prefixed with the string "hash:", 
>   and in the event that we do have legacy records in the
>   reclaim_str_hashtbl we may need to do a second lookup using the hash
>   in the event that a lookup using the client name string fails.
> 
> v2:
> - Addressed some coding style issues in nfsd4_create_clid_dir() &
>   nfsd4_remove_clid_dir()
> 
> Scott Mayhew (5):
>   nfsd: make nfs4_client_reclaim use an xdr_netobj instead of a fixed
>     char array
>   nfsd: un-deprecate nfsdcld
>   nfsd: keep a tally of RECLAIM_COMPLETE operations when using nfsdcld
>   nfsd: re-order client tracking method selection
>   nfsd: handle legacy client tracking records sent by nfsdcld
> 
>  fs/nfsd/netns.h               |   3 +
>  fs/nfsd/nfs4recover.c         | 436 +++++++++++++++++++++++++++++++---
>  fs/nfsd/nfs4state.c           |  63 +++--
>  fs/nfsd/nfsctl.c              |   1 +
>  fs/nfsd/state.h               |   8 +-
>  include/uapi/linux/nfsd/cld.h |   1 +
>  6 files changed, 457 insertions(+), 55 deletions(-)
> 
> -- 
> 2.17.2