CVS commit: src/sys

["Taylor R Campbell" <riastradh%netbsd.org@localhost>]

Module Name:    src
Committed By:   riastradh
Date:           Sat Mar 12 15:32:33 UTC 2022

Modified Files:
        src/sys/arch/aarch64/aarch64: pmap.c
        src/sys/arch/alpha/alpha: pmap.c
        src/sys/arch/arm/arm32: pmap.c
        src/sys/arch/hppa/hppa: pmap.c
        src/sys/arch/ia64/ia64: pmap.c
        src/sys/arch/powerpc/oea: pmap.c
        src/sys/arch/sparc/sparc: pmap.c
        src/sys/arch/sparc64/sparc64: pmap.c
        src/sys/dev/hyperv: vmbus.c
        src/sys/dev/marvell: mvxpsec.c
        src/sys/dev/scsipi: atapiconf.c scsiconf.c scsipi_base.c
        src/sys/external/bsd/drm2/linux: linux_stop_machine.c
        src/sys/kern: kern_auth.c kern_exec.c kern_mutex_obj.c kern_resource.c
            kern_rwlock_obj.c kern_sig.c subr_kcpuset.c sys_futex.c uipc_mbuf.c
            vfs_cwd.c vfs_mount.c vfs_vnode.c vfs_wapbl.c
        src/sys/net: if.c
        src/sys/net/npf: npf_nat.c npf_rproc.c npf_tableset.c
        src/sys/uvm: uvm_aobj.c uvm_map.c
        src/sys/uvm/pmap: pmap.c

Log Message:
sys: Membar audit around reference count releases.

If two threads are using an object that is freed when the reference
count goes to zero, we need to ensure that all memory operations
related to the object happen before freeing the object.

Using an atomic_dec_uint_nv(&refcnt) == 0 ensures that only one
thread takes responsibility for freeing, but it's not enough to
ensure that the other thread's memory operations happen before the
freeing.

Consider:

          Thread A                        Thread B
        obj->foo = 42;                  obj->baz = 73;
        mumble(&obj->bar);              grumble(&obj->quux);
        /* membar_exit(); */            /* membar_exit(); */
        atomic_dec -- not last          atomic_dec -- last
                                        /* membar_enter(); */
                                        KASSERT(invariant(obj->foo,
                                            obj->bar));
                                        free_stuff(obj);

The memory barriers ensure that

        obj->foo = 42;
        mumble(&obj->bar);

in thread A happens before

        KASSERT(invariant(obj->foo, obj->bar));
        free_stuff(obj);

in thread B.  Without them, this ordering is not guaranteed.

So in general it is necessary to do

        membar_exit();
        if (atomic_dec_uint_nv(&obj->refcnt) != 0)
                return;
        membar_enter();

to release a reference, for the `last one out hit the lights' style
of reference counting.  (This is in contrast to the style where one
thread blocks new references and then waits under a lock for existing
ones to drain with a condvar -- no membar needed thanks to mutex(9).)

I searched for atomic_dec to find all these.  Obviously we ought to
have a better abstraction for this because there's so much copypasta.
This is a stop-gap measure to fix actual bugs until we have that.  It
would be nice if an abstraction could gracefully handle the different
styles of reference counting in use -- some years ago I drafted an
API for this, but making it cover everything got a little out of hand
(particularly with struct vnode::v_usecount) and I ended up setting
it aside to work on psref/localcount instead for better scalability.

I got bored of adding #ifdef __HAVE_ATOMIC_AS_MEMBAR everywhere, so I
only put it on things that look performance-critical on 5sec review.
We should really adopt membar_enter_preatomic/membar_exit_postatomic
or something (except they are applicable only to atomic r/m/w, not to
atomic_load/store_*, making the naming annoying) and get rid of all
the ifdefs.


To generate a diff of this commit:
cvs rdiff -u -r1.129 -r1.130 src/sys/arch/aarch64/aarch64/pmap.c
cvs rdiff -u -r1.304 -r1.305 src/sys/arch/alpha/alpha/pmap.c
cvs rdiff -u -r1.432 -r1.433 src/sys/arch/arm/arm32/pmap.c
cvs rdiff -u -r1.114 -r1.115 src/sys/arch/hppa/hppa/pmap.c
cvs rdiff -u -r1.40 -r1.41 src/sys/arch/ia64/ia64/pmap.c
cvs rdiff -u -r1.111 -r1.112 src/sys/arch/powerpc/oea/pmap.c
cvs rdiff -u -r1.375 -r1.376 src/sys/arch/sparc/sparc/pmap.c
cvs rdiff -u -r1.313 -r1.314 src/sys/arch/sparc64/sparc64/pmap.c
cvs rdiff -u -r1.15 -r1.16 src/sys/dev/hyperv/vmbus.c
cvs rdiff -u -r1.10 -r1.11 src/sys/dev/marvell/mvxpsec.c
cvs rdiff -u -r1.93 -r1.94 src/sys/dev/scsipi/atapiconf.c
cvs rdiff -u -r1.298 -r1.299 src/sys/dev/scsipi/scsiconf.c
cvs rdiff -u -r1.187 -r1.188 src/sys/dev/scsipi/scsipi_base.c
cvs rdiff -u -r1.2 -r1.3 src/sys/external/bsd/drm2/linux/linux_stop_machine.c
cvs rdiff -u -r1.78 -r1.79 src/sys/kern/kern_auth.c
cvs rdiff -u -r1.515 -r1.516 src/sys/kern/kern_exec.c
cvs rdiff -u -r1.7 -r1.8 src/sys/kern/kern_mutex_obj.c
cvs rdiff -u -r1.187 -r1.188 src/sys/kern/kern_resource.c
cvs rdiff -u -r1.5 -r1.6 src/sys/kern/kern_rwlock_obj.c
cvs rdiff -u -r1.402 -r1.403 src/sys/kern/kern_sig.c
cvs rdiff -u -r1.12 -r1.13 src/sys/kern/subr_kcpuset.c
cvs rdiff -u -r1.15 -r1.16 src/sys/kern/sys_futex.c
cvs rdiff -u -r1.244 -r1.245 src/sys/kern/uipc_mbuf.c
cvs rdiff -u -r1.6 -r1.7 src/sys/kern/vfs_cwd.c
cvs rdiff -u -r1.87 -r1.88 src/sys/kern/vfs_mount.c
cvs rdiff -u -r1.135 -r1.136 src/sys/kern/vfs_vnode.c
cvs rdiff -u -r1.109 -r1.110 src/sys/kern/vfs_wapbl.c
cvs rdiff -u -r1.501 -r1.502 src/sys/net/if.c
cvs rdiff -u -r1.50 -r1.51 src/sys/net/npf/npf_nat.c
cvs rdiff -u -r1.20 -r1.21 src/sys/net/npf/npf_rproc.c
cvs rdiff -u -r1.36 -r1.37 src/sys/net/npf/npf_tableset.c
cvs rdiff -u -r1.153 -r1.154 src/sys/uvm/uvm_aobj.c
cvs rdiff -u -r1.391 -r1.392 src/sys/uvm/uvm_map.c
cvs rdiff -u -r1.62 -r1.63 src/sys/uvm/pmap/pmap.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.