hi Trond,
I had a report that bash shell "truncate" redirection was not updating
the mtime of a file, if that file already existed, and its size was
already zero.
That's trivial to reproduce, here on v5.14-rc3, NFSv4.1 mount:
# date; > file1
Tue 10 Aug 14:41:08 PDT 2021
# ls -l file1
-rw-r--r-- 1 root root 0 Aug 10 14:41 file1
# date; > file1
Tue 10 Aug 14:43:06 PDT 2021
# ls -l file1
-rw-r--r-- 1 root root 0 Aug 10 14:41 file1
An strace (of the second, above) shows that the bash shell is using
open(O_TRUNC):
10581 14:52:36.965048 open("file1", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
<0.012124>
and a pcap shows that the client is sending an OPEN(OPEN4_NOCREATE),
then a CLOSE, but no SETATTR(size=0).
I think this might be because of this optimisation, in the inode setattr
op nfs_setattr():
if (attr->ia_valid & ATTR_SIZE) {
…
if (attr->ia_size == i_size_read(inode))
attr->ia_valid &= ~ATTR_SIZE;
}
/* Optimization: if the end result is no change, don't RPC */
if (((attr->ia_valid & NFS_VALID_ATTRS) & ~(ATTR_FILE|ATTR_OPEN)) == 0)
return 0;
and, indeed, there is no change here: the file already exists, and its
size doesn't change.
However, POSIX says, for open():
> If O_TRUNC is set and the file did previously exist, upon successful
> completion, open() shall mark for update the last data modification and
> last file status change timestamps of the file.
[https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html]
which suggest that this optimisation may not be valid, in this case.
[there's a similar issue perhaps for ftruncate() where the size doesn't
change]
I haven't yet worked out whether in this case it's deliberate, but not
POSIX compliant, or accidental.
I'll continue looking, and come up with a patch if one is needed, but
would appreciate any comments?
thanks very much,
cheers,
calum.
and I forgot to add…
On 11/08/2021 12:36 am, Calum Mackay wrote:
> hi Trond,
>
> I had a report that bash shell "truncate" redirection was not updating
> the mtime of a file, if that file already existed, and its size was
> already zero.
>
> That's trivial to reproduce, here on v5.14-rc3, NFSv4.1 mount:
>
> # date; > file1
> Tue 10 Aug 14:41:08 PDT 2021
> # ls -l file1
> -rw-r--r-- 1 root root 0 Aug 10 14:41 file1
>
> # date; > file1
> Tue 10 Aug 14:43:06 PDT 2021
> # ls -l file1
> -rw-r--r-- 1 root root 0 Aug 10 14:41 file1
>
>
> An strace (of the second, above) shows that the bash shell is using
> open(O_TRUNC):
>
> 10581 14:52:36.965048 open("file1", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
> <0.012124>
>
> and a pcap shows that the client is sending an OPEN(OPEN4_NOCREATE),
> then a CLOSE, but no SETATTR(size=0).
>
>
> I think this might be because of this optimisation, in the inode setattr
> op nfs_setattr():
>
> if (attr->ia_valid & ATTR_SIZE) {
>
> …
>
> if (attr->ia_size == i_size_read(inode))
> attr->ia_valid &= ~ATTR_SIZE;
> }
>
> /* Optimization: if the end result is no change, don't RPC */
> if (((attr->ia_valid & NFS_VALID_ATTRS) & ~(ATTR_FILE|ATTR_OPEN))
> == 0)
> return 0;
>
> and, indeed, there is no change here: the file already exists, and its
> size doesn't change.
>
> However, POSIX says, for open():
>
>> If O_TRUNC is set and the file did previously exist, upon successful
>> completion, open() shall mark for update the last data modification and
>> last file status change timestamps of the file.
>
> [https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html]
>
> which suggest that this optimisation may not be valid, in this case.
This has been discussed before, way back in 2006:
https://lore.kernel.org/linux-nfs/[email protected]/
but in relation to SuSv43, where a similar clause did seem to include
"if and only if the file size changes" which isn't in the POSIX IEEE Std
1003.1-2017 I quoted above.
So, I take it that this is still intentional?
thanks,
calum.
>
> [there's a similar issue perhaps for ftruncate() where the size doesn't
> change]
>
>
> I haven't yet worked out whether in this case it's deliberate, but not
> POSIX compliant, or accidental.
>
> I'll continue looking, and come up with a patch if one is needed, but
> would appreciate any comments?
>
>
> thanks very much,
>
> cheers,
> calum.
>
--
Calum Mackay
Linux Kernel Engineering
Oracle Linux and Virtualisation
On Wed, 2021-08-11 at 00:36 +0100, Calum Mackay wrote:
> hi Trond,
>
> I had a report that bash shell "truncate" redirection was not updating
> the mtime of a file, if that file already existed, and its size was
> already zero.
>
> That's trivial to reproduce, here on v5.14-rc3, NFSv4.1 mount:
>
> # date; > file1
> Tue 10 Aug 14:41:08 PDT 2021
> # ls -l file1
> -rw-r--r-- 1 root root 0 Aug 10 14:41 file1
>
> # date; > file1
> Tue 10 Aug 14:43:06 PDT 2021
> # ls -l file1
> -rw-r--r-- 1 root root 0 Aug 10 14:41 file1
>
>
> An strace (of the second, above) shows that the bash shell is using
> open(O_TRUNC):
>
> 10581 14:52:36.965048 open("file1", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
> <0.012124>
>
> and a pcap shows that the client is sending an OPEN(OPEN4_NOCREATE),
> then a CLOSE, but no SETATTR(size=0).
>
>
> I think this might be because of this optimisation, in the inode
> setattr
> op nfs_setattr():
>
> if (attr->ia_valid & ATTR_SIZE) {
>
> …
>
> if (attr->ia_size == i_size_read(inode))
> attr->ia_valid &= ~ATTR_SIZE;
> }
>
> /* Optimization: if the end result is no change, don't RPC */
> if (((attr->ia_valid & NFS_VALID_ATTRS) &
> ~(ATTR_FILE|ATTR_OPEN)) == 0)
> return 0;
>
> and, indeed, there is no change here: the file already exists, and its
> size doesn't change.
>
> However, POSIX says, for open():
>
> > If O_TRUNC is set and the file did previously exist, upon successful
> > completion, open() shall mark for update the last data modification
> > and
> > last file status change timestamps of the file.
>
> [https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html]
>
> which suggest that this optimisation may not be valid, in this case.
>
> [there's a similar issue perhaps for ftruncate() where the size doesn't
> change]
>
>
> I haven't yet worked out whether in this case it's deliberate, but not
> POSIX compliant, or accidental.
>
> would appreciate any comments?
>
I vaguely recall that the Open Group has been rather wishy washy about
this behaviour previously.
<looks\>
Yep, for some versions of the Single Unix Spec, the current NFS client
behaviour was mandated. For instance
https://pubs.opengroup.org/onlinepubs/009695399/functions/truncate.html
states that the c/mtime change if and only if the file size changed.
IOW: The problem here is that POSIX, and the earlier SUS have diverged.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
On Wed, 2021-08-11 at 00:52 +0100, Calum Mackay wrote:
> and I forgot to add…
>
> On 11/08/2021 12:36 am, Calum Mackay wrote:
> > hi Trond,
> >
> > I had a report that bash shell "truncate" redirection was not
> > updating
> > already zero.
> >
> > That's trivial to reproduce, here on v5.14-rc3, NFSv4.1 mount:
> >
> > # date; > file1
> > Tue 10 Aug 14:41:08 PDT 2021
> > # ls -l file1
> > -rw-r--r-- 1 root root 0 Aug 10 14:41 file1
> >
> > # date; > file1
> > Tue 10 Aug 14:43:06 PDT 2021
> > # ls -l file1
> > -rw-r--r-- 1 root root 0 Aug 10 14:41 file1
> >
> >
> > open(O_TRUNC):
> >
> > 10581 14:52:36.965048 open("file1", O_WRONLY|O_CREAT|O_TRUNC, 0666)
> > = 3
> > <0.012124>
> >
> > and a pcap shows that the client is sending an
> > OPEN(OPEN4_NOCREATE),
> > then a CLOSE, but no SETATTR(size=0).
> >
> >
> > I think this might be because of this optimisation, in the inode
> > setattr
> > op nfs_setattr():
> >
> > if (attr->ia_valid & ATTR_SIZE) {
> >
> > …
> >
> > if (attr->ia_size == i_size_read(inode))
> > attr->ia_valid &= ~ATTR_SIZE;
> > }
> >
> > /* Optimization: if the end result is no change, don't RPC */
> > if (((attr->ia_valid & NFS_VALID_ATTRS) &
> > ~(ATTR_FILE|ATTR_OPEN))
> > == 0)
> > return 0;
> >
> > and, indeed, there is no change here: the file already exists, and
> > its
> > size doesn't change.
> >
> > However, POSIX says, for open():
> >
> > > If O_TRUNC is set and the file did previously exist, upon
> > > successful
> > > completion, open() shall mark for update the last data
> > > modification and
> > > last file status change timestamps of the file.
> >
> > [
> > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
> > ]
> >
> > which suggest that this optimisation may not be valid, in this
> > case.
>
> This has been discussed before, way back in 2006:
>
>
> https://lore.kernel.org/linux-nfs/[email protected]/
>
> "if and only if the file size changes" which isn't in the POSIX IEEE
> Std
> 1003.1-2017 I quoted above.
>
>
> So, I take it that this is still intentional?
>
Sort of...
We implemented the current functionality as an optimisation back when
the SUS was the most up to date authority. We could undo that today in
order to be POSIX compatible, but any change we make on the client will
rely on the server implementing the POSIX and not the SUS semantics.
IOW: in doing so, we'd be moving the problem to the server.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
On 11/08/2021 1:01 am, Trond Myklebust wrote:
> On Wed, 2021-08-11 at 00:52 +0100, Calum Mackay wrote:
>> and I forgot to add…
>>
>> On 11/08/2021 12:36 am, Calum Mackay wrote:
>>> hi Trond,
>>>
>>> I had a report that bash shell "truncate" redirection was not
>>> updating
>>> already zero.
>>>
>>> That's trivial to reproduce, here on v5.14-rc3, NFSv4.1 mount:
>>>
>>> # date; > file1
>>> Tue 10 Aug 14:41:08 PDT 2021
>>> # ls -l file1
>>> -rw-r--r-- 1 root root 0 Aug 10 14:41 file1
>>>
>>> # date; > file1
>>> Tue 10 Aug 14:43:06 PDT 2021
>>> # ls -l file1
>>> -rw-r--r-- 1 root root 0 Aug 10 14:41 file1
>>>
>>>
>>> open(O_TRUNC):
>>>
>>> 10581 14:52:36.965048 open("file1", O_WRONLY|O_CREAT|O_TRUNC, 0666)
>>> = 3
>>> <0.012124>
>>>
>>> and a pcap shows that the client is sending an
>>> OPEN(OPEN4_NOCREATE),
>>> then a CLOSE, but no SETATTR(size=0).
>>>
>>>
>>> I think this might be because of this optimisation, in the inode
>>> setattr
>>> op nfs_setattr():
>>>
>>> if (attr->ia_valid & ATTR_SIZE) {
>>>
>>> …
>>>
>>> if (attr->ia_size == i_size_read(inode))
>>> attr->ia_valid &= ~ATTR_SIZE;
>>> }
>>>
>>> /* Optimization: if the end result is no change, don't RPC */
>>> if (((attr->ia_valid & NFS_VALID_ATTRS) &
>>> ~(ATTR_FILE|ATTR_OPEN))
>>> == 0)
>>> return 0;
>>>
>>> and, indeed, there is no change here: the file already exists, and
>>> its
>>> size doesn't change.
>>>
>>> However, POSIX says, for open():
>>>
>>>> If O_TRUNC is set and the file did previously exist, upon
>>>> successful
>>>> completion, open() shall mark for update the last data
>>>> modification and
>>>> last file status change timestamps of the file.
>>>
>>> [
>>> https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
>>> ]
>>>
>>> which suggest that this optimisation may not be valid, in this
>>> case.
>>
>> This has been discussed before, way back in 2006:
>>
>>
>> https://lore.kernel.org/linux-nfs/[email protected]/
>>
>> "if and only if the file size changes" which isn't in the POSIX IEEE
>> Std
>> 1003.1-2017 I quoted above.
>>
>>
>> So, I take it that this is still intentional?
>>
>
> Sort of...
>
> We implemented the current functionality as an optimisation back when
> the SUS was the most up to date authority. We could undo that today in
> order to be POSIX compatible, but any change we make on the client will
> rely on the server implementing the POSIX and not the SUS semantics.
>
> IOW: in doing so, we'd be moving the problem to the server.
Quite.
As an added dimension, I note that with NFSv3, it /does/ update the
mtime [same client, same (Solaris) server], via a SETATTR(mtime: set to
server time; size: no value).
Indeed, this was how my reporter noticed: they moved from NFSv3 to
NFSv4, and their scripts started producing different results.
Presumably the other problem with changing v4 to match POSIX, now, is
then upsetting all the people who are used to the current SUS behaviour.
thanks,
calum.