Differential D57096
Close files that were opened by guest VM via 9pfs Authored by njain15_protonmail.com on Tue, May 19, 4:31 PM. Tags None Referenced Files
Details
See: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=294327 It seems this bug appears because calling close() on an open file in the guest will not send a clunk request to the P9 server. This is deferred until the vnode is reclaimed, which can be after a while. The suggested fix clunks the open FID if the calling process is the last one using it. Tested the following on bhyve and KVM by monitoring output of lsof.
#!/bin/sh for i in <some list> do find . -iregex <something here> | xargs -I{} sh -c "echo <something> >> {}" & done
Diff Detail
Event TimelineComment Actions I updated, but I'm seeing the same errors from poudriere-bulk runs: [00:00:54] Warning: (devel/air-go): make: /usr/ports/Mk/bsd.port.mk: Invalid argument [00:00:54] Warning: (devel/air-go): Error: Error looking up dependencies for devel/air-go ... [00:00:55] Warning: (devel/lua-language-server): make: /usr/ports/Mk/bsd.port.mk:1053: Cannot open /usr/ports/Mk/bsd.commands.mk [00:00:55] Warning: (devel/lua-language-server): in /usr/share/mk/bsd.port.mk:27 [00:00:55] Warning: (devel/lua-language-server): in /usr/share/mk/bsd.port.post.mk:4 [00:00:55] Warning: (devel/lua-language-server): in /usr/ports/devel/lua-language-server/Makefile:104 ... [00:00:55] Warning: (devel/lua-language-server): make: Fatal errors encountered -- cannot continue [00:00:55] devel/lua-luarocks depends on devel/gmake [00:00:55] Warning: (devel/lua-language-server): Error: Error looking up dependencies for devel/lua-language-server .. I think you have a race and will need to introduce locking or find a clever use of atomics for vofid->v_opens. Comment Actions Something like?: if (atomic_fetchadd_int(&vofid->v_opens, -1) == 1) {
p9fs_fid_remove(np, vofid, VOFID);
p9_client_clunk(vofid);
}Comment Actions So its going to be everywhere not just one location. If one place treats v_opens as atomic, they pretty much all have to change to check atomically that they didn't get a closed struct p9_fid and it they did to retry. Or it may be better to push all of that functionality into the functions that manage the struct p9_fid cache. But before going either of those routes are we sure there isn't some simpler off by one bug that is preventing things from aging out today? I am not the right person to answer that, I just use p9fs a lot but with bhyve instances that I only have up briefly so it hasn't been a priority for me to look into this. Comment Actions @dave_freedave.net Could you try a bulk poudriere run now? I seem to have gotten a successful run. Comment Actions When all handles referencing a vnode are closed, VOP_INACTIVE (implemented by p9fs_inactive()) should be called. Isn't that the right place to clunk the file? From what I can see, that's currently happening in p9fs_reclaim() which is too late as you say.
Comment Actions Generally VOP_INACTIVE() is a good place, but a (small) problem with it is that it might be skipped. That said, VOP_INACTIVE + VOP_RECLAIM should be good enough. Comment Actions
I had initially called p9fs_fid_remove_all() in VOP_INACTIVE, but as you said, it isn't guaranteed to be called. When I tested this solution, the number of open files on the host still grew quickly. For example, after a poudriere run, lsof shows ~25k file descriptors. Closing it in VOP_CLOSE keeps this number at <40 for me. Comment Actions Yes, but. I do not expect that it would be so drastic, and there is probably something else broken. VOP_INACTIVE() is skipped when the current lock in vput() is not exclusive and an immediate upgrade request failed because there are other shared lock owners. At very least this means that there are hold owners on the vnode without refs, which is not typical. | |||||||||||||||||||||||||||||||||||||||||||||