* [Qemu-devel] [PATCH] Avoid embedding struct mbuf in other structures
@ 2016-02-22 21:29 Samuel Thibault
2016-02-25 15:22 ` Peter Maydell
0 siblings, 1 reply; 4+ messages in thread
From: Samuel Thibault @ 2016-02-22 21:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Jan Kiszka, Jason Wang, Samuel Thibault, Peter Maydell
struct mbuf uses a C99 open char array to allow inlining data. Inlining
this in another structure is however a GNU extension. The inlines used
so far in struct Slirp were actually only needed as head of struct
mbuf lists. This replaces these inline with mere struct quehead,
and use casts as appropriate.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
slirp/if.c | 27 ++++++++++++++-------------
slirp/mbuf.c | 19 ++++++++++---------
slirp/misc.c | 5 -----
slirp/misc.h | 5 +++++
slirp/slirp.h | 8 +++++---
5 files changed, 34 insertions(+), 30 deletions(-)
diff --git a/slirp/if.c b/slirp/if.c
index 93d7cc0..ce381ad 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -28,9 +28,9 @@ ifs_remque(struct mbuf *ifm)
void
if_init(Slirp *slirp)
{
- slirp->if_fastq.ifq_next = slirp->if_fastq.ifq_prev = &slirp->if_fastq;
- slirp->if_batchq.ifq_next = slirp->if_batchq.ifq_prev = &slirp->if_batchq;
- slirp->next_m = &slirp->if_batchq;
+ slirp->if_fastq.qh_link = slirp->if_fastq.qh_rlink = &slirp->if_fastq;
+ slirp->if_batchq.qh_link = slirp->if_batchq.qh_rlink = &slirp->if_batchq;
+ slirp->next_m = (struct mbuf *) &slirp->if_batchq;
}
/*
@@ -74,7 +74,8 @@ if_output(struct socket *so, struct mbuf *ifm)
* We mustn't put this packet back on the fastq (or we'll send it out of order)
* XXX add cache here?
*/
- for (ifq = slirp->if_batchq.ifq_prev; ifq != &slirp->if_batchq;
+ for (ifq = (struct mbuf *) slirp->if_batchq.qh_rlink;
+ (struct quehead *) ifq != &slirp->if_batchq;
ifq = ifq->ifq_prev) {
if (so == ifq->ifq_so) {
/* A match! */
@@ -86,7 +87,7 @@ if_output(struct socket *so, struct mbuf *ifm)
/* No match, check which queue to put it on */
if (so && (so->so_iptos & IPTOS_LOWDELAY)) {
- ifq = slirp->if_fastq.ifq_prev;
+ ifq = (struct mbuf *) slirp->if_fastq.qh_rlink;
on_fastq = 1;
/*
* Check if this packet is a part of the last
@@ -98,9 +99,9 @@ if_output(struct socket *so, struct mbuf *ifm)
goto diddit;
}
} else {
- ifq = slirp->if_batchq.ifq_prev;
+ ifq = (struct mbuf *) slirp->if_batchq.qh_rlink;
/* Set next_m if the queue was empty so far */
- if (slirp->next_m == &slirp->if_batchq) {
+ if ((struct quehead *) slirp->next_m == &slirp->if_batchq) {
slirp->next_m = ifm;
}
}
@@ -166,10 +167,10 @@ void if_start(Slirp *slirp)
}
slirp->if_start_busy = true;
- if (slirp->if_fastq.ifq_next != &slirp->if_fastq) {
- ifm_next = slirp->if_fastq.ifq_next;
+ if (slirp->if_fastq.qh_link != &slirp->if_fastq) {
+ ifm_next = (struct mbuf *) slirp->if_fastq.qh_link;
next_from_batchq = false;
- } else if (slirp->next_m != &slirp->if_batchq) {
+ } else if ((struct quehead *) slirp->next_m != &slirp->if_batchq) {
/* Nothing on fastq, pick up from batchq via next_m */
ifm_next = slirp->next_m;
next_from_batchq = true;
@@ -182,12 +183,12 @@ void if_start(Slirp *slirp)
from_batchq = next_from_batchq;
ifm_next = ifm->ifq_next;
- if (ifm_next == &slirp->if_fastq) {
+ if ((struct quehead *) ifm_next == &slirp->if_fastq) {
/* No more packets in fastq, switch to batchq */
ifm_next = slirp->next_m;
next_from_batchq = true;
}
- if (ifm_next == &slirp->if_batchq) {
+ if ((struct quehead *) ifm_next == &slirp->if_batchq) {
/* end of batchq */
ifm_next = NULL;
}
@@ -218,7 +219,7 @@ void if_start(Slirp *slirp)
/* Next packet in fastq is from the same session */
ifm_next = next;
next_from_batchq = false;
- } else if (slirp->next_m == &slirp->if_batchq) {
+ } else if ((struct quehead *) slirp->next_m == &slirp->if_batchq) {
/* Set next_m and ifm_next if the session packet is now the
* only one on batchq */
slirp->next_m = ifm_next = next;
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index c959758..a718e08 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -29,16 +29,16 @@
void
m_init(Slirp *slirp)
{
- slirp->m_freelist.m_next = slirp->m_freelist.m_prev = &slirp->m_freelist;
- slirp->m_usedlist.m_next = slirp->m_usedlist.m_prev = &slirp->m_usedlist;
+ slirp->m_freelist.qh_link = slirp->m_freelist.qh_rlink = &slirp->m_freelist;
+ slirp->m_usedlist.qh_link = slirp->m_usedlist.qh_rlink = &slirp->m_usedlist;
}
void m_cleanup(Slirp *slirp)
{
struct mbuf *m, *next;
- m = slirp->m_usedlist.m_next;
- while (m != &slirp->m_usedlist) {
+ m = (struct mbuf *) slirp->m_usedlist.qh_link;
+ while ((struct quehead *) m != &slirp->m_usedlist) {
next = m->m_next;
if (m->m_flags & M_EXT) {
free(m->m_ext);
@@ -46,8 +46,8 @@ void m_cleanup(Slirp *slirp)
free(m);
m = next;
}
- m = slirp->m_freelist.m_next;
- while (m != &slirp->m_freelist) {
+ m = (struct mbuf *) slirp->m_freelist.qh_link;
+ while ((struct quehead *) m != &slirp->m_freelist) {
next = m->m_next;
free(m);
m = next;
@@ -70,7 +70,7 @@ m_get(Slirp *slirp)
DEBUG_CALL("m_get");
- if (slirp->m_freelist.m_next == &slirp->m_freelist) {
+ if (slirp->m_freelist.qh_link == &slirp->m_freelist) {
m = (struct mbuf *)malloc(SLIRP_MSIZE);
if (m == NULL) goto end_error;
slirp->mbuf_alloced++;
@@ -78,7 +78,7 @@ m_get(Slirp *slirp)
flags = M_DOFREE;
m->slirp = slirp;
} else {
- m = slirp->m_freelist.m_next;
+ m = (struct mbuf *) slirp->m_freelist.qh_link;
remque(m);
}
@@ -225,7 +225,8 @@ dtom(Slirp *slirp, void *dat)
DEBUG_ARG("dat = %p", dat);
/* bug corrected for M_EXT buffers */
- for (m = slirp->m_usedlist.m_next; m != &slirp->m_usedlist;
+ for (m = (struct mbuf *) slirp->m_usedlist.qh_link;
+ (struct quehead *) m != &slirp->m_usedlist;
m = m->m_next) {
if (m->m_flags & M_EXT) {
if( (char *)dat>=m->m_ext && (char *)dat<(m->m_ext + m->m_size) )
diff --git a/slirp/misc.c b/slirp/misc.c
index e2eea2e..2fbd048 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -17,11 +17,6 @@
int slirp_debug = DBG_CALL|DBG_MISC|DBG_ERROR;
#endif
-struct quehead {
- struct quehead *qh_link;
- struct quehead *qh_rlink;
-};
-
inline void
insque(void *a, void *b)
{
diff --git a/slirp/misc.h b/slirp/misc.h
index 41a3258..0d0c059 100644
--- a/slirp/misc.h
+++ b/slirp/misc.h
@@ -45,6 +45,11 @@ struct emu_t {
struct emu_t *next;
};
+struct slirp_quehead {
+ struct slirp_quehead *qh_link;
+ struct slirp_quehead *qh_rlink;
+};
+
void slirp_insque(void *, void *);
void slirp_remque(void *);
int add_exec(struct ex_list **, int, char *, struct in_addr, int);
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 239fe29..591899c 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -126,6 +126,7 @@ void free(void *ptr);
have different prototypes. */
#define insque slirp_insque
#define remque slirp_remque
+#define quehead slirp_quehead
#ifdef HAVE_SYS_STROPTS_H
#include <sys/stropts.h>
@@ -216,12 +217,13 @@ struct Slirp {
struct ex_list *exec_list;
/* mbuf states */
- struct mbuf m_freelist, m_usedlist;
+ struct quehead m_freelist;
+ struct quehead m_usedlist;
int mbuf_alloced;
/* if states */
- struct mbuf if_fastq; /* fast queue (for interactive data) */
- struct mbuf if_batchq; /* queue for non-interactive data */
+ struct quehead if_fastq; /* fast queue (for interactive data) */
+ struct quehead if_batchq; /* queue for non-interactive data */
struct mbuf *next_m; /* pointer to next mbuf to output */
bool if_start_busy; /* avoid if_start recursion */
--
2.7.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] Avoid embedding struct mbuf in other structures
2016-02-22 21:29 [Qemu-devel] [PATCH] Avoid embedding struct mbuf in other structures Samuel Thibault
@ 2016-02-25 15:22 ` Peter Maydell
2016-02-25 15:29 ` Samuel Thibault
0 siblings, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2016-02-25 15:22 UTC (permalink / raw)
To: Samuel Thibault; +Cc: Jan Kiszka, Jason Wang, QEMU Developers
On 22 February 2016 at 21:29, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
> struct mbuf uses a C99 open char array to allow inlining data.
At the moment it doesn't, it uses a 1 byte array :-) Turning that
into a C99 flexible array is what we'd like to do...
> Inlining
> this in another structure is however a GNU extension. The inlines used
> so far in struct Slirp were actually only needed as head of struct
> mbuf lists. This replaces these inline with mere struct quehead,
> and use casts as appropriate.
>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> slirp/if.c | 27 ++++++++++++++-------------
> slirp/mbuf.c | 19 ++++++++++---------
> slirp/misc.c | 5 -----
> slirp/misc.h | 5 +++++
> slirp/slirp.h | 8 +++++---
> 5 files changed, 34 insertions(+), 30 deletions(-)
It's a slight shame that we end up having to have the extra casts,
but I guess it can't be helped. Converting the code to use one of
the qemu/queue.h data structures doesn't seem worth the effort
and risk of introducing bugs.
(Does this pass checkpatch.pl? I wasn't sure about the spacing
round some of the casts...)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] Avoid embedding struct mbuf in other structures
2016-02-25 15:22 ` Peter Maydell
@ 2016-02-25 15:29 ` Samuel Thibault
0 siblings, 0 replies; 4+ messages in thread
From: Samuel Thibault @ 2016-02-25 15:29 UTC (permalink / raw)
To: Peter Maydell; +Cc: Jan Kiszka, Jason Wang, QEMU Developers
Peter Maydell, on Thu 25 Feb 2016 15:22:25 +0000, wrote:
> On 22 February 2016 at 21:29, Samuel Thibault
> <samuel.thibault@ens-lyon.org> wrote:
> > struct mbuf uses a C99 open char array to allow inlining data.
>
> At the moment it doesn't, it uses a 1 byte array :-) Turning that
> into a C99 flexible array is what we'd like to do...
I know :)
> It's a slight shame that we end up having to have the extra casts,
> but I guess it can't be helped.
Yes.
> Converting the code to use one of the qemu/queue.h data structures
> doesn't seem worth the effort and risk of introducing bugs.
Yes :)
> (Does this pass checkpatch.pl? I wasn't sure about the spacing
> round some of the casts...)
It does, except warnings about indent, which we probably don't want to
fix here.
Samuel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH] Avoid embedding struct mbuf in other structures
@ 2016-02-25 15:30 Samuel Thibault
0 siblings, 0 replies; 4+ messages in thread
From: Samuel Thibault @ 2016-02-25 15:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Jan Kiszka, Jason Wang, Samuel Thibault, Peter Maydell
struct mbuf uses an open char array to allow inlining data, and we aim at
using a C99 flexible array. Inlining this in another structure is however a
GNU extension. The inlines used so far in struct Slirp were actually only
needed as head of struct mbuf lists. This replaces these inline with mere
struct quehead, and use casts as appropriate.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
slirp/if.c | 27 ++++++++++++++-------------
slirp/mbuf.c | 19 ++++++++++---------
slirp/misc.c | 5 -----
slirp/misc.h | 5 +++++
slirp/slirp.h | 8 +++++---
5 files changed, 34 insertions(+), 30 deletions(-)
diff --git a/slirp/if.c b/slirp/if.c
index 93d7cc0..ce381ad 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -28,9 +28,9 @@ ifs_remque(struct mbuf *ifm)
void
if_init(Slirp *slirp)
{
- slirp->if_fastq.ifq_next = slirp->if_fastq.ifq_prev = &slirp->if_fastq;
- slirp->if_batchq.ifq_next = slirp->if_batchq.ifq_prev = &slirp->if_batchq;
- slirp->next_m = &slirp->if_batchq;
+ slirp->if_fastq.qh_link = slirp->if_fastq.qh_rlink = &slirp->if_fastq;
+ slirp->if_batchq.qh_link = slirp->if_batchq.qh_rlink = &slirp->if_batchq;
+ slirp->next_m = (struct mbuf *) &slirp->if_batchq;
}
/*
@@ -74,7 +74,8 @@ if_output(struct socket *so, struct mbuf *ifm)
* We mustn't put this packet back on the fastq (or we'll send it out of order)
* XXX add cache here?
*/
- for (ifq = slirp->if_batchq.ifq_prev; ifq != &slirp->if_batchq;
+ for (ifq = (struct mbuf *) slirp->if_batchq.qh_rlink;
+ (struct quehead *) ifq != &slirp->if_batchq;
ifq = ifq->ifq_prev) {
if (so == ifq->ifq_so) {
/* A match! */
@@ -86,7 +87,7 @@ if_output(struct socket *so, struct mbuf *ifm)
/* No match, check which queue to put it on */
if (so && (so->so_iptos & IPTOS_LOWDELAY)) {
- ifq = slirp->if_fastq.ifq_prev;
+ ifq = (struct mbuf *) slirp->if_fastq.qh_rlink;
on_fastq = 1;
/*
* Check if this packet is a part of the last
@@ -98,9 +99,9 @@ if_output(struct socket *so, struct mbuf *ifm)
goto diddit;
}
} else {
- ifq = slirp->if_batchq.ifq_prev;
+ ifq = (struct mbuf *) slirp->if_batchq.qh_rlink;
/* Set next_m if the queue was empty so far */
- if (slirp->next_m == &slirp->if_batchq) {
+ if ((struct quehead *) slirp->next_m == &slirp->if_batchq) {
slirp->next_m = ifm;
}
}
@@ -166,10 +167,10 @@ void if_start(Slirp *slirp)
}
slirp->if_start_busy = true;
- if (slirp->if_fastq.ifq_next != &slirp->if_fastq) {
- ifm_next = slirp->if_fastq.ifq_next;
+ if (slirp->if_fastq.qh_link != &slirp->if_fastq) {
+ ifm_next = (struct mbuf *) slirp->if_fastq.qh_link;
next_from_batchq = false;
- } else if (slirp->next_m != &slirp->if_batchq) {
+ } else if ((struct quehead *) slirp->next_m != &slirp->if_batchq) {
/* Nothing on fastq, pick up from batchq via next_m */
ifm_next = slirp->next_m;
next_from_batchq = true;
@@ -182,12 +183,12 @@ void if_start(Slirp *slirp)
from_batchq = next_from_batchq;
ifm_next = ifm->ifq_next;
- if (ifm_next == &slirp->if_fastq) {
+ if ((struct quehead *) ifm_next == &slirp->if_fastq) {
/* No more packets in fastq, switch to batchq */
ifm_next = slirp->next_m;
next_from_batchq = true;
}
- if (ifm_next == &slirp->if_batchq) {
+ if ((struct quehead *) ifm_next == &slirp->if_batchq) {
/* end of batchq */
ifm_next = NULL;
}
@@ -218,7 +219,7 @@ void if_start(Slirp *slirp)
/* Next packet in fastq is from the same session */
ifm_next = next;
next_from_batchq = false;
- } else if (slirp->next_m == &slirp->if_batchq) {
+ } else if ((struct quehead *) slirp->next_m == &slirp->if_batchq) {
/* Set next_m and ifm_next if the session packet is now the
* only one on batchq */
slirp->next_m = ifm_next = next;
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index c959758..a718e08 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -29,16 +29,16 @@
void
m_init(Slirp *slirp)
{
- slirp->m_freelist.m_next = slirp->m_freelist.m_prev = &slirp->m_freelist;
- slirp->m_usedlist.m_next = slirp->m_usedlist.m_prev = &slirp->m_usedlist;
+ slirp->m_freelist.qh_link = slirp->m_freelist.qh_rlink = &slirp->m_freelist;
+ slirp->m_usedlist.qh_link = slirp->m_usedlist.qh_rlink = &slirp->m_usedlist;
}
void m_cleanup(Slirp *slirp)
{
struct mbuf *m, *next;
- m = slirp->m_usedlist.m_next;
- while (m != &slirp->m_usedlist) {
+ m = (struct mbuf *) slirp->m_usedlist.qh_link;
+ while ((struct quehead *) m != &slirp->m_usedlist) {
next = m->m_next;
if (m->m_flags & M_EXT) {
free(m->m_ext);
@@ -46,8 +46,8 @@ void m_cleanup(Slirp *slirp)
free(m);
m = next;
}
- m = slirp->m_freelist.m_next;
- while (m != &slirp->m_freelist) {
+ m = (struct mbuf *) slirp->m_freelist.qh_link;
+ while ((struct quehead *) m != &slirp->m_freelist) {
next = m->m_next;
free(m);
m = next;
@@ -70,7 +70,7 @@ m_get(Slirp *slirp)
DEBUG_CALL("m_get");
- if (slirp->m_freelist.m_next == &slirp->m_freelist) {
+ if (slirp->m_freelist.qh_link == &slirp->m_freelist) {
m = (struct mbuf *)malloc(SLIRP_MSIZE);
if (m == NULL) goto end_error;
slirp->mbuf_alloced++;
@@ -78,7 +78,7 @@ m_get(Slirp *slirp)
flags = M_DOFREE;
m->slirp = slirp;
} else {
- m = slirp->m_freelist.m_next;
+ m = (struct mbuf *) slirp->m_freelist.qh_link;
remque(m);
}
@@ -225,7 +225,8 @@ dtom(Slirp *slirp, void *dat)
DEBUG_ARG("dat = %p", dat);
/* bug corrected for M_EXT buffers */
- for (m = slirp->m_usedlist.m_next; m != &slirp->m_usedlist;
+ for (m = (struct mbuf *) slirp->m_usedlist.qh_link;
+ (struct quehead *) m != &slirp->m_usedlist;
m = m->m_next) {
if (m->m_flags & M_EXT) {
if( (char *)dat>=m->m_ext && (char *)dat<(m->m_ext + m->m_size) )
diff --git a/slirp/misc.c b/slirp/misc.c
index e2eea2e..2fbd048 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -17,11 +17,6 @@
int slirp_debug = DBG_CALL|DBG_MISC|DBG_ERROR;
#endif
-struct quehead {
- struct quehead *qh_link;
- struct quehead *qh_rlink;
-};
-
inline void
insque(void *a, void *b)
{
diff --git a/slirp/misc.h b/slirp/misc.h
index 41a3258..0d0c059 100644
--- a/slirp/misc.h
+++ b/slirp/misc.h
@@ -45,6 +45,11 @@ struct emu_t {
struct emu_t *next;
};
+struct slirp_quehead {
+ struct slirp_quehead *qh_link;
+ struct slirp_quehead *qh_rlink;
+};
+
void slirp_insque(void *, void *);
void slirp_remque(void *);
int add_exec(struct ex_list **, int, char *, struct in_addr, int);
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 239fe29..591899c 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -126,6 +126,7 @@ void free(void *ptr);
have different prototypes. */
#define insque slirp_insque
#define remque slirp_remque
+#define quehead slirp_quehead
#ifdef HAVE_SYS_STROPTS_H
#include <sys/stropts.h>
@@ -216,12 +217,13 @@ struct Slirp {
struct ex_list *exec_list;
/* mbuf states */
- struct mbuf m_freelist, m_usedlist;
+ struct quehead m_freelist;
+ struct quehead m_usedlist;
int mbuf_alloced;
/* if states */
- struct mbuf if_fastq; /* fast queue (for interactive data) */
- struct mbuf if_batchq; /* queue for non-interactive data */
+ struct quehead if_fastq; /* fast queue (for interactive data) */
+ struct quehead if_batchq; /* queue for non-interactive data */
struct mbuf *next_m; /* pointer to next mbuf to output */
bool if_start_busy; /* avoid if_start recursion */
--
2.7.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-25 15:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-22 21:29 [Qemu-devel] [PATCH] Avoid embedding struct mbuf in other structures Samuel Thibault
2016-02-25 15:22 ` Peter Maydell
2016-02-25 15:29 ` Samuel Thibault
2016-02-25 15:30 Samuel Thibault
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.