Alfred Perlstein wrote:

> * Alfred Perlstein <[EMAIL PROTECTED]> [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 - [[EMAIL PROTECTED]|[EMAIL PROTECTED]]
> "I have the heart of a child; I keep it in a jar on my desk."

Regards,
Bosko.




To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to