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():

quoted

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/4485C3FE.5070504@redhat.com/ (local)

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