Link [ NetBSD | NetBSD OpenGrok source search | PR fulltext-search | Summary of daily snapshot builds | history of daily build result | pkgsrc commit viewer ]


   
        usage: [branch:branch] [user:user] [path@revision] keyword [... [-excludekeyword [...]]] (e.g. branch:MAIN sys/arch/arm, if_wm.c@1.234 )




switch to index mode

recent branches: MAIN (8m)  netbsd-10 (7d)  netbsd-9 (7d)  netbsd-8 (12d) 

2024-05-24 10:22:15 UTC Now

2023-07-30 12:09:51 UTC netbsd-10 commitmail json YAML

Pull up following revision(s) (requested by riastradh in ticket #262):

sys/kern/kern_descrip.c: revision 1.252
sys/kern/kern_descrip.c: revision 1.253
sys/kern/kern_descrip.c: revision 1.254

kern_descrip.c: Fix membars around reference count decrement.

In general, the `last one out hit the lights' style of reference
counting (as opposed to the `whoever's destroying must wait for
pending users to finish' style) requires memory barriers like so:
        ... usage of resources associated with object ...
        membar_release();
        if (atomic_dec_uint_nv(&obj->refcnt) != 0)
                return;
        membar_acquire();
        ... freeing of resources associated with object ...

This way, all usage happens-before all freeing.  This fixes several
errors:
- fd_close failed to ensure whatever its caller did would
  happen-before the freeing, in the case where another thread is
  concurrently trying to close the fd (ff->ff_file == NULL).
  Fix: Add membar_release before atomic_dec_uint(&ff->ff_refcnt) in
  that branch.
- fd_close failed to ensure all loads its caller had issued will have
  happened-before the freeing, in the case where the fd is still in
  use by another thread (fdp->fd_refcnt > 1 and ff->ff_refcnt-- > 0).
  Fix: Change membar_producer to membar_release before
  atomic_dec_uint(&ff->ff_refcnt).
- fd_close failed to ensure that any usage of fp by other callers
  would happen-before any freeing it does.
  Fix: Add membar_acquire after atomic_dec_uint_nv(&ff->ff_refcnt).
- fd_free failed to ensure that any usage of fdp by other callers
  would happen-before any freeing it does.
  Fix: Add membar_acquire after atomic_dec_uint_nv(&fdp->fd_refcnt).

While here, change membar_exit -> membar_release.  No semantic
change, just updating away from the legacy API.

kern_descrip.c: Use atomic_store_relaxed/release for ff->ff_file.
1. atomic_store_relaxed in fd_close avoids the appearance of race in
  sanitizers (minor bug).
2. atomic_store_release in fd_affix is necessary because the lock
  activity was not, in fact, enough to guarantee ordering (real bug
  some architectures like aarch64).
  The premise appears to have been that the mutex_enter/exit earlier
  in fd_affix is enough to guarantee that initialization of fp (A)
  happens before use of fp by a user once fp is published (B):
        fp->f_... = ...;                // A
        /* fd_affix */
        mutex_enter(&fp->f_lock);
        fp->f_count++;
        mutex_exit(&fp->f_lock);
        ...
        ff->ff_file = fp;              // B
  But actually mutex_enter/exit allow the following reordering by
  the CPU:
        mutex_enter(&fp->f_lock);
        ff->ff_file = fp;              // B
        fp->f_count++;
        fp->f_... = ...;                // A
        mutex_exit(&fp->f_lock);
  The only constraints they imply are:
        1. fp->f_count++ and B cannot precede mutex_enter
        2. mutex_exit cannot precede A and fp->f_count++
  They imply no constraint on the relative ordering of A, B, and
  fp->f_count++ amongst each other, however.
  This affects any architecture that has a native load-acquire or
  store-release operation in mutex_enter/exit, like aarch64, instead
  of explicit load-before-load/store and load/store-before-store
  barrier.

No need for atomic_store_* in fd_copy or fd_free because we have
exclusive access to ff as is.

kern_descrip.c: Change membar_enter to membar_acquire in fd_getfile.
membar_acquire is cheaper on many CPUs, and unlikely to be costlier
on any CPUs, than the legacy membar_enter.
Add a long comment explaining the interaction between fd_getfile and
fd_close and why membar_acquire is safe.

(martin)