Re: mtx_lock_recurse/mtx_unlock_recurse functions (proof-of-concept).

From: Seigo Tanimura (tanimura_at_tanimura.dyndns.org)
Date: 04/09/04

  • Next message: Robert Watson: "Re: mtx_lock_recurse/mtx_unlock_recurse functions (proof-of-concept)." 
    Date: Fri, 09 Apr 2004 17:53:58 +0900
To: Robert Watson <rwatson@FreeBSD.org>

    On Thu, 8 Apr 2004 18:33:00 -0400 (EDT),
      Robert Watson <rwatson@FreeBSD.org> said:

    rwatson> On Thu, 8 Apr 2004, Pawel Jakub Dawidek wrote:

    >> As was discussed, it will be helpful to have functions, that are able to
    >> acquire lock recursively, even if lock itself isn't recursable.
    >>
    >> Here is a patch, that should implement this functionality:
    >>
    >> http://people.freebsd.org/~pjd/patches/mtx_lock_recurse.patch
    >>
    >> I also added a KASSERT() to protect against mixed locking modes, e.g.:

    rwatson> As you know, this is of interest to me because several situations have
    rwatson> come up in the network stack locking code where we've worked around
    rwatson> locking issues by conditionally acquiring a lock based on whether it's not
    rwatson> already held. For example, in the rwatson_netperf branch, we
    rwatson> conditionally acquire the socket buffer lock in the KQueue filter because
    rwatson> we don't know if we're being called from a context that has already
    rwatson> acquired the mutex:

    rwatson> @@ -1875,8 +1915,11 @@
    rwatson> filt_sowrite(struct knote *kn, long hint)
    rwatson> {
    rwatson> struct socket *so = kn->kn_fp->f_data;
    rwatson> - int result;
    rwatson> + int needlock, result;
     
    rwatson> + needlock = !SOCKBUF_OWNED(&so->so_snd);
    rwatson> + if (needlock)
    rwatson> + SOCKBUF_LOCK(&so->so_snd);
    rwatson> kn->kn_data = sbspace(&so->so_snd);
    rwatson> if (so->so_state & SS_CANTSENDMORE) {
    rwatson> kn->kn_flags |= EV_EOF;
    rwatson> @@ -1891,6 +1934,8 @@
    rwatson> result = (kn->kn_data >= kn->kn_sdata);
    rwatson> else
    rwatson> result = (kn->kn_data >= so->so_snd.sb_lowat);
    rwatson> + if (needlock)
    rwatson> + SOCKBUF_UNLOCK(&so->so_snd);
    rwatson> return (result);
    rwatson> }

    rwatson> This stems directly from the fact that in general we wanted to avoid
    rwatson> recursion on socket buffer mutexes, but that in one case due to existing
    rwatson> implementation constraints, we don't know what state the lock will be in.
    rwatson> In this case, that happens because invocation of filt_sowrite() sometimes
    rwatson> occurs directly from the KQueue code to poll a filter, and sometimes from
    rwatson> the socket code during a wakeup that invokes KNOTE(). There are arguably
    rwatson> at least two ways in which the current code might be modified to try and
    rwatson> address this:

    rwatson> (0) Use recursive mutexes so that this recursion is permitted.

    rwatson> (1) Use separate APIs to enter the per-object filters so that the origins
    rwatson> of the invocation (and hence locking state) are more clear.

    rwatson> (2) Avoid holding existing locks over calls into KNOTE(), which goes off
    rwatson> and does a lot of stuff.

    rwatson> Neither of these is 100% desirable, and I guess I'd prefer (1) over (2)
    rwatson> since (2) conflicts with the use of locks as a form of reference counting.
    rwatson> However, the easy answer (0) is also undesirable because the socket code
    rwatson> generally doesn't need recursion of the socket buffer mutex. Given the
    rwatson> ability to selectively say "Ok, recursion here is fine" we would change
    rwatson> the above conditional locking into such a case.

    (3) Let a dedicated kernel thread call the methods of struct
        filterops.

    The headache of kqueue is that KNOTE() ultimately calls back the
    caller subsystem. Tty would suffer from the same pain as well.

    Assuming that a kqueue event need not be delivered synchronously upon
    the event, maybe we can attach an event queue to a knote list.
    KNOTE() appends an event to the queue and wakes up a dedicated kernel
    thread (kqueued). It then dequeues the event in a queue to call a
    filter *without* the lock of the event source. (socket, tty, etc.) 

    -- 
Seigo Tanimura <tanimura@tanimura.dyndns.org> <tanimura@FreeBSD.org>
_______________________________________________
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: Robert Watson: "Re: mtx_lock_recurse/mtx_unlock_recurse functions (proof-of-concept)."

    Relevant Pages

    • Re: [RFC] callout overhaul: part I
      ... Some benchmarks posted by rwatson some time ago ( ... This moved me in the direction of working on callouts, ... I modified a little bit this patch in order to make stuff saner. ... I introduced lock assertions in callout_stop. ...
      (freebsd-arch)
    • [RFC] callout overhaul: part I
      ... Some benchmarks posted by rwatson some time ago ( ... would add the support for callout working with rwlock. ... only possible kind of lock to be used in conjuction with callouts and ...
      (freebsd-arch)