On Sun, May 01, 2022 at 10:38:09AM -0700, Dai Ngo wrote:
> Hi Chuck, Bruce
>
> This series of patches implement the NFSv4 Courteous Server.
Looks good, my only remaining comments (see other email) are minor--with
those fixed, feel free to add my reviewed-by: for the series.
--b.
>
> A server which does not immediately expunge the state on lease expiration
> is known as a Courteous Server. A Courteous Server continues to recognize
> previously generated state tokens as valid until conflict arises between
> the expired state and the requests from another client, or the server
> reboots.
>
> v2:
>
> . add new callback, lm_expire_lock, to lock_manager_operations to
> allow the lock manager to take appropriate action with conflict lock.
>
> . handle conflicts of NFSv4 locks with NFSv3/NLM and local locks.
>
> . expire courtesy client after 24hr if client has not reconnected.
>
> . do not allow expired client to become courtesy client if there are
> waiters for client's locks.
>
> . modify client_info_show to show courtesy client and seconds from
> last renew.
>
> . fix a problem with NFSv4.1 server where the it keeps returning
> SEQ4_STATUS_CB_PATH_DOWN in the successful SEQUENCE reply, after
> the courtesy client reconnects, causing the client to keep sending
> BCTS requests to server.
>
> v3:
>
> . modified posix_test_lock to check and resolve conflict locks
> to handle NLM TEST and NFSv4 LOCKT requests.
>
> . separate out fix for back channel stuck in SEQ4_STATUS_CB_PATH_DOWN.
>
> v4:
>
> . rework nfsd_check_courtesy to avoid dead lock of fl_lock and client_lock
> by asking the laudromat thread to destroy the courtesy client.
>
> . handle NFSv4 share reservation conflicts with courtesy client. This
> includes conflicts between access mode and deny mode and vice versa.
>
> . drop the patch for back channel stuck in SEQ4_STATUS_CB_PATH_DOWN.
>
> v5:
>
> . fix recursive locking of file_rwsem from posix_lock_file.
>
> . retest with LOCKDEP enabled.
>
> v6:
>
> . merge witn 5.15-rc7
>
> . fix a bug in nfs4_check_deny_bmap that did not check for matched
> nfs4_file before checking for access/deny conflict. This bug causes
> pynfs OPEN18 to fail since the server taking too long to release
> lots of un-conflict clients' state.
>
> . enhance share reservation conflict handler to handle case where
> a large number of conflict courtesy clients need to be expired.
> The 1st 100 clients are expired synchronously and the rest are
> expired in the background by the laundromat and NFS4ERR_DELAY
> is returned to the NFS client. This is needed to prevent the
> NFS client from timing out waiting got the reply.
>
> v7:
>
> . Fix race condition in posix_test_lock and posix_lock_inode after
> dropping spinlock.
>
> . Enhance nfsd4_fl_expire_lock to work with with new lm_expire_lock
> callback
>
> . Always resolve share reservation conflicts asynchrously.
>
> . Fix bug in nfs4_laundromat where spinlock is not used when
> scanning cl_ownerstr_hashtbl.
>
> . Fix bug in nfs4_laundromat where idr_get_next was called
> with incorrect 'id'.
>
> . Merge nfs4_destroy_courtesy_client into nfsd4_fl_expire_lock.
>
> v8:
>
> . Fix warning in nfsd4_fl_expire_lock reported by test robot.
>
> v9:
>
> . Simplify lm_expire_lock API by (1) remove the 'testonly' flag
> and (2) specifying return value as true/false to indicate
> whether conflict was succesfully resolved.
>
> . Rework nfsd4_fl_expire_lock to mark client with
> NFSD4_DESTROY_COURTESY_CLIENT then tell the laundromat to expire
> the client in the background.
>
> . Add a spinlock in nfs4_client to synchronize access to the
> NFSD4_COURTESY_CLIENT and NFSD4_DESTROY_COURTESY_CLIENT flag to
> handle race conditions when resolving lock and share reservation
> conflict.
>
> . Courtesy client that was marked as NFSD4_DESTROY_COURTESY_CLIENT
> are now consisdered 'dead', waiting for the laundromat to expire
> it. This client is no longer allowed to use its states if it
> reconnects before the laundromat finishes expiring the client.
>
> For v4.1 client, the detection is done in the processing of the
> SEQUENCE op and returns NFS4ERR_BAD_SESSION to force the client
> to re-establish new clientid and session.
> For v4.0 client, the detection is done in the processing of the
> RENEW and state-related ops and return NFS4ERR_EXPIRE to force
> the client to re-establish new clientid.
>
> v10:
>
> Resolve deadlock in v9 by avoiding getting cl_client and
> cl_cs_lock together. The laundromat needs to determine whether
> the expired client has any state and also has no blockers on
> its locks. Both of these conditions are allowed to change after
> the laundromat transits an expired client to courtesy client.
> When this happens, the laundromat will detect it on the next
> run and and expire the courtesy client.
>
> Remove client persistent record before marking it as COURTESY_CLIENT
> and add client persistent record before clearing the COURTESY_CLIENT
> flag to allow the courtesy client to transist to normal client to
> continue to use its state.
>
> Lock/delegation/share reversation conflict with courtesy client is
> resolved by marking the courtesy client as DESTROY_COURTESY_CLIENT,
> effectively disable it, then allow the current request to proceed
> immediately.
>
> Courtesy client marked as DESTROY_COURTESY_CLIENT is not allowed
> to reconnect to reuse itsstate. It is expired by the laundromat
> asynchronously in the background.
>
> Move processing of expired clients from nfs4_laudromat to a
> separate function, nfs4_get_client_reaplist, that creates the
> reaplist and also to process courtesy clients.
>
> Update Documentation/filesystems/locking.rst to include new
> lm_lock_conflict call.
>
> Modify leases_conflict to call lm_breaker_owns_lease only if
> there is real conflict. This is to allow the lock manager to
> resolve the delegation conflict if possible.
>
> v11:
>
> Add comment for lm_lock_conflict callback.
>
> Replace static const courtesy_client_expiry with macro.
>
> Remove courtesy_clnt argument from find_in_sessionid_hashtbl.
> Callers use nfs4_client->cl_cs_client boolean to determined if
> it's the courtesy client and take appropriate actions.
>
> Rename NFSD4_COURTESY_CLIENT and NFSD4_DESTROY_COURTESY_CLIENT
> with NFSD4_CLIENT_COURTESY and NFSD4_CLIENT_DESTROY_COURTESY.
>
> v12:
>
> Remove unnecessary comment in nfs4_get_client_reaplist.
>
> Replace nfs4_client->cl_cs_client boolean with
> NFSD4_CLIENT_COURTESY_CLNT flag.
>
> Remove courtesy_clnt argument from find_client_in_id_table and
> find_clp_in_name_tree. Callers use NFSD4_CLIENT_COURTESY_CLNT to
> determined if it's the courtesy client and take appropriate actions.
>
> v13:
>
> Merge with 5.17-rc3.
>
> Cleanup Documentation/filesystems/locking.rst: replace i_lock
> with flc_lock, update API's that use flc_lock.
>
> Rename lm_lock_conflict to lm_lock_expired().
>
> Remove comment of lm_lock_expired API in lock_manager_operations.
> Same information is in patch description.
>
> Update commit messages of 4/4.
>
> Add some comment for NFSD4_CLIENT_COURTESY_CLNT.
>
> Add nfsd4_discard_courtesy_clnt() to eliminate duplicate code of
> discarding courtesy client; setting NFSD4_DESTROY_COURTESY_CLIENT.
>
> v14:
>
> . merge with Chuck's public for-next branch.
>
> . remove courtesy_client_expiry, use client's last renew time.
>
> . simplify comment of nfs4_check_access_deny_bmap.
>
> . add comment about race condition in nfs4_get_client_reaplist.
>
> . add list_del when walking cslist in nfs4_get_client_reaplist.
>
> . remove duplicate INIT_LIST_HEAD(&reaplist) from nfs4_laundromat
>
> . Modify find_confirmed_client and find_confirmed_client_by_name
> to detect courtesy client and destroy it.
>
> . refactor lookup_clientid to use find_client_in_id_table
> directly instead of find_confirmed_client.
>
> . refactor nfsd4_setclientid to call find_clp_in_name_tree
> directly instead of find_confirmed_client_by_name.
>
> . remove comment of NFSD4_CLIENT_COURTESY.
>
> . replace NFSD4_CLIENT_DESTROY_COURTESY with NFSD4_CLIENT_EXPIRED.
>
> . replace NFSD4_CLIENT_COURTESY_CLNT with NFSD4_CLIENT_RECONNECTED.
>
> v15:
>
> . add helper locks_has_blockers_locked in fs.h to check for
> lock blockers
>
> . rename nfs4_conflict_clients to nfs4_resolve_deny_conflicts_locked
>
> . update nfs4_upgrade_open() to handle courtesy clients.
>
> . add helper nfs4_check_and_expire_courtesy_client and
> nfs4_is_courtesy_client_expired to deduplicate some code.
>
> . update nfs4_anylock_blocker:
> . replace list_for_each_entry_safe with list_for_each_entry
> . break nfs4_anylock_blocker into 2 smaller functions.
>
> . update nfs4_get_client_reaplist:
> . remove unnecessary commets
> . acquire cl_cs_lock before setting NFSD4_CLIENT_COURTESY flag
>
> . update client_info_show to show 'time since last renew: 00:00:38'
> instead of 'seconds from last renew: 38'.
>
> v16:
>
> . update client_info_show to display 'status' as
> 'confirmed/unconfirmed/courtesy'
>
> . replace helper locks_has_blockers_locked in fs.h in v15 with new
> locks_owner_has_blockers call in fs/locks.c
>
> . update nfs4_lockowner_has_blockers to use locks_owner_has_blockers
>
> . move nfs4_check_and_expire_courtesy_client from 5/11 to 4/11
>
> . remove unnecessary check for NULL clp in find_in_sessionid_hashtb
>
> . fix typo in commit messages
>
> v17:
>
> . replace flags used for courtesy client with enum courtesy_client_state
>
> . add state table in nfsd/state.h
>
> . make nfsd4_expire_courtesy_clnt, nfsd4_discard_courtesy_clnt and
> nfsd4_courtesy_clnt_expired as static inline.
>
> . update nfsd_breaker_owns_lease to use dl->dl_stid.sc_client directly
>
> . fix kernel test robot warning when CONFIG_FILE_LOCKING not defined.
>
> v18:
>
> . modify 0005-NFSD-Update-nfs4_get_vfs_file-to-handle-courtesy-cli.patch to:
>
> . remove nfs4_check_access_deny_bmap, fold this functionality
> into nfs4_resolve_deny_conflicts_locked by making use of
> bmap_to_share_mode.
>
> . move nfs4_resolve_deny_conflicts_locked into nfs4_file_get_access
> and nfs4_file_check_deny.
>
> v19:
>
> . modify 0002-NFSD-Add-courtesy-client-state-macro-and-spinlock-to.patch to
>
> . add NFSD4_CLIENT_ACTIVE
>
> . redo Courtesy client state table
>
> . modify 0007-NFSD-Update-find_in_sessionid_hashtbl-to-handle-cour.patch and
> 0008-NFSD-Update-find_client_in_id_table-to-handle-courte.patch to:
>
> . set cl_cs_client_stare to NFSD4_CLIENT_ACTIVE when reactive
> courtesy client
>
> v20:
>
> . modify 0006-NFSD-Update-find_clp_in_name_tree-to-handle-courtesy.patch to:
> . add nfsd4_discard_reconnect_clnt
> . replace call to nfsd4_discard_courtesy_clnt with
> nfsd4_discard_reconnect_clnt
>
> . modify 0007-NFSD-Update-find_in_sessionid_hashtbl-to-handle-cour.patch to:
> . replace call to nfsd4_discard_courtesy_clnt with
> nfsd4_discard_reconnect_clnt
>
> . modify 0008-NFSD-Update-find_client_in_id_table-to-handle-courte.patch
> . replace call to nfsd4_discard_courtesy_clnt with
> nfsd4_discard_reconnect_clnt
>
> v21:
>
> . merged with 5.18.0-rc3
>
> . Redo based on Bruce's suggestion by breaking the patches into functionality
> and also don't remove client record of courtesy client until the client is
> actually expired.
>
> 0001: courteous server framework with support for client with delegation only.
> This patch also handles COURTESY and EXPIRABLE reconnect.
> Conflict is resolved by set the courtesy client to EXPIRABLE, let the
> laundromat expires the client on next run and return NFS4ERR_DELAY
> OPEN request.
>
> 0002: add support for opens/share reservation to courteous server
> Conflict is resolved by set the courtesy client to EXPIRABLE, let the
> laundromat expires the client on next run and return NFS4ERR_DELAY
> OPEN request.
>
> 0003: mv creation/destroying laundromat workqueue from nfs4_state_start and
> and nfs4_state_shutdown_net to init_nfsd and exit_nfsd.
>
> 0004: fs/lock: add locks_owner_has_blockers helper
>
> 0005: add 2 callbacks to lock_manager_operations for resolving lock conflict
>
> 0006: add support for locks to courteous server, making use of 0004 and 0005
> Conflict is resolved by set the courtesy client to EXPIRABLE, run the
> laundromat immediately and wait for it to complete before returning to
> fs/lock code to recheck the lock list from the beginning.
>
> NOTE: I could not get queue_work/queue_delay_work and flush_workqueue
> to work as expected, I have to use mod_delayed_work and flush_workqueue
> to get the laundromat to run immediately.
>
> When we check for blockers in nfs4_anylock_blockers, we do not check
> for client with delegation conflict. This is because we already hold
> the client_lock and to check for delegation conflict we need the state_lock
> and scanning the del_recall_lru list each time. So to avoid this overhead
> and potential deadlock (not sure about lock of ordering of these locks)
> we check and set the COURTESY client with delegation being recalled to
> EXPIRABLE later in nfs4_laundromat.
>
> 0007: show state of courtesy client in client info.
>
> v22:
>
> . modify 0001:
> . allow EXPIRABLE client to reconnect.
> . modify try_to_expire_client to return false if cl_state is
> either COURTEY or EXPIRABLE.
> . remove try_to_activate_client and set cl_state to ACTIVE in
> get_client_locked and renew_client_locked.
> . remove unnecessary cl_cs_lock. Synchronization between expiring
> client and client reconnect is provided by mark_client_expired_locked
> and get_client_locked or renew_client_locked
>
> . modify 0003:
> . fix 'ld' error with laundry_wq when CONFIG_NFSD is defined
> and CONFIG_NFSD_V4 is not defined.
>
> v23:
> . rework try_to_expire_client to return true when cl_state in EXPIRABLE
> and its callers to work accordingly.
>
> . add missing mod_delay_work in nfsd4_lm_lock_expirable.
>
> . add check for cl_rpc_users before setting client state to COURTESY
> in nfs4_get_client_reaplist.
>
> . setting client to COURTESY before nfs4_anylock_blockers to handle
> race between the laundromat and thread resolving lock conflict.
>
> . cleanup 2 fs/lock callbacks: lm_lock_expirable to return bool and
> lm_expire_lock takes no argument.
> v24:
> . add new counter, cl_delegs_in_recall, in nfs4_client to maintain
> delegation recalls and is checked by nfs4_anylock_blockers.
>
> . remove resolve_lock_conflict_locked and move its logic into the
> callers posix_lock_inode and posix_test_lock for clarity.
>
> . rename 'conflict' to 'resolvable' in nfs4_resolve_deny_conflicts_locked.
>
> . fix kernel robot test warning about missing semicolon in nfsd.h
>