| Message ID | 20190326220630.2911-1-smayhew@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | un-deprecate nfsdcld | expand |
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
On Thu, 28 Mar 2019, J. Bruce Fields wrote: > 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? No, I just added RFC because of the new stuff I added for handling the legacy records. Plus I wasn't sure if you would have a problem with the server not lifting the grace period early if there were legacy records. I could do it if I moved some code out of nfs4recover.c but I chose not to since it should really be a one-time thing and the original legacy tracking didn't lift the grace period early anyway. -Scott > > --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
On Fri, Mar 29, 2019 at 08:16:09AM -0400, Scott Mayhew wrote: > On Thu, 28 Mar 2019, J. Bruce Fields wrote: > > 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? > > No, I just added RFC because of the new stuff I added for handling the > legacy records. Plus I wasn't sure if you would have a problem with the > server not lifting the grace period early if there were legacy records. > I could do it if I moved some code out of nfs4recover.c but I chose not > to since it should really be a one-time thing and the original legacy > tracking didn't lift the grace period early anyway. Yeah, I don't think that case is worth spending time optimizing. The kernel patches look OK to me. I'm inclined to target them for the next merge window (5.2). If Jeff has the time to take a look, it'd also be good to know if he has any more concerns. --b.
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(-)