On Tue, 2018-09-11 at 16:23 -0400, schumaker.anna@xxxxxxxxx wrote:
> From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
>
> Moving all of this into a new function removes the need for cramped
> indentation, making the code overall easier to look at. I also take
> this chance to switch copy recovery over to using
> nfs4_stateid_match_other()
>
> Signed-off-by: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
> ---
> fs/nfs/nfs4state.c | 83 ++++++++++++++++++++++++++----------------
> ----
> 1 file changed, 47 insertions(+), 36 deletions(-)
>
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 3df0eb52da1c..aa46a3216d60 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1547,10 +1547,51 @@ static int nfs4_reclaim_locks(struct
> nfs4_state *state, const struct nfs4_state_
> return status;
> }
>
> +static int __nfs4_reclaim_open_state(struct nfs4_state_owner *sp,
> struct nfs4_state *state,
> + const struct
> nfs4_state_recovery_ops *ops)
> +{
> + struct nfs4_lock_state *lock;
> + int status;
> +
> + status = ops->recover_open(sp, state);
> + if (status < 0)
> + return status;
> +
> + status = nfs4_reclaim_locks(state, ops);
> + if (status < 0)
> + return status;
> +
> + if (!test_bit(NFS_DELEGATED_STATE, &state->flags)) {
> + spin_lock(&state->state_lock);
> + list_for_each_entry(lock, &state->lock_states,
> ls_locks) {
> + if (!test_bit(NFS_LOCK_INITIALIZED, &lock-
> >ls_flags))
> + pr_warn_ratelimited("NFS: %s: Lock
> reclaim failed!\n", __func__);
> + }
> + spin_unlock(&state->state_lock);
> + }
> +
> +#ifdef CONFIG_NFS_V4_2
> + if (test_bit(NFS_CLNT_DST_SSC_COPY_STATE, &state->flags)) {
> + struct nfs4_copy_state *copy;
> + spin_lock(&sp->so_server->nfs_client->cl_lock);
> + list_for_each_entry(copy, &sp->so_server->ss_copies,
> copies) {
> + if (nfs4_stateid_match_other(&state->stateid,
> ©->parent_state->stateid))
> + continue;
> + copy->flags = 1;
> + complete(©->completion);
> + break;
> + }
> + spin_unlock(&sp->so_server->nfs_client->cl_lock);
> + }
> +#endif /* CONFIG_NFS_V4_2 */
Applied, but could you please follow up with a separate patch that
moves this #ifdef section into a separate function? That would help
with readability of the code.
> +
> + clear_bit(NFS_STATE_RECLAIM_NOGRACE, &state->flags);
> + return status;
> +}
> +
> static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp,
> const struct nfs4_state_recovery_ops *ops)
> {
> struct nfs4_state *state;
> - struct nfs4_lock_state *lock;
> int status = 0;
>
> /* Note: we rely on the sp->so_states list being ordered
> @@ -1573,43 +1614,13 @@ static int nfs4_reclaim_open_state(struct
> nfs4_state_owner *sp, const struct nfs
> continue;
> atomic_inc(&state->count);
> spin_unlock(&sp->so_lock);
> - status = ops->recover_open(sp, state);
> + status = __nfs4_reclaim_open_state(sp, state, ops);
> if (status >= 0) {
> - status = nfs4_reclaim_locks(state, ops);
> - if (status >= 0) {
> - if (!test_bit(NFS_DELEGATED_STATE,
> &state->flags)) {
> - spin_lock(&state->state_lock);
> - list_for_each_entry(lock,
> &state->lock_states, ls_locks) {
> - if
> (!test_bit(NFS_LOCK_INITIALIZED, &lock->ls_flags))
> - pr_warn_ratelim
> ited("NFS: "
> -
> "%s: Lock reclaim "
> -
> "failed!\n", __func__);
> - }
> - spin_unlock(&state-
> >state_lock);
> - }
> - clear_bit(NFS_STATE_RECLAIM_NOGRACE,
> - &state->flags);
> -#ifdef CONFIG_NFS_V4_2
> - if
> (test_bit(NFS_CLNT_DST_SSC_COPY_STATE, &state->flags)) {
> - struct nfs4_copy_state *copy;
> -
> - spin_lock(&sp->so_server-
> >nfs_client->cl_lock);
> - list_for_each_entry(copy, &sp-
> >so_server->ss_copies, copies) {
> - if (memcmp(&state-
> >stateid.other, ©->parent_state->stateid.other,
> NFS4_STATEID_SIZE))
> - continue;
> - copy->flags = 1;
> - complete(©-
> >completion);
> - printk("AGLO: server
> rebooted waking up the copy\n");
> - break;
> - }
> - spin_unlock(&sp->so_server-
> >nfs_client->cl_lock);
> - }
> -#endif /* CONFIG_NFS_V4_2 */
> - nfs4_put_open_state(state);
> - spin_lock(&sp->so_lock);
> - goto restart;
> - }
> + nfs4_put_open_state(state);
> + spin_lock(&sp->so_lock);
> + goto restart;
> }
> +
> switch (status) {
> default:
> printk(KERN_ERR "NFS: %s: unhandled
> error %d\n",
--
Trond Myklebust
CTO, Hammerspace Inc
4300 El Camino Real, Suite 105
Los Altos, CA 94022
www.hammer.space
