Page MenuHomeFreeBSD

vfs: replace the MNTK_TEXT_REFS flag with VIRF_TEXT_REF
AbandonedPublic

Authored by mjg on May 18 2021, 3:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jan 10, 1:07 PM
Unknown Object (File)
Tue, Dec 17, 9:57 PM
Unknown Object (File)
Dec 4 2024, 11:11 AM
Unknown Object (File)
Dec 1 2024, 1:17 PM
Unknown Object (File)
Nov 19 2024, 11:53 AM
Unknown Object (File)
Nov 6 2024, 11:15 AM
Unknown Object (File)
Oct 31 2024, 1:05 PM
Unknown Object (File)
Oct 28 2024, 11:17 AM
Subscribers

Details

Reviewers
kib
Summary

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.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

mjg requested review of this revision.May 18 2021, 3:32 PM
mjg created this revision.
mjg edited the summary of this revision. (Show Details)

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.

In D30337#681331, @mjg wrote:

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.