> On 2 Mar 2017, at 21:19, Mike Belopuhov <m...@belopuhov.com> wrote:
>
> 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.
im really not keen, but i guess i will submit to populism if everyone else
feels strongly about this.
>
> ---
> 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
>
