Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/uvm/pmap sys: Membar audit around reference count releases.



details:   https://anonhg.NetBSD.org/src/rev/6901842a46a9
branches:  trunk
changeset: 363430:6901842a46a9
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Mar 12 15:32:30 2022 +0000

description:
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.

diffstat:

 sys/arch/aarch64/aarch64/pmap.c                  |   6 ++-
 sys/arch/alpha/alpha/pmap.c                      |   5 ++-
 sys/arch/arm/arm32/pmap.c                        |   6 ++-
 sys/arch/hppa/hppa/pmap.c                        |   6 ++-
 sys/arch/ia64/ia64/pmap.c                        |   6 ++-
 sys/arch/powerpc/oea/pmap.c                      |   6 ++-
 sys/arch/sparc/sparc/pmap.c                      |   6 ++-
 sys/arch/sparc64/sparc64/pmap.c                  |   6 ++-
 sys/dev/hyperv/vmbus.c                           |   6 ++-
 sys/dev/marvell/mvxpsec.c                        |   7 +++-
 sys/dev/scsipi/atapiconf.c                       |   9 ++++--
 sys/dev/scsipi/scsiconf.c                        |   9 ++++--
 sys/dev/scsipi/scsipi_base.c                     |   6 ++-
 sys/external/bsd/drm2/linux/linux_stop_machine.c |  11 ++++---
 sys/kern/kern_auth.c                             |  10 +++++-
 sys/kern/kern_exec.c                             |   7 +++-
 sys/kern/kern_mutex_obj.c                        |  10 +++++-
 sys/kern/kern_resource.c                         |   6 ++-
 sys/kern/kern_rwlock_obj.c                       |  10 +++++-
 sys/kern/kern_sig.c                              |   6 ++-
 sys/kern/subr_kcpuset.c                          |   6 ++-
 sys/kern/sys_futex.c                             |  10 +++++-
 sys/kern/uipc_mbuf.c                             |  10 +++++-
 sys/kern/vfs_cwd.c                               |   6 ++-
 sys/kern/vfs_mount.c                             |  10 +++++-
 sys/kern/vfs_vnode.c                             |  34 +++++++++++++++++++----
 sys/kern/vfs_wapbl.c                             |   6 ++-
 sys/net/if.c                                     |  16 +++++++---
 sys/net/npf/npf_nat.c                            |   8 ++++-
 sys/net/npf/npf_rproc.c                          |   8 ++++-
 sys/net/npf/npf_tableset.c                       |  12 +++++--
 sys/uvm/pmap/pmap.c                              |   6 ++-
 sys/uvm/uvm_aobj.c                               |  10 +++++-
 sys/uvm/uvm_map.c                                |   7 +++-
 34 files changed, 212 insertions(+), 81 deletions(-)

diffs (truncated from 1116 to 300 lines):

diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/arch/aarch64/aarch64/pmap.c
--- a/sys/arch/aarch64/aarch64/pmap.c   Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/arch/aarch64/aarch64/pmap.c   Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pmap.c,v 1.129 2022/03/05 16:53:24 skrll Exp $ */
+/*     $NetBSD: pmap.c,v 1.130 2022/03/12 15:32:30 riastradh Exp $     */
 
 /*
  * Copyright (c) 2017 Ryo Shimizu <ryo%nerv.org@localhost>
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.129 2022/03/05 16:53:24 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.130 2022/03/12 15:32:30 riastradh Exp $");
 
 #include "opt_arm_debug.h"
 #include "opt_cpuoptions.h"
@@ -1560,9 +1560,11 @@
        if (pm == pmap_kernel())
                panic("cannot destroy kernel pmap");
 
+       membar_exit();
        refcnt = atomic_dec_uint_nv(&pm->pm_refcnt);
        if (refcnt > 0)
                return;
+       membar_enter();
 
        KASSERT(LIST_EMPTY(&pm->pm_pvlist));
        pmap_tlb_asid_release_all(pm);
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/arch/alpha/alpha/pmap.c
--- a/sys/arch/alpha/alpha/pmap.c       Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/arch/alpha/alpha/pmap.c       Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: pmap.c,v 1.304 2021/12/09 21:13:18 andvar Exp $ */
+/* $NetBSD: pmap.c,v 1.305 2022/03/12 15:32:31 riastradh Exp $ */
 
 /*-
  * Copyright (c) 1998, 1999, 2000, 2001, 2007, 2008, 2020
@@ -135,7 +135,7 @@
 
 #include <sys/cdefs.h>                 /* RCS ID & Copyright macro defns */
 
