From:   "Benjamin Coddington" <bcodding@redhat.com>
To:     "Trond Myklebust" <trondmy@hammerspace.com>
Cc:     linux-nfs@vger.kernel.org, anna.schumaker@netapp.com
Subject: Re: [PATCH 2/3] NFSv42: Don't drop NFS_INO_INVALID_CHANGE if we hold
 a delegation
Date:   Tue, 16 Nov 2021 09:34:39 -0500
Message-ID: <66EE327D-C8F1-44D0-9364-573CA1208D46@redhat.com>
In-Reply-To: <879b0f03b5c3b786568aaefd26bc8c714e1d7aae.camel@hammerspace.com>
References: <cover.1637069577.git.bcodding@redhat.com>
 <c91e224b847e697e42b25cdc36cd164a61ad1ade.1637069577.git.bcodding@redhat.com>
 <a06d3d97a865747058c7d1cbcd4f70911c336fce.camel@hammerspace.com>
 <9FE34DCC-0F28-4960-B25C-B006DA6D9A38@redhat.com>
 <879b0f03b5c3b786568aaefd26bc8c714e1d7aae.camel@hammerspace.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On 16 Nov 2021, at 9:17, Trond Myklebust wrote:

> On Tue, 2021-11-16 at 09:12 -0500, Benjamin Coddington wrote:
>> On 16 Nov 2021, at 9:03, Trond Myklebust wrote:
>>> No, we really shouldn't need to care what the server thinks or
>>> does.
>>> The client is authoritative for the change attribute while it holds
>>> a
>>> delegation, not the server.
>>
>> My understanding of the intention of the code (which I had to sort of
>> put
>> together from historical patches in this area) is that we want to see
>> ctime,
>> mtime, and block size updates after copy/clone even if we hold a
>> delegation,
>> but without NFS_INO_INVALID_CHANGE, the client won't update those
>> attributes.
>>
>> If that's not necessary, we can drop this patch.
>>
>
> We will still see the ctime/mtime/block size updates even if
> NFS_INO_INVALID_CHANGE is not set. Those attributes' cache status are
> tracked separately through their own NFS_INO_INVALID_* bits.
>
> That said, there really is no reason why we shouldn't treat the copy
> and clone code exactly the same way we would treat a regular write.
> Perhaps we can fix up the arguments of nfs_writeback_update_inode() so
> that it can be called here?

I wonder if that just kicks the problem down the road to when we return the
delegation, and we'll see cases where ctime/mtime move backward.  And I
don't think we can ever be authoritative for space_used, but maybe it doesn't
matter if we're within the attribute timeouts.