On Tue, 2025-07-22 at 16:03 -0400, Jeff Layton wrote: > On Tue, 2025-07-22 at 15:51 -0400, Trond Myklebust wrote: > > On Tue, 2025-07-22 at 15:27 -0400, Jeff Layton wrote: > > > I've been chasing some problems with the git regression testsuite > > > that > > > crop up with delegated timestamps enabled. I've knocked down a > > > couple > > > of problems on the server side, but I'm not sure how to fix the > > > latest > > > issue. > > > > > > Most of the problems with gitr suite and delegated timestamps > > > manifest > > > as spurious changes to the timestamps. e.g., it will do a git > > > checkout > > > and then later find that some file in the checkout appears to > > > have > > > changed when it didn't expect that. > > > > > > I reproduced one of the problems with some debugging turned up. > > > What > > > we > > > see is this in the wireshark capture (filtered on the > > > filehandle): > > > > > > 939238 1753209666.985827 192.168.122.151 → 192.168.122.103 NFS > > > 486 V4 > > > Reply (Call In 939237) OPEN StateID: 0xafa9 > > > 939239 1753209666.987808 192.168.122.103 → 192.168.122.151 NFS > > > 298 V4 > > > Call SETATTR FH: 0x68fcd843 > > > 939240 1753209666.995860 192.168.122.151 → 192.168.122.103 NFS > > > 294 V4 > > > Reply (Call In 939239) SETATTR > > > 939241 1753209666.999909 192.168.122.103 → 192.168.122.151 NFS > > > 278 V4 > > > Call WRITE StateID: 0xe7e8 Offset: 0 Len: 2 > > > 939242 1753209667.019570 192.168.122.151 → 192.168.122.103 NFS > > > 182 V4 > > > Reply (Call In 939241) WRITE > > > 944922 1753209696.313938 192.168.122.103 → 192.168.122.151 NFS > > > 1514 > > > V4 Call SETATTR FH: 0xb6dd63b6 | DELEGRETURN StateID: 0x3eebV4 > > > Call > > > SETATTR FH: 0xcf57bbcb | DELEGRETURN StateID: 0x69ca ; V4 Call > > > SETATTR FH: 0x68fcd843 | DELEGRETURN StateID: 0xe245 ; V4 Call > > > SETATTR FH: 0x02d757ea | DELEGRETURN StateID: 0xc788 ; V4 Call > > > SETATTR FH: 0x130870b2 | DELEGRETURN StateID: 0x8c12 > > > 946410 1753209702.893917 192.168.122.103 → 192.168.122.151 NFS > > > 254 V4 > > > Call GETATTR FH: 0x68fcd843 > > > 946411 1753209702.895304 192.168.122.151 → 192.168.122.103 NFS > > > 310 V4 > > > Reply (Call In 946410) GETATTR > > > > > > We get an open for write (with no open stateid and delegated > > > timestamps), a write, and then and setattr|delegreturn. git had > > > the > > > delegated mtime (1753209666.995071118) on file because the > > > delegation > > > allowed getattr() on the client to return before writeback had > > > completed. > > > > > > In this case, the setattr for the delegated mtime was for a value > > > older > > > than the existing mtime, so it was ignored. Note that the reply > > > to > > > > Uhh... Why is the existing value of the mtime on the server grounds > > for > > rejecting the delegated mtime? The client owns that value. > > > > From RFC 9754: > > When the time presented is before the original time, then the > update > is ignored. When the time presented is in the future, the server > can > either clamp the new time to the current time or return > NFS4ERR_DELAY > to the client, allowing it to retry. > > In this case, the preceding WRITE operation from the client updated > the > mtime and ctime on the server. That operation happened after the > mtime > was updated on the client for that write. > > Are you suggesting that the server needs to "disable" mtime/ctime > updates from WRITE calls (and I guess atime updates from READs) when > there is a delegation outstanding? If so, that would potentially be > quite ugly in the face of a crash. It just needs to record what the original atime and mtime was on the file when it issued the delegation. That gets a little complicated if the server reboots, and the client has to reclaim the delegation and so you might want to give it a little more leeway in that situation. > > > > the > > > WRITE didn't go out until 1753209667.019570, which is after > > > 1753209666.995071118. > > > > > > Eventually the client gets the "real" mtime from the server after > > > returning the delegation, which now doesn't match the one git has > > > on > > > file. > > > > > > I don't see a way to fix this right offhand. Any thoughts?
