Accounting for mbufs and clusters assigned to a socket buffer

[Giorgos Keramidas]

On Sun, 27 Apr 2008 17:07:56 -0700, gnn at freebsd.org wrote:
>At Fri, 25 Apr 2008 15:58:28 +0200, andre wrote:
>>gnn at freebsd.org wrote:
>>> Howdy,
>>>
>>> The following patch updates the kernel (CURRENT as of 23 April or
>>> so) and netstat(1) to show not only the bytes in the receive and
>>> send queues but also the mbuf and cluster usage per socket buffer.
>>> I'd be interested in people's comments on this.  I'd like to extend
>>> such counting to show more information, in particular how much of a
>>> cluster or mbuf is actually in use.
>>
>> The intent of tracking that information is good.  However there are
>> some problems with your approach: M_EXT does not mean the mbuf has a
>> 2k cluster attached.  It could by any external storage.  That is a 2k
>> (classic) cluster, a 4k (page size) cluster, a 9k cluster, a VM page
>> (sendfile), and so on.
>
> Yup, this is a first cut.
>
>> The field sb_mbcnt already gives you the aggregated gross storage
>> space in use for the socket.  And sb_cc tells how much actual payload
>> it contains.
>
> Right but it does not tell us the mix of clusters vs. mbufs, which is
> something useful for tuning.

That would be very useful indeed :-)

I guess we could check *both* (m_flags & M_EXT) and m_ext.ext_type.

This won't really show how many mbufs use a classic cluster vs. a 9k
cluster, but checking for all the m_ext.ext_types is probably going to
look a bit ugly in a single if-check :(

We have at least EXT_CLUSTER, EXT_JUMBOP, EXT_JUMBO9, EXT_JUMBO16 and
EXT_PACKET which may be interesting for cluster-related accounting, but
in socketvar.h we can't really use the uma zone accounting.  I just
noticed that the 'interesting' ext_types are conveniently short integers
though.  Maybe we could change sb_ccnt to a small array, and then change

    if ((m)->m_flags & M_EXT) \
        (sb)->sb_ccnt += 1;

to something like:

    if ((m)->m_flags & M_EXT && (m)->m_ext.ext_type < EXT_MBUF) \
        (sb)->sb_ccnt[(m)->m_ext.ext_type - 1] += 1; \

With our current ext_type's

#define EXT_CLUSTER     1       /* mbuf cluster */
#define EXT_SFBUF       2       /* sendfile(2)'s sf_bufs */
#define EXT_JUMBOP      3       /* jumbo cluster 4096 bytes */
#define EXT_JUMBO9      4       /* jumbo cluster 9216 bytes */
#define EXT_JUMBO16     5       /* jumbo cluster 16184 bytes */
#define EXT_PACKET      6       /* mbuf+cluster from packet zone */
#define EXT_MBUF        7       /* external mbuf reference (M_IOVEC) */
...

the [ext_type - 1] would move the burden of parsing the ext_type at the
place where we actually have to print something about the sb_ccnt
counters.

Would the 'bloat' of u_int[6] for ext_type's which are unused  be too
much to keep around?