Bug #41192
closedmds: atime not being updated persistently
0%
Description
xfstest generic/192 fails with kcephfs. It basically:
mounts the fs
creates a file and records the atime
waits a few seconds and then reads the file
fetches the atime again
unmounts and remounts the fs
fetches the atime again and does some comparison
What I see happening is that the atime changes after the read as expected, but after the mount and remount cycle, it's back to its original value.
While I have not been able to confirm it yet, I suspect that this is an MDS bug. _update_cap_fields() does this:
if ((dirty & CEPH_CAP_FILE_EXCL) && atime != pi->atime) {
dout(7) << " atime " << pi->atime << " -> " << atime
<< " for " << *in << dendl;
pi->atime = atime;
}
It seems like that shouldn't depend on Fx caps, but also Fr?
Updated by Jeff Layton over 4 years ago
Tracepoints from adding and removing caps for the inode:
kworker/11:1-4503 [011] ...2 12397.551185: ceph_add_cap: ino=NOSNAP:0x10000146930 mds=0 issued=pAsLsXsFscr implemented=pAsLsXsFscr mds_wanted=pFscr
umount-4769 [015] ...2 12427.410308: ceph_remove_cap: ino=NOSNAP:0x10000146930 mds=0 issued=pAsLsXsFscr implemented=pAsLsXsFscr mds_wanted=pFscr
It looks like we are flushing these back to the server, it's just that the MDS seems to ignore the atime update when we send them back.
Updated by Jeff Layton over 4 years ago
- Assignee set to Jeff Layton
Now that I've done a bit more investigation, I think there are actually two parts to this bug:
1) the MDS only updates the atime when Fx caps are flushed. It should be doing that anytime that Fx is flushed or when Fr is flushed and the atime is newer than the one the MDS has. I have a patch that I think will fix this.
2) the kernel doesn't seem to mark Fr caps dirty on read. I'm still looking at this bit.
Updated by Patrick Donnelly over 4 years ago
- Subject changed from atime not being updated persistently to mds: atime not being updated persistently
- Category set to Correctness/Safety
- Status changed from New to Fix Under Review
- Target version set to v15.0.0
- Start date deleted (
08/09/2019) - Source set to Development
- Backport set to nautilus,mimic
- Pull request ID set to 29582
- Component(FS) MDS added
Updated by Jeff Layton over 4 years ago
- Status changed from Fix Under Review to In Progress
I've dropped the PR for now, as Zheng pointed out that atime is not actually tied to Fr caps after all, but rather to Fwx caps, and if you don't have one of those, then you need to do a SETATTR to update the atime.
I think it'd make a lot more sense to tie it Fr caps, but Zheng has implied that that wouldn't be simple. For now, I'm still considering our options to fix this.
In practice, I'm not too worried about leaving this broken as not many apps use the atime. If we can fix it for those who do without adding too much overhead, then fine. Otherwise, we may just have to live with it.
Updated by Jeff Layton almost 2 years ago
- Status changed from In Progress to Won't Fix
I don't see us fixing this in order to get local atime semantics. Closing WONTFIX.