Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 17 Jan 2001 17:36:50 -0500
From:      "Bosko Milekic" <bmilekic@technokratis.com>
To:        "Alfred Perlstein" <bright@wintelcom.net>, "Randell Jesup" <rjesup@wgate.com>, <dillon@FreeBSD.ORG>
Cc:        "Soren Schmidt" <sos@freebsd.dk>, <arch@FreeBSD.ORG>, <current@FreeBSD.ORG>
Subject:   Re: HEADS-UP: await/asleep removal imminent
Message-ID:  <012a01c080d5$fc532270$1f90c918@jehovah>
References:  <200101171138.MAA11834@freebsd.dk> <ybug0iixiee.fsf@jesup.eng.tvol.net.jesup.eng.tvol.net> <20010117092109.O7240@fw.wintelcom.net> <20010117100516.Q7240@fw.wintelcom.net>

next in thread | previous in thread | raw e-mail | index | archive | help

Alfred Perlstein wrote:

> * Alfred Perlstein <bright@wintelcom.net> [010117 09:24] wrote:
> >
> > I'm not going to axe it for a few days, this is a really amazing
> > API that Matt added, the problem is utility and useage over code
> > complexity.
> >
> > It's just a proposal.
>
> I found several places where it may be useful, but I'm not sure if the
> benefits outweigh the gains.
>
> In a copy of the tree I've locked down the socket layer (not the entire
> stack, just sockets :) ) there's code like this:
>
>             SOCKBUF_UNLOCK(&so->so_snd, 0);
>             if (top == 0) {
>                 MGETHDR(m, M_TRYWAIT, MT_DATA);
>                 if (m == NULL) {
>                     error = ENOBUFS;
>                     SOCKBUF_LOCK(&so->so_snd, 0);
>                     goto release;
>                 }
>                 mlen = MHLEN;
> ...
>             SOCKBUF_LOCK(&so->so_snd, 0);       /* XXX */
>
> The lock must be unwound becasue we're calling MGETHDR with M_TRYWAIT.
> If wae used M_TRY'A'WAIT the code would probably look something like
> this:
>
>             /* SOCKBUF_UNLOCK(&so->so_snd, 0); */
> again:
>             if (top == 0) {
>                 MGETHDR(m, M_TRYWAIT, MT_DATA);
>                 if (m == NULL) {
>                     error = mawait(&so->so_snd.sb_mtx, -1, -1);
>                     if (error) {
>                       if (error == EWOULDBLOCK)
>                          error = ENOBUFS;
>                       goto release;
>                     }
>                     goto again;
>                     /* SOCKBUF_LOCK(&so->so_snd, 0); */
>                 }
>                 mlen = MHLEN;
> ...
>             /* SOCKBUF_LOCK(&so->so_snd, 0); */      /* XXX */
>
> Which means we don't have to drop the lock over the socket unless
> we'd block on allocation.

    No. You'd still have to drop it for now. Remember? (Last commit to
uipc_mbuf.c). You have to drop it because of the problem you may have if
Giant is gotten before your sockbuf/socket lock. In the allocation, you may
be acquiring Giant again. I don't know the exact semantics, but if you at
some point may grab the sockbuf/socket lock without already holding Giant and
call the allocation routine, you're opening the door for deadlock.

> Matt, is this what you intended for it to do?  So far I've only
> seen it used to avoid races, but not to optimize out mutex
> aquire/release.

    I've only seen it to be useful to avoid races. If you're holding a lock
and you need to sleep but if you drop the lock before you actually switch you
may get woken up and never find out, thus still going to sleep. With the
asleep you could hold the lock and place yourself on the sleep queue such
that when you drop the lock and call await, you'll find out if you've gotten
awoken (you'll be removed from the sleep queue). With the interlocking with
sched_lock now down in msleep(), this "feature" of asleep/await is useless.

> --
> -Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
> "I have the heart of a child; I keep it in a jar on my desk."

Regards,
Bosko.




To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?012a01c080d5$fc532270$1f90c918>