On Sun, Apr 25, 2021 at 09:44:16AM +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> I think this should go in as-is. Though I have one question/idea
> to share at the moment.
>
> </snip>
> > @@ -672,20 +666,18 @@ arpcache(struct ifnet *ifp, struct ether
> >
> > la->la_asked = 0;
> > la->la_refreshed = 0;
> > - while ((len = ml_len(&la->la_ml)) != 0) {
> > - struct mbuf *mh;
> > + while ((m = mq_dequeue(&la->la_mq)) != NULL) {
> > + unsigned int len;
> >
> > - mh = ml_dequeue(&la->la_ml);
> > - la_hold_total--;
> > + atomic_dec_int(&la_hold_total);
> > + len = mq_len(&la->la_mq);
> >
> > - ifp->if_output(ifp, mh, rt_key(rt), rt);
> > + ifp->if_output(ifp, m, rt_key(rt), rt);
> >
> > - if (ml_len(&la->la_ml) == len) {
> > + /* XXXSMP we discard if other CPU enqueues */
> > + if (mq_len(&la->la_mq) > len) {
> > /* mbuf is back in queue. Discard. */
> > - while ((mh = ml_dequeue(&la->la_ml)) != NULL) {
> > - la_hold_total--;
> > - m_freem(mh);
> > - }
> > + atomic_sub_int(&la_hold_total, mq_purge(&la->la_mq));
> > break;
> > }
>
> would it make sense to have let's say
>
> mq_move2mlist(struct mbuf_queue *, struct mbuf_list *)
This already exists, it's called mq_delist()
>
> This would allow as to move whole globally visible la->la_mq into
> into mbuf list, which will be a local variable. This way we won't
> need to jump on la->la_mq's mutex with every loop iteration.
>
> I it makes sense, we can do it as a follow up change.
We'd need some other way to do the 'mbuf is back in queue' detection,
but I agree this seems like a sensible thing to do.
>
>
> thanks and
> regards
> sashan
>
