This allows to stop maintaing the VI_TEXT_REF flag and consequently opens up fully lockless v_writecount adjustment. The new flag indicates that the vnodes gets refed/unrefed on -1 <-> 0 writecount transitions.
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
But why keep any flag at all? Specialize set/unset text for tmpfs. Then std versions are streamlined, and for tmpfs we always know that there should be a vref() if v_writecount < 0.
I have WIP code to roll with atomics-only. Short of code duplication I don't see how this can work. Say tmpfs was to call the general routines. Regardless if it checks v_writecount before or after it may be completely stale and consequently useless to deciding whether vref/vunref is necessary.
I am not proposing to make tmpfs to call into std. tmpfs can start with exactly the code we have right now in std minus check for the vnode flag. On the other hand, std then does not need to do vref/vrele at all and are left with just atomics.
Converting tmpfs to use atomics-only would be a separate work, it if it possible at all.
The routine looks like this:
int vop_stdset_text(struct vop_set_text_args *ap) { struct vnode *vp; int n; vp = ap->a_vp; n = atomic_load_int(&vp->v_writecount); for (;;) { if (n > 0) { return (ETXTBSY); } /* * Transition point, we may need to grab a reference on the vnode. */ if (n == 0) { if (atomic_fcmpset_int(&vp->v_writecount, &n, -1)) { if ((vn_irflag_read(vp) & VIRF_TEXT_REF) != 0) { vref(vp); } return (0); } continue; } MPASS(n < 0); if (atomic_fcmpset_int(&vp->v_writecount, &n, n - 1)) { return (0); } } __assert_unreachable(); } static int vop_stdunset_text(struct vop_unset_text_args *ap) { struct vnode *vp; int n; vp = ap->a_vp; n = atomic_load_int(&vp->v_writecount); for (;;) { if (n >= 0) { return (EINVAL); } /* * Transition point, we may need to release a reference on the vnode. */ if (n == -1) { if (atomic_fcmpset_int(&vp->v_writecount, &n, 0)) { if ((vn_irflag_read(vp) & VIRF_TEXT_REF) != 0) { vunref(vp); } return (0); } continue; } MPASS(n < -1); if (atomic_fcmpset_int(&vp->v_writecount, &n, n + 1)) { return (0); } } __assert_unreachable(); }
The entire point of a VIRF flag is that it does not suffer from forced unmount, so it is clear for the lifetime of the vnode if an extra ref is provided. Consequently there is no need to set any flags when mainting writecount.
Yes, I said above the same: for tmpfs there is no need for additional flag if we know that the vnode is tmpfs. And for std version there is no need in vref/vrele.
Pasted code demonstrates how both std and tmpfs case can be handled with atomics and the need to handle extra ref does not change much in the bigger picture -- the transition against 0 still has to be treated specially due to potential races against writers. Thus tmpfs would duplicate all of std + add the few lines to handle vref/vunref. As is, I think it's best to keep one set of routines in this case.