Re: [PATCH v2 4/4] NFSD: handle GETATTR conflict with write delegation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 5/15/23 5:06 PM, Chuck Lever III wrote:

On May 15, 2023, at 6:53 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:

On Mon, 2023-05-15 at 21:37 +0000, Chuck Lever III wrote:
On May 15, 2023, at 4:21 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:

On Mon, 2023-05-15 at 16:10 -0400, Olga Kornievskaia wrote:
On Mon, May 15, 2023 at 2:58 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
On Mon, 2023-05-15 at 11:26 -0700, dai.ngo@xxxxxxxxxx wrote:
On 5/15/23 11:14 AM, Olga Kornievskaia wrote:
On Sun, May 14, 2023 at 8:56 PM Dai Ngo <dai.ngo@xxxxxxxxxx> wrote:
If the GETATTR request on a file that has write delegation in effect
and the request attributes include the change info and size attribute
then the request is handled as below:

Server sends CB_GETATTR to client to get the latest change info and file
size. If these values are the same as the server's cached values then
the GETATTR proceeds as normal.

If either the change info or file size is different from the server's
cached values, or the file was already marked as modified, then:

   . update time_modify and time_metadata into file's metadata
     with current time

   . encode GETATTR as normal except the file size is encoded with
     the value returned from CB_GETATTR

   . mark the file as modified

If the CB_GETATTR fails for any reasons, the delegation is recalled
and NFS4ERR_DELAY is returned for the GETATTR.
Hi Dai,

I'm curious what does the server gain by implementing handling of
GETATTR with delegations? As far as I can tell it is not strictly
required by the RFC(s). When the file is being written any attempt at
querying its attribute is immediately stale.
Yes, you're right that handling of GETATTR with delegations is not
required by the spec. The only benefit I see is that the server
provides a more accurate state of the file as whether the file has
been changed/updated since the client's last GETATTR. This allows
the app on the client to take appropriate action (whatever that
might be) when sharing files among multiple clients.



 From RFC 8881 10.4.3:

"It should be noted that the server is under no obligation to use
CB_GETATTR, and therefore the server MAY simply recall the delegation to
avoid its use."
This is a "MAY" which means the server can choose to not to and just
return the info it currently has without recalling a delegation.


That's not at all how I read that. To me, it sounds like it's saying
that the only alternative to implementing CB_GETATTR is to recall the
delegation. If that's not the case, then we should clarify that in the
spec.
The meaning of MAY is spelled out in RFC 2119. MAY does not mean
"the only alternative". I read this statement as alerting client
implementers that a compliant server is permitted to skip
CB_GETATTR, simply by recalling the delegation. Technically
speaking this compliance statement does not otherwise restrict
server behavior, though the author might have had something else
in mind.

I'm leery of the complexity that CB_GETATTR adds to NFSD and
the protocol. In addition, section 10.4 is riddled with errors,
albeit minor ones; that suggests this part of the protocol is
not well-reviewed.

It's not apparent how much gain is provided by CB_GETATTR.
IIRC NFSD can recall a delegation on the same nfsd thread as an
incoming request, so the turnaround for a recall from a local
client is going to be quick.

It would be good to know how many other server implementations
support CB_GETATTR.
I'm rather leaning towards postponing 3/4 and 4/4 and instead
taking a more incremental approach. Let's get the basic Write
delegation support in, and possibly add a counter or two to
find out how often a GETATTR on a write-delegated file results
in a delegation recall.

We can then take some time to disambiguate the spec language and
look at other implementations to see if this extra protocol is
really of value.

I think it would be good to understand whether Write delegation
without CB_GETATTR can result in a performance regression (say,
because the server is recalling delegations more often for a
given workload).
Ganesha has had write delegation and CB_GETATTR support for years.
Does OnTAP support write delegation? I heard a rumor NetApp
disabled it because of the volume of customer calls involving
delegation with the Linux client, but that could be old news.

So far I have not run into any problem of NFS client using write
delegation. My testing so far, besides manual tests, were the delegation
tests in pynfs for 4.0 and 4.1 and nfstest_delegation test.

The nfstest_delegation result is:
1785 tests (1783 passed, 2 failed)

The 2 failed test is regarding whether the NFSv4 server should allow
the client to use write delegation state to do READ.


How about Solaris? My close contact with the Solaris NFS team
as the Linux NFS client implementation matured has colored my
experience with write delegation. It is complex and subtle.

Solaris has write delegation support for years.



Isn't CB_GETATTR the main benefit of a write delegation in the first
place? A write deleg doesn't really give any benefit otherwise, as you
can buffer writes anyway without one.

AIUI, the point of a write delegation is to allow other clients (and
potentially the server) to get up to date information on file sizes and
change attr when there is a single, active writer.
The benefits of write delegation depend on the client implementation
and the workload. A client may use a write delegation to indicate
that it can handle locking requests for a file locally, for example.

This is what the Linux NFS client does, handling locking requests for a
file locally if the file has write delegation.



Without CB_GETATTR, your first stat() against the file will give you
fairly up to date info (since you'll have to recall the delegation), but
then you'll be back to the server just reporting the size and change
attr that it has at the time.
Which is the current behavior, yes? As long as behavior is not
regressing, I don't foresee a problem with doing CB_GETATTR in 6.6
or later.

I think defer the handling of GETATTR with CB_GETATTR until 6.6 is okay
so we can get some runtime with write delegation to have some confident
with its stability.

For now we just return the (potentially stale) file attribute available
at the server. It should not cause significant problem since there won't
be any real application depends on this feature yet, at least on the Linux
NFS server.

-Dai



--
Chuck Lever





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux