Re: duplicate read/write locks in net/pfil.c and netinet/ip_fw2.c

From: Luigi Rizzo (rizzo_at_icir.org)
Date: 08/18/05

  • Next message: Stephan Uphoff: "Re: duplicate read/write locks in net/pfil.c and netinet/ip_fw2.c" 
    Date: Thu, 18 Aug 2005 00:57:39 -0700
To: Max Laier <max@love2party.net>

    On Thu, Aug 18, 2005 at 03:32:19AM +0200, Max Laier wrote:
    > On Thursday 18 August 2005 02:02, Luigi Rizzo wrote:
    ...
    > > could you guys look at the following code and see if it makes sense,
    > > or tell me where i am wrong ?
    > >
    > > It should solve the starvation and blocking trylock problems,
    > > because the active reader does not hold the mutex in the critical
                           ^^^^^^

    i meant 'writer', sorry... as max said even in the current implementation
    the reader does not hold the lock.

    > > int
    > > RLOCK(struct rwlock *rwl, int try)
    > > {
    > > if (!try)
    > > mtx_lock(&rwl->m);
    > > else if (!mtx_trylock(&rwl->m))
    > > return EBUSY;
    > > if (rwl->writers == 0) /* no writer, pass */
    > > rwl->readers++;
    > > else {
    > > rwl->br++;
    > > cv_wait(&rwl->qr, &rwl->m);
    > ^^^^^^^
    >
    > That we can't do. That's exactly the thing the existing sx(9) implementation
    > does and where it breaks. The problem is that cv_wait() is an implicit sleep
    > which breaks when we try to RLOCK() with other mutex already acquired.

    but that is not a solvable problem given that the *LOCK may be blocking.
    And the cv_wait is not an unconditioned sleep, it is one where you release
    the lock right before ans wait for an event to wake you up.
    In fact i don't understand why you consider spinning and sleeping
    on a mutex two different things.

    > Moreover will this break for recursive reads e.g.:
    >
    > Thread 1: RLOCK() ... RLOCK() -> cv_wait ...
    > Thread 2: WLOCK() -> cv_wait ...
    >
    > This is exactly what pfil_hooks must be able to do as the packet filter may
    > want to call back to the stack in order to send rejects etc.

    that's another story (also in issue in ipfw) and the way it is addressed
    elsewhere is by releasing and reaquiring the lock. In fact is the topic
    that started this thread.

    > change). The problem is, that we still have to invest 4 mutex operations for
    > every access. The current implementation has the same basic problem (though
    > it only uses 2 mutex operations for the WLOCK/UNLOCK). Ideally the RLOCK/
    > UNLOCK should be free unless there is a writer waiting for the lock. In

    "free unless" means not free - you always have to check, be it through
    an atomic cmpswap or something else. But this is what mtx_lock does
    anyways in the fast path.

            cheers
            luigi

    _______________________________________________
    freebsd-arch@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-arch
    To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"

  • Next message: Stephan Uphoff: "Re: duplicate read/write locks in net/pfil.c and netinet/ip_fw2.c"

    Relevant Pages

    • Re: duplicate read/write locks in net/pfil.c and netinet/ip_fw2.c
      ... >> It should solve the starvation and blocking trylock problems, ... The problem is that cv_waitis an implicit sleep ... > which breaks when we try to RLOCKwith other mutex already acquired. ... but that is not a solvable problem given that the *LOCK may be blocking. ...
      (freebsd-net)
    • Re: Recursive mutex that can be waited upon (pthread)
      ... While you can use a mutex to avoid that data is changed, for me having a mutex does not mean that data is not changed, it only means that data is not changed by a different thread. ... My own thread may of course change the data, hence functions I call may want to change the data and if they do so, they must be sure that these changes are atomically, hence they must lock the object and they simply can't rely that I locked the object before -> thus I need recursive locks. ... Then I could as well throw out threads of my code and just use a single thread going through an event queue. ... you have a predicate condition on an invariant. ...
      (comp.programming.threads)
    • [patch 6/8] mutex subsystem, core
      ... mutex implementation, core files: just the basic subsystem, no users of it. ... straightforward mutexes with strict semantics: ... + * Block on a lock - add ourselves to the list of waiters. ...
      (Linux-Kernel)
    • [patch 6/8] mutex subsystem, core
      ... mutex implementation, core files: just the basic subsystem, no users of it. ... straightforward mutexes with strict semantics: ... + * Block on a lock - add ourselves to the list of waiters. ...
      (Linux-Kernel)
    • [PATCH 4/8] adaptive real-time lock support
      ... The Real Time patches to the Linux kernel converts the architecture ... compromising the integrity of critical sections protected by the lock. ... while retaining both the priority inheritance protocol as well as the ... the RT Mutex has been ...
      (Linux-Kernel)