Date: Wed, 14 Jan 2015 23:44:00 +0000 (UTC) From: Robert Watson <rwatson@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r277203 - in head/sys: kern sys Message-ID: <201501142344.t0ENi0tI088747@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rwatson Date: Wed Jan 14 23:44:00 2015 New Revision: 277203 URL: https://svnweb.freebsd.org/changeset/base/277203 Log: In order to support ongoing work to implement variable-size mbufs, and more generally make it easier to extend 'struct mbuf in the future', make a number of changes to the data structure: - As we anticipate embedding mbufs headers within variable-size regions of memory in the future, change the definitions of byte arrays embedded in mbufs to be of size [0] rather than [MLEN] and [MHLEN]. In fact, the cxgbe driver already uses 'struct mbuf' on the front of other storage sizes, but we would like the global mbuf allocator do be able to do this as well. - Fold 'struct m_hdr' into 'struct mbuf' itself, eliminating a set of macros that aliased 'mh_foo' field names to 'm_foo' names such as 'm_next'. These present a particular problem as we would like to add new mbuf-header fields -- e.g., 'm_size' -- that, if similarly named via macros, would introduce collisions with many other variable names in the kernel. - Rename 'struct m_ext' to 'struct struct_m_ext' so that we can add compile-time assertions without bumping into the still-extant 'm_ext' macro. - Remove the MSIZE compile-time assertion for 'struct mbuf', but add new assertions for alignment of embedded data arrays (64-bit alignment even on 32-bit platforms), and for the sizes the mbuf header, packet header, and m_ext structure. - Document that these assertions exist in comments in mbuf.h. This change is not intended to cause (non-trivial) behavioural differences, but is a precursor to further mbuf-allocator work. Differential Revision: https://reviews.freebsd.org/D1483 Reviewed by: bz, gnn, np, glebius ("go ahead, I trust you") Sponsored by: EMC / Isilon Storage Division Modified: head/sys/kern/uipc_mbuf.c head/sys/sys/mbuf.h Modified: head/sys/kern/uipc_mbuf.c ============================================================================== --- head/sys/kern/uipc_mbuf.c Wed Jan 14 23:34:00 2015 (r277202) +++ head/sys/kern/uipc_mbuf.c Wed Jan 14 23:44:00 2015 (r277203) @@ -88,11 +88,38 @@ SYSCTL_INT(_kern_ipc, OID_AUTO, m_defrag * Ensure the correct size of various mbuf parameters. It could be off due * to compiler-induced padding and alignment artifacts. */ -CTASSERT(sizeof(struct mbuf) == MSIZE); CTASSERT(MSIZE - offsetof(struct mbuf, m_dat) == MLEN); CTASSERT(MSIZE - offsetof(struct mbuf, m_pktdat) == MHLEN); /* + * mbuf data storage should be 64-bit aligned regardless of architectural + * pointer size; check this is the case with and without a packet header. + */ +CTASSERT(offsetof(struct mbuf, m_dat) % 8 == 0); +CTASSERT(offsetof(struct mbuf, m_pktdat) % 8 == 0); + +/* + * While the specific values here don't matter too much (i.e., +/- a few + * words), we do want to ensure that changes to these values are carefully + * reasoned about and properly documented. This is especially the case as + * network-protocol and device-driver modules encode these layouts, and must + * be recompiled if the structures change. Check these values at compile time + * against the ones documented in comments in mbuf.h. + * + * NB: Possibly they should be documented there via #define's and not just + * comments. + */ +#if defined(__LP64__) +CTASSERT(offsetof(struct mbuf, m_dat) == 32); +CTASSERT(sizeof(struct pkthdr) == 56); +CTASSERT(sizeof(struct struct_m_ext) == 48); +#else +CTASSERT(offsetof(struct mbuf, m_dat) == 24); +CTASSERT(sizeof(struct pkthdr) == 48); +CTASSERT(sizeof(struct struct_m_ext) == 28); +#endif + +/* * m_get2() allocates minimum mbuf that would fit "size" argument. */ struct mbuf * Modified: head/sys/sys/mbuf.h ============================================================================== --- head/sys/sys/mbuf.h Wed Jan 14 23:34:00 2015 (r277202) +++ head/sys/sys/mbuf.h Wed Jan 14 23:44:00 2015 (r277203) @@ -60,9 +60,15 @@ * MLEN is data length in a normal mbuf. * MHLEN is data length in an mbuf with pktheader. * MINCLSIZE is a smallest amount of data that should be put into cluster. + * + * Compile-time assertions in uipc_mbuf.c test these values to ensure that + * they are sensible. */ -#define MLEN ((int)(MSIZE - sizeof(struct m_hdr))) -#define MHLEN ((int)(MLEN - sizeof(struct pkthdr))) +struct mbuf; +#define MHSIZE offsetof(struct mbuf, M_dat.M_databuf) +#define MPKTHSIZE offsetof(struct mbuf, M_dat.MH.MH_dat.MH_databuf) +#define MLEN ((int)(MSIZE - MHSIZE)) +#define MHLEN ((int)(MSIZE - MPKTHSIZE)) #define MINCLSIZE (MHLEN + 1) #ifdef _KERNEL @@ -87,23 +93,6 @@ struct mb_args { #endif /* _KERNEL */ /* - * Header present at the beginning of every mbuf. - * Size ILP32: 24 - * LP64: 32 - */ -struct m_hdr { - struct mbuf *mh_next; /* next buffer in chain */ - struct mbuf *mh_nextpkt; /* next chain in queue/record */ - caddr_t mh_data; /* location of data */ - int32_t mh_len; /* amount of data in this mbuf */ - uint32_t mh_type:8, /* type of data in this mbuf */ - mh_flags:24; /* flags; see below */ -#if !defined(__LP64__) - uint32_t mh_pad; /* pad for 64bit alignment */ -#endif -}; - -/* * Packet tag structure (see below for details). */ struct m_tag { @@ -118,6 +107,8 @@ struct m_tag { * Record/packet header in first mbuf of chain; valid only if M_PKTHDR is set. * Size ILP32: 48 * LP64: 56 + * Compile-time assertions in uipc_mbuf.c test these values to ensure that + * they are correct. */ struct pkthdr { struct ifnet *rcvif; /* rcv interface */ @@ -166,8 +157,10 @@ struct pkthdr { * set. * Size ILP32: 28 * LP64: 48 + * Compile-time assertions in uipc_mbuf.c test these values to ensure that + * they are correct. */ -struct m_ext { +struct struct_m_ext { volatile u_int *ext_cnt; /* pointer to ref count info */ caddr_t ext_buf; /* start of buffer */ uint32_t ext_size; /* size of buffer, for ext_free */ @@ -184,24 +177,41 @@ struct m_ext { * purposes. */ struct mbuf { - struct m_hdr m_hdr; + /* + * Header present at the beginning of every mbuf. + * Size ILP32: 24 + * LP64: 32 + * Compile-time assertions in uipc_mbuf.c test these values to ensure + * that they are correct. + */ + struct mbuf *m_next; /* next buffer in chain */ + struct mbuf *m_nextpkt; /* next chain in queue/record */ + caddr_t m_data; /* location of data */ + int32_t m_len; /* amount of data in this mbuf */ + uint32_t m_type:8, /* type of data in this mbuf */ + m_flags:24; /* flags; see below */ +#if !defined(__LP64__) + uint32_t m_pad; /* pad for 64bit alignment */ +#endif + + /* + * A set of optional headers (packet header, external storage header) + * and internal data storage. Historically, these arrays were sized + * to MHLEN (space left after a packet header) and MLEN (space left + * after only a regular mbuf header); they are now variable size in + * order to support future work on variable-size mbufs. + */ union { struct { struct pkthdr MH_pkthdr; /* M_PKTHDR set */ union { - struct m_ext MH_ext; /* M_EXT set */ - char MH_databuf[MHLEN]; + struct struct_m_ext MH_ext; /* M_EXT set */ + char MH_databuf[0]; } MH_dat; } MH; - char M_databuf[MLEN]; /* !M_PKTHDR, !M_EXT */ + char M_databuf[0]; /* !M_PKTHDR, !M_EXT */ } M_dat; }; -#define m_next m_hdr.mh_next -#define m_len m_hdr.mh_len -#define m_data m_hdr.mh_data -#define m_type m_hdr.mh_type -#define m_flags m_hdr.mh_flags -#define m_nextpkt m_hdr.mh_nextpkt #define m_pkthdr M_dat.MH.MH_pkthdr #define m_ext M_dat.MH.MH_dat.MH_ext #define m_pktdat M_dat.MH.MH_dat.MH_databuf
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201501142344.t0ENi0tI088747>