All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <Trond.Myklebust@netapp.com>
To: Ricardo Labiaga <Ricardo.Labiaga@netapp.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 4/5] nfs41: New NFS4CLNT_RECLAIM_COMPLETE_PENDING state
Date: Mon, 07 Dec 2009 09:47:51 -0500	[thread overview]
Message-ID: <1260197272.32136.19.camel@localhost> (raw)
In-Reply-To: <1260174111-23160-5-git-send-email-Ricardo.Labiaga@netapp.com>

On Mon, 2009-12-07 at 00:21 -0800, Ricardo Labiaga wrote: 
> nfs4_state_end_reclaim_reboot() can also be invoked as a
> result of error processing, so it is not a safe place to
> invoke RECLAIM_COMPLETE.  Instead, create a new state
> flag that tracks the fact that a RECLAIM_COMPLETE needs
> to be issued when all state has been reclaimed, or when
> we're done establishing the session for the first time.
> 
> If an error occurs in the main state manager loop, just clear the
> flag.  No sense in checking if the flag is set in order to clear it.
> We're not going to issue the RECLAIM_COMPLETE since there's a high
> probability that we had some kind of communication or session problem
> which is s how we ended up in the error case.

This patch looks wrong for two reasons.

     1. We only want to call RECLAIM_COMPLETE if we saw a STALE_CLIENTID
        error prior to the last attempt to re-establish the client id. 
     2. We have to call it before we can start the no-grace reclaims.

Looking at the code, I'm not convinced that we need a separate
'RECLAIM_COMPLETE_PENDING' state. It should be pretty much identical to
the existing NFS4CLNT_RECLAIM_REBOOT state.
The only difference is that in the NFSv4.1 case we want to be able to
call RECLAIM_COMPLETE even in the case where we have no state to
reclaim.

Cheers
  Trond

  parent reply	other threads:[~2009-12-07 14:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-07  8:21 [PATCH 0/5] Reclaim Stage Bug Fixes Version 2 Ricardo Labiaga
2009-12-07  8:21 ` [PATCH 1/5] nfs41: Mark stateids in need of reclaim if state manager gets stale clientid Ricardo Labiaga
2009-12-07  8:21   ` [PATCH 2/5] nfs41: Handle session errors during delegation return Ricardo Labiaga
2009-12-07  8:21     ` [PATCH 3/5] nfs41: Retry delegation return if it failed with session error Ricardo Labiaga
2009-12-07  8:21       ` [PATCH 4/5] nfs41: New NFS4CLNT_RECLAIM_COMPLETE_PENDING state Ricardo Labiaga
2009-12-07  8:21         ` [PATCH 5/5] nfs41: Handle NFSv4.1 session errors in the delegation recall code Ricardo Labiaga
2009-12-07 14:47         ` Trond Myklebust [this message]
2009-12-07 17:51           ` [PATCH 4/5] nfs41: New NFS4CLNT_RECLAIM_COMPLETE_PENDING state Labiaga, Ricardo
2009-12-07 18:11             ` Trond Myklebust
2009-12-07 21:41               ` Labiaga, Ricardo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1260197272.32136.19.camel@localhost \
    --to=trond.myklebust@netapp.com \
    --cc=Ricardo.Labiaga@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.