svn commit: r248417 - head/sys/sys

[Andre Oppermann]

On 17.03.2013 08:39, Gleb Smirnoff wrote:
> Author: glebius
> Date: Sun Mar 17 07:39:45 2013
> New Revision: 248417
> URL: http://svnweb.freebsd.org/changeset/base/248417
>
> Log:
>    Add MEXT_ALIGN() macro, similar to M_ALIGN() and MH_ALIGN(), but for
>    mbufs with external buffer.

While you are cleaning up the mbuf usage wouldn't it make sense to remove
these macros, instead of adding new ones, and use m_align() which handles
all these cases internally?

IMHO we should not have the caller worry about the different types of
mbuf's but to be able to apply a particular function which then does the
right thing.  This would remove a lot of duplicated code all over the tree
again.

While discussing m_align() it should be extended to do the inverse from
aligning to the end as well.  Many times mbufs have to have an offset
from the start of the mbuf as well (ethernet).  A negative len value
could be used for that like in a_adj().

PS: It's really great you're doing this spring-cleaning.  The mbuf area
is littered with historic, complex and duplicate macros, functions and code.

-- 
Andre

> Modified:
>    head/sys/sys/mbuf.h
>
> Modified: head/sys/sys/mbuf.h
> ==============================================================================
> --- head/sys/sys/mbuf.h	Sun Mar 17 07:37:10 2013	(r248416)
> +++ head/sys/sys/mbuf.h	Sun Mar 17 07:39:45 2013	(r248417)
> @@ -195,7 +195,7 @@ struct mbuf {
>   #define	M_FIRSTFRAG	0x00001000 /* packet is first fragment */
>   #define	M_LASTFRAG	0x00002000 /* packet is last fragment */
>   #define	M_SKIP_FIREWALL	0x00004000 /* skip firewall processing */
> -#define	M_FREELIST	0x00008000 /* mbuf is on the free list */
> +		     /*	0x00008000    free */
>   #define	M_VLANTAG	0x00010000 /* ether_vtag is valid */
>   #define	M_PROMISC	0x00020000 /* packet was not for us */
>   #define	M_NOFREE	0x00040000 /* do not free mbuf, embedded in cluster */
> @@ -708,6 +708,18 @@ m_last(struct mbuf *m)
>   } while (0)
>
>   /*
> + * As above, for mbuf with external storage.
> + */
> +#define	MEXT_ALIGN(m, len) do {						\
> +	KASSERT((m)->m_flags & M_EXT,					\
> +		("%s: MEXT_ALIGN not an M_EXT mbuf", __func__));	\
> +	KASSERT((m)->m_data == (m)->m_ext.ext_buf,			\
> +		("%s: MEXT_ALIGN not a virgin mbuf", __func__));	\
> +	(m)->m_data += ((m)->m_ext.ext_size - (len)) &			\
> +	    ~(sizeof(long) - 1); 					\
> +} while (0)
> +
> +/*
>    * Compute the amount of space available before the current start of data in
>    * an mbuf.
>    *
>
>