On Tue, 2025-07-22 at 16:16 -0400, Trond Myklebust wrote: > 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. Ok, so you interpret "original time" as the timestamp before there was a delegation, not the "original" timestamp on the file just before the SETATTR? That's not exactly clear in my reading of the RFC, but that does make a bit more sense. > 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. > Yeah, that's the ugly bit. I could store the original times in the delegation, but nfsd doesn't have a way to store per-delegation persistent state. I guess we could flag the delegation after a reclaim (like you suggest), and clear it on the next WRITE. If the client just sends a SETATTR after reclaiming, then I guess we could allow for a window of 5-10s or so before the current timestamp on the file? Or maybe we could just trust the client in that case. I'll have to think about how to implement that. > > > > > > 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? -- Jeff Layton <jlayton@xxxxxxxxxx>
