All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Benjamin Coddington" <bcodding@redhat.com>
To: "Olga Kornievskaia" <aglo@umich.edu>
Cc: "Trond Myklebust" <trond.myklebust@primarydata.com>,
	linux-nfs <linux-nfs@vger.kernel.org>,
	"Jeff Layton" <jlayton@redhat.com>
Subject: Re: [PATCH 1/2] NFSv4: Fix CLOSE races with OPEN
Date: Wed, 14 Feb 2018 10:05:25 -0500	[thread overview]
Message-ID: <698A2B0C-1E38-4B65-ABA3-60C396393E30@redhat.com> (raw)
In-Reply-To: <CAN-5tyGEn3vGJicR+00ATSp7PzUEm_6FzjBwYwiS-++OcqqfaA@mail.gmail.com>

Hi Olga,

On 13 Feb 2018, at 15:08, Olga Kornievskaia wrote:

> On Mon, Nov 14, 2016 at 11:19 AM, Trond Myklebust
> <trond.myklebust@primarydata.com> wrote:
>> If the reply to a successful CLOSE call races with an OPEN to the same
>> file, we can end up scribbling over the stateid that represents the
>> new open state.
>> The race looks like:
>>
>>   Client                                Server
>>   ======                                ======
>>
>>   CLOSE stateid A on file "foo"
>>                                         CLOSE stateid A, return stateid C
>
> Hi folks,
>
> I'd like to understand this particular issue. Specifically I don't
> understand how can server return stateid C to the close with stateid
> A.

I think in this explanation of the race, stateid C is an incremented version
of A -- the stateid's "other" fields match -- OR it is the invalid special
stateid according to RFC 5661, Section 18.2.4.

> As per RFC 7530 or 5661. It says that state is returned by the close
> shouldn't be used.
>
> Even though CLOSE returns a stateid, this stateid is not useful to
>    the client and should be treated as deprecated.  CLOSE "shuts down"
>    the state associated with all OPENs for the file by a single
>    open-owner.  As noted above, CLOSE will either release all file
>    locking state or return an error.  Therefore, the stateid returned by
>    CLOSE is not useful for the operations that follow.
>
> Is this because the spec says "should" and not a "must"?

I don't understand - is what because?  The state returned from CLOSE is
useful to be used by the client for housekeeping, but it won't be re-used in
the protocol.

> Linux server increments a state's sequenceid on CLOSE. Ontap server
> does not. I'm not sure what other servers do. Are all these
> implementations equality correct?

Ah, good question there..  Even though the stateid is not useful for
operations that follow, I think the sequenceid should be incremented because
the CLOSE is an operation that modifies the set of locks or state:

In RFC 7530, Section 9.1.4.2.:
...
   When such a set of locks is first created, the server returns a
   stateid with a seqid value of one.  On subsequent operations that
   modify the set of locks, the server is required to advance the
   seqid field by one whenever it returns a stateid for the same
   state-owner/file/type combination and the operation is one that might
   make some change in the set of locks actually designated.  In this
   case, the server will return a stateid with an "other" field the same
   as previously used for that state-owner/file/type combination, with
   an incremented seqid field.

But, in RFC 5661, the CLOSE response SHOULD be the invalid special stateid.
I don't recall, does the linux server increment the existing stateid or send
back the invalid special stateid on v4.1?

Ben

  reply	other threads:[~2018-02-14 15:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-14 16:19 [PATCH 0/2] Fix CLOSE races Trond Myklebust
2016-11-14 16:19 ` [PATCH 1/2] NFSv4: Fix CLOSE races with OPEN Trond Myklebust
2016-11-14 16:19   ` [PATCH 2/2] NFSv4: Don't call close if the open stateid has already been cleared Trond Myklebust
2016-11-14 16:40   ` [PATCH 1/2] NFSv4: Fix CLOSE races with OPEN Jeff Layton
2016-11-14 16:48     ` Trond Myklebust
2018-02-13 20:08   ` Olga Kornievskaia
2018-02-14 15:05     ` Benjamin Coddington [this message]
2018-02-14 15:21       ` Jeff Layton
2018-02-14 15:29         ` Trond Myklebust
2018-02-14 15:42           ` Benjamin Coddington
2018-02-14 16:06             ` Olga Kornievskaia
2018-02-14 16:59               ` Trond Myklebust
2018-02-14 22:17                 ` Olga Kornievskaia
2018-02-15 12:19     ` Mkrtchyan, Tigran
2018-02-15 12:23       ` Jeff Layton
2018-02-15 15:29         ` Bruce Fields
2018-02-15 15:37           ` Trond Myklebust

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=698A2B0C-1E38-4B65-ABA3-60C396393E30@redhat.com \
    --to=bcodding@redhat.com \
    --cc=aglo@umich.edu \
    --cc=jlayton@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.com \
    /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.