On Thu, Mar 02, 2017 at 10:11 +0100, Martin Pieuchot wrote:
> On 02/03/17(Thu) 01:16, Mike Belopuhov wrote:
> > On 2 March 2017 at 00:56, David Gwynne <da...@gwynne.id.au> wrote:
> > >
> > >> On 2 Mar 2017, at 06:43, Mike Belopuhov <m...@belopuhov.com> wrote:
> > >>
> > >> This convers hand rolled lists into exactly the same mbuf_lists.
> > >> I need this because of the next diff that uses the ml_len packet
> > >> counter that mbuf_lists have. Otherwise there's no functional
> > >> change.
> > >
> > > i didnt use mbuf lists here because they have an extra counter
> > > that isnt, or wasnt, needed.
> > >
> > > im not sure you need to know how long a list is in your later
> > > diff, you just need to know if it is not empty. you can do that
> > > by checking if the head is NULL.
> > >
> >
> > true, i was thinking about clarifying this, but i like the
> > "declarativeness" of the length check. i don't think that an
> > extra counter is a big deal. also mbuf lists make this code look
> > simpler which is a good thing, imo.
>
> I agree. I find the code much easier to understand with mikeb@'s diff.
>
> IMHO we should avoid hand-rolled lists.
>
> ok mpi@
>
Here's an updated diff with suggestions from bluhm@ and it's
even slimmer now but I don't mind either way. If David isn't
comfortable with this change, we don't have to do it.
---
sys/net/ifq.c | 50 ++++++++++++++++++--------------------------------
1 file changed, 18 insertions(+), 32 deletions(-)
diff --git sys/net/ifq.c sys/net/ifq.c
index 40731637a2c..a601e9b9bd1 100644
--- sys/net/ifq.c
+++ sys/net/ifq.c
@@ -51,17 +51,12 @@ const struct ifq_ops * const ifq_priq_ops = &priq_ops;
/*
* priq internal structures
*/
-struct priq_list {
- struct mbuf *head;
- struct mbuf *tail;
-};
-
struct priq {
- struct priq_list pq_lists[IFQ_NQUEUES];
+ struct mbuf_list pq_lists[IFQ_NQUEUES];
};
/*
* ifqueue serialiser
*/
@@ -392,11 +387,17 @@ priq_idx(unsigned int nqueues, const struct mbuf *m)
}
void *
priq_alloc(unsigned int idx, void *null)
{
- return (malloc(sizeof(struct priq), M_DEVBUF, M_WAITOK | M_ZERO));
+ struct priq *pq;
+ int i;
+
+ pq = malloc(sizeof(struct priq), M_DEVBUF, M_WAITOK);
+ for (i = 0; i < IFQ_NQUEUES; i++)
+ ml_init(&pq->pq_lists[i]);
+ return (pq);
}
void
priq_free(unsigned int idx, void *pq)
{
@@ -405,40 +406,35 @@ priq_free(unsigned int idx, void *pq)
int
priq_enq(struct ifqueue *ifq, struct mbuf *m)
{
struct priq *pq;
- struct priq_list *pl;
+ struct mbuf_list *pl;
if (ifq_len(ifq) >= ifq->ifq_maxlen)
return (ENOBUFS);
pq = ifq->ifq_q;
KASSERT(m->m_pkthdr.pf.prio <= IFQ_MAXPRIO);
pl = &pq->pq_lists[m->m_pkthdr.pf.prio];
- m->m_nextpkt = NULL;
- if (pl->tail == NULL)
- pl->head = m;
- else
- pl->tail->m_nextpkt = m;
- pl->tail = m;
+ ml_enqueue(pl, m);
return (0);
}
struct mbuf *
priq_deq_begin(struct ifqueue *ifq, void **cookiep)
{
struct priq *pq = ifq->ifq_q;
- struct priq_list *pl;
+ struct mbuf_list *pl;
unsigned int prio = nitems(pq->pq_lists);
struct mbuf *m;
do {
pl = &pq->pq_lists[--prio];
- m = pl->head;
+ m = MBUF_LIST_FIRST(pl);
if (m != NULL) {
*cookiep = pl;
return (m);
}
} while (prio > 0);
@@ -447,35 +443,25 @@ priq_deq_begin(struct ifqueue *ifq, void **cookiep)
}
void
priq_deq_commit(struct ifqueue *ifq, struct mbuf *m, void *cookie)
{
- struct priq_list *pl = cookie;
-
- KASSERT(pl->head == m);
+ struct mbuf_list *pl = cookie;
- pl->head = m->m_nextpkt;
- m->m_nextpkt = NULL;
+ KASSERT(MBUF_LIST_FIRST(pl) == m);
- if (pl->head == NULL)
- pl->tail = NULL;
+ ml_dequeue(pl);
}
void
priq_purge(struct ifqueue *ifq, struct mbuf_list *ml)
{
struct priq *pq = ifq->ifq_q;
- struct priq_list *pl;
+ struct mbuf_list *pl;
unsigned int prio = nitems(pq->pq_lists);
- struct mbuf *m, *n;
+ struct mbuf *m;
do {
pl = &pq->pq_lists[--prio];
-
- for (m = pl->head; m != NULL; m = n) {
- n = m->m_nextpkt;
- ml_enqueue(ml, m);
- }
-
- pl->head = pl->tail = NULL;
+ ml_enlist(ml, pl);
} while (prio > 0);
}
--
2.12.0
