OS-8054 inotify watches lead to EBUSY during zfs mount #239
Conversation
There was a problem hiding this comment.
I'm not at all confident that the fem code is the right place to be accounting for this. There's absolutely no guarantee of a v_count / fem correlation. e.g. a FEM client could install twice if they liked, sure that they won't lose their vnode if they just have a count of one.
This accounting needs to be pushed out to the clients at the point of inotify/portfs's VN_HOLD/RELEs, with a new vnode_t::v_phantom_count or somesuch.
That won't account for all FEM users but should at least fix this case, and I'm hoping it won't get too tricky, as the fem_install/uninstall callers don't look too bad. Ideally we'd get a good paragraph and what this is and isn't fixing - in particular what the count actually means and when it's suitable to use it.
- you've left out other file systems, presumably on purpose. Could you explicitly clarify that?
- maybe an update to the comment in common/fs/portfs/port_fop.c ?
- this should really get upstreamed first then we can add the inotify test after it's merged
| } | ||
|
|
||
| void | ||
| watch_port(char *path) |
| @@ -1832,7 +1831,7 @@ zfs_mount(vfs_t *vfsp, vnode_t *mvp, struct mounta *uap, cred_t *cr) | |||
| mutex_enter(&mvp->v_lock); | |||
| if ((uap->flags & MS_REMOUNT) == 0 && | |||
| (uap->flags & MS_OVERLAY) == 0 && | |||
| (mvp->v_count != 1 || (mvp->v_flag & VROOT))) { | |||
| (fem_getvnrefs(mvp) != 1 || (mvp->v_flag & VROOT))) { | |||
There was a problem hiding this comment.
So I understand that you don't want to go and fix all the filesystems right now, but could we at least abstract this check out to a general vfs function so others can use it if we ever want to fix them?
|
Already integrated with #305 |
No description provided.