On Wed, Dec 05, 2012 at 13:29 +0100, Mike Belopuhov wrote:
> converting mallocs to pools has an advantage of getting rid of
> the malloc in the forwarding path which gives a tiny bit of
> speed up but the main thing is that it makes life easier for
> those who have started working on the locking in the stack.
>
> henning and markus agree that having a single pool for all
> tags is better than having separate pools for each type so
> here's a diff that does it.
>
> ok?
>
Is anyone willing to OK this?
> diff --git sys/kern/uipc_mbuf.c sys/kern/uipc_mbuf.c
> index 4ec2bed..2595155 100644
> --- sys/kern/uipc_mbuf.c
> +++ sys/kern/uipc_mbuf.c
> @@ -99,6 +99,7 @@
>
> struct mbstat mbstat; /* mbuf stats */
> struct pool mbpool; /* mbuf pool */
> +struct pool mtagpool;
>
> /* mbuf cluster pools */
> u_int mclsizes[] = {
> @@ -150,6 +151,10 @@ mbinit(void)
> pool_set_constraints(&mbpool, &kp_dma_contig);
> pool_setlowat(&mbpool, mblowat);
>
> + pool_init(&mtagpool, PACKET_TAG_MAXSIZE + sizeof(struct m_tag),
> + 0, 0, 0, "mtagpl", NULL);
> + pool_setipl(&mtagpool, IPL_NET);
> +
> for (i = 0; i < nitems(mclsizes); i++) {
> snprintf(mclnames[i], sizeof(mclnames[0]), "mcl%dk",
> mclsizes[i] >> 10);
> diff --git sys/kern/uipc_mbuf2.c sys/kern/uipc_mbuf2.c
> index cca045d..4cfa9a8 100644
> --- sys/kern/uipc_mbuf2.c
> +++ sys/kern/uipc_mbuf2.c
> @@ -68,6 +68,8 @@
> #include <sys/malloc.h>
> #include <sys/mbuf.h>
>
> +extern struct pool mtagpool;
> +
> /* can't call it m_dup(), as freebsd[34] uses m_dup() with different arg */
> static struct mbuf *m_dup1(struct mbuf *, int, int, int);
>
> @@ -256,7 +258,9 @@ m_tag_get(int type, int len, int wait)
>
> if (len < 0)
> return (NULL);
> - t = malloc(sizeof(struct m_tag) + len, M_PACKET_TAGS, wait);
> + if (len > mtagpool.pr_size - sizeof(struct m_tag))
> + panic("requested tag size for pool %#x is too big", type);
> + t = pool_get(&mtagpool, wait == M_WAITOK ? PR_WAITOK : PR_NOWAIT);
> if (t == NULL)
> return (NULL);
> t->m_tag_id = type;
> @@ -280,7 +284,7 @@ m_tag_delete(struct mbuf *m, struct m_tag *t)
> struct m_tag *p;
>
> SLIST_REMOVE(&m->m_pkthdr.tags, t, m_tag, m_tag_link);
> - free(t, M_PACKET_TAGS);
> + pool_put(&mtagpool, t);
>
> SLIST_FOREACH(p, &m->m_pkthdr.tags, m_tag_link)
> tagsset |= p->m_tag_id;
> @@ -296,7 +300,7 @@ m_tag_delete_chain(struct mbuf *m)
>
> while ((p = SLIST_FIRST(&m->m_pkthdr.tags)) != NULL) {
> SLIST_REMOVE_HEAD(&m->m_pkthdr.tags, m_tag_link);
> - free(p, M_PACKET_TAGS);
> + pool_put(&mtagpool, p);
> }
> m->m_pkthdr.tagsset = 0;
> }
> diff --git sys/sys/mbuf.h sys/sys/mbuf.h
> index 8d9515c..2ca37eb 100644
> --- sys/sys/mbuf.h
> +++ sys/sys/mbuf.h
> @@ -451,5 +451,13 @@ struct m_tag *m_tag_next(struct mbuf *, struct m_tag *);
> #define PACKET_TAG_PIPEX 0x0400 /* pipex session cache */
> #define PACKET_TAG_PF_REASSEMBLED 0x0800 /* pf reassembled ipv6 packet */
>
> +/*
> + * Maximum tag payload length (that is excluding the m_tag structure).
> + * Please make sure to update this value when increasing the payload
> + * length for an existing packet tag type or when adding a new one that
> + * has payload larger than the value below.
> + */
> +#define PACKET_TAG_MAXSIZE 40
> +
> #endif /* _KERNEL */
> #endif /* _SYS_MBUF_H_ */
> diff --git sys/sys/malloc.h sys/sys/malloc.h
> index 80e4c10..e1be6ae 100644
> --- sys/sys/malloc.h
> +++ sys/sys/malloc.h
> @@ -143,8 +143,7 @@
> #define M_CRYPTO_DATA 108 /* Crypto framework data buffers (keys
> etc.) */
> /* 109 - free */
> #define M_CREDENTIALS 110 /* IPsec-related credentials and ID
> info */
> -#define M_PACKET_TAGS 111 /* Packet-attached information */
> -/* 112-113 - free */
> +/* 111-113 - free */
> #define M_EMULDATA 114 /* Per-process emulation data */
> /* 115-122 - free */
