Page MenuHomeFreeBSD

sockets: use socket buffer mutexes in struct socket directly
AbandonedPublic

Authored by glebius on Mon, May 9, 6:45 PM.

Details

Reviewers
markj
glebius
Group Reviewers
network
transport
Summary
Since c67f3b8b78e the sockbuf mutexes belong to the containing socket,
and socket buffers just point to it.  In 74a68313b50 macros that access
this mutex directly were added.  Go over the core socket code and
eliminate code that reaches the mutex by dereferencing the sockbuf
compatibility pointer.

This change requires a KPI change, as some functions were given the
sockbuf pointer only without any hint if it is a receive or send buffer.

This change doesn't cover the whole kernel, many protocols still use
compatibility pointers internally.  However, it allows operation of a
protocol that doesn't use them.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 45543
Build 42431: arc lint + arc unit

Event Timeline

I have a large patch which converts most users of SOCKBUF_* to the new macros. If it will not conflict with what you are doing, I can post them.

This revision is now accepted and ready to land.Tue, May 10, 1:10 PM

I have a large patch which converts most users of SOCKBUF_* to the new macros. If it will not conflict with what you are doing, I can post them.

Thanks Mark. I got minimal patch that converts only those parts of code that are generic and doesn't cover all of the protocols. Let me check them in first.

I will update this revision to include changes to sbwait(), which unfortunately touch multiple modules. Without sbwait() this revision alone is not very useful. No reason to commit it separately.

glebius retitled this revision from sockets: in sowakeup macros access socket buffer mutexes directly to sockets: use socket buffer mutexes in struct socket directly.Thu, May 12, 5:51 PM
glebius edited the summary of this revision. (Show Details)

Make the patch larger to cover all of the core socket code.

This revision now requires review to proceed.Thu, May 12, 5:52 PM
sys/kern/uipc_sockbuf.c
546

For frequently called functions where which is fixed at compile time, it may be profitable to parameterize over which. For example, in sockbuf.h, have:

static inline void
sowakeup(struct socket *so, const sb_which which)
{
    if (which == SO_SND)
        sowakeup_snd(so);
    else
        sowakeup_rcv(so);
}

and in uipc_sockbuf.c:

static __always_inline void
_sowakeup(struct socket *so, const sb_which which)
{
    ...
}

static void
sowakeup_snd(struct socket *so)
{
    _sowakeup(so, SO_SND);
}

static void
sowakeup_rcv(struct socket *so)
{
    _sowakeup(so, SO_RCV);
}

The compiler can eliminate the conditional in sowakeup() in many cases since which is known at compile time. And if you force _sowakeup() to be inlined, the compiler can substitute fixed values for which and eliminate some more branches.

I'm not sure if it is worth it, and it makes the KPIs less general (you might want to add a third socket buffer for some reason).

sys/sys/socketvar.h
295

style(9) does not require these blank lines anymore, BTW.

sys/kern/uipc_sockbuf.c
546

Good idea. While implementing it, found out that sowakeup() is never used directly, so patch going to be even smaller.

Implement Mark's suggestion and go a little bit further.