-__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.304 2021/12/09 21:13:18 andvar Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.305 2022/03/12 15:32:31 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1663,6 +1663,7 @@
        KASSERT(atomic_load_relaxed(&pmap->pm_count) > 0);
        if (atomic_dec_uint_nv(&pmap->pm_count) > 0)
                return;
+       PMAP_MP(membar_enter());
 
        pt_entry_t *lev1map = pmap_lev1map(pmap);
 
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/arch/arm/arm32/pmap.c
--- a/sys/arch/arm/arm32/pmap.c Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/arch/arm/arm32/pmap.c Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pmap.c,v 1.432 2022/01/02 11:20:03 riastradh Exp $     */
+/*     $NetBSD: pmap.c,v 1.433 2022/03/12 15:32:31 riastradh Exp $     */
 
 /*
  * Copyright 2003 Wasabi Systems, Inc.
@@ -192,7 +192,7 @@
 #endif
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.432 2022/01/02 11:20:03 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.433 2022/03/12 15:32:31 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -5275,6 +5275,7 @@
        /*
         * Drop reference count
         */
+       membar_exit();
        if (atomic_dec_uint_nv(&pm->pm_refs) > 0) {
 #ifndef ARM_MMU_EXTENDED
                if (pmap_is_current(pm)) {
@@ -5285,6 +5286,7 @@
 #endif
                return;
        }
+       membar_enter();
 
        /*
         * reference count is zero, free pmap resources and then free pmap.
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/arch/hppa/hppa/pmap.c
--- a/sys/arch/hppa/hppa/pmap.c Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/arch/hppa/hppa/pmap.c Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pmap.c,v 1.114 2020/08/19 07:29:00 simonb Exp $        */
+/*     $NetBSD: pmap.c,v 1.115 2022/03/12 15:32:31 riastradh Exp $     */
 
 /*-
  * Copyright (c) 2001, 2002, 2020 The NetBSD Foundation, Inc.
@@ -65,7 +65,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.114 2020/08/19 07:29:00 simonb Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.115 2022/03/12 15:32:31 riastradh Exp $");
 
 #include "opt_cputype.h"
 
@@ -1249,8 +1249,10 @@
        off_t off;
 #endif
 
+       membar_exit();
        if (atomic_dec_uint_nv(&pmap->pm_obj.uo_refs) > 0)
                return;
+       membar_enter();
 
 #ifdef DIAGNOSTIC
        uvm_page_array_init(&a, &pmap->pm_obj, 0);
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/arch/ia64/ia64/pmap.c
--- a/sys/arch/ia64/ia64/pmap.c Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/arch/ia64/ia64/pmap.c Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: pmap.c,v 1.40 2020/03/14 14:05:42 ad Exp $ */
+/* $NetBSD: pmap.c,v 1.41 2022/03/12 15:32:31 riastradh Exp $ */
 
 /*-
  * Copyright (c) 1998, 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -81,7 +81,7 @@
 
 #include <sys/cdefs.h>
 
-__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.40 2020/03/14 14:05:42 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.41 2022/03/12 15:32:31 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/atomic.h>
@@ -1517,8 +1517,10 @@
        UVMHIST_FUNC(__func__); UVMHIST_CALLED(maphist);
        UVMHIST_LOG(maphist, "(pm=%p)", pmap, 0, 0, 0);
        
+       membar_exit();
        if (atomic_dec_64_nv(&pmap->pm_refcount) > 0)
                return;
+       membar_enter();
 
        KASSERT(pmap->pm_stats.resident_count == 0);
        KASSERT(pmap->pm_stats.wired_count == 0);
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/arch/powerpc/oea/pmap.c
--- a/sys/arch/powerpc/oea/pmap.c       Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/arch/powerpc/oea/pmap.c       Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pmap.c,v 1.111 2022/02/18 19:04:52 martin Exp $        */
+/*     $NetBSD: pmap.c,v 1.112 2022/03/12 15:32:31 riastradh Exp $     */
 /*-
  * Copyright (c) 2001 The NetBSD Foundation, Inc.
  * All rights reserved.
@@ -63,7 +63,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.111 2022/02/18 19:04:52 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.112 2022/03/12 15:32:31 riastradh Exp $");
 
 #define        PMAP_NOOPNAMES
 
@@ -1230,7 +1230,9 @@
 void
 pmap_destroy(pmap_t pm)
 {
+       membar_exit();
        if (atomic_dec_uint_nv(&pm->pm_refs) == 0) {
+               membar_enter();
                pmap_release(pm);
                pool_put(&pmap_pool, pm);
        }
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/arch/sparc/sparc/pmap.c
--- a/sys/arch/sparc/sparc/pmap.c       Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/arch/sparc/sparc/pmap.c       Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pmap.c,v 1.375 2021/08/09 21:08:06 andvar Exp $ */
+/*     $NetBSD: pmap.c,v 1.376 2022/03/12 15:32:31 riastradh Exp $ */
 
 /*
  * Copyright (c) 1996
@@ -56,7 +56,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.375 2021/08/09 21:08:06 andvar Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.376 2022/03/12 15:32:31 riastradh Exp $");
 
 #include "opt_ddb.h"
 #include "opt_kgdb.h"
@@ -4465,7 +4465,9 @@
 {
 
        DPRINTF(PDB_DESTROY, "pmap_destroy[%d](%p)", cpu_number(), pm);
+       membar_exit();
        if (atomic_dec_uint_nv(&pm->pm_refcount) == 0) {
+               membar_enter();
                pmap_quiet_check(pm);
                pool_cache_put(&pmap_cache, pm);
        }
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/arch/sparc64/sparc64/pmap.c
--- a/sys/arch/sparc64/sparc64/pmap.c   Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/arch/sparc64/sparc64/pmap.c   Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pmap.c,v 1.313 2022/01/01 11:56:15 hannken Exp $       */
+/*     $NetBSD: pmap.c,v 1.314 2022/03/12 15:32:31 riastradh Exp $     */
 /*
  *
  * Copyright (C) 1996-1999 Eduardo Horvath.
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.313 2022/01/01 11:56:15 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.314 2022/03/12 15:32:31 riastradh Exp $");
 
 #undef NO_VCACHE /* Don't forget the locked TLB in dostart */
 #define        HWREF
@@ -1510,9 +1510,11 @@
 #endif
        struct vm_page *pg;
 
+       membar_exit();
        if ((int)atomic_dec_uint_nv(&pm->pm_refs) > 0) {
                return;
        }
+       membar_enter();
        DPRINTF(PDB_DESTROY, ("pmap_destroy: freeing pmap %p\n", pm));
 #ifdef MULTIPROCESSOR
        CPUSET_CLEAR(pmap_cpus_active);
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/dev/hyperv/vmbus.c
--- a/sys/dev/hyperv/vmbus.c    Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/dev/hyperv/vmbus.c    Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vmbus.c,v 1.15 2021/12/23 04:06:51 yamaguchi Exp $     */
+/*     $NetBSD: vmbus.c,v 1.16 2022/03/12 15:32:31 riastradh Exp $     */
 /*     $OpenBSD: hyperv.c,v 1.43 2017/06/27 13:56:15 mikeb Exp $       */
 
 /*-
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vmbus.c,v 1.15 2021/12/23 04:06:51 yamaguchi Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vmbus.c,v 1.16 2022/03/12 15:32:31 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1452,8 +1452,10 @@
        KASSERTMSG(ch->ch_refs > 0, "channel%u: invalid refcnt %d",
            ch->ch_id, ch->ch_refs);
 
+       membar_exit();
        refs = atomic_dec_uint_nv(&ch->ch_refs);
        if (refs == 0) {
+               membar_enter();
                /* Detach the target channel. */
                vmbus_devq_enqueue(ch->ch_sc, VMBUS_DEV_TYPE_DETACH, ch);
        }
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/dev/marvell/mvxpsec.c
--- a/sys/dev/marvell/mvxpsec.c Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/dev/marvell/mvxpsec.c Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: mvxpsec.c,v 1.10 2022/02/12 03:24:36 riastradh Exp $   */
+/*     $NetBSD: mvxpsec.c,v 1.11 2022/03/12 15:32:32 riastradh Exp $   */
 /*
  * Copyright (c) 2015 Internet Initiative Japan Inc.
  * All rights reserved.
@@ -1552,9 +1552,12 @@
 {
        uint32_t refs;
 
+       membar_exit();
        refs = atomic_dec_32_nv(&mv_s->refs);
-       if (refs == 0)
+       if (refs == 0) {
+               membar_enter();
                pool_cache_put(mv_s->sc->sc_session_pool, mv_s);
+       }
 }
 
 /*
diff -r b1b0f9b5ac1e -r 6901842a46a9 sys/dev/scsipi/atapiconf.c
--- a/sys/dev/scsipi/atapiconf.c        Sat Mar 12 15:30:51 2022 +0000
+++ b/sys/dev/scsipi/atapiconf.c        Sat Mar 12 15:32:30 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atapiconf.c,v 1.93 2021/08/07 16:19:16 thorpej Exp $   */
+/*     $NetBSD: atapiconf.c,v 1.94 2022/03/12 15:32:32 riastradh Exp $ */
 
 /*
  * Copyright (c) 1996, 2001 Manuel Bouyer.  All rights reserved.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: atapiconf.c,v 1.93 2021/08/07 16:19:16 thorpej Exp $");



Home | Main Index | Thread Index | Old Index