IF_DRV_PREPEND unlocked?

Marko Zec zec at fer.hr
Fri Jul 17 21:54:47 UTC 2020


On Fri, 17 Jul 2020 11:56:09 -0700
John-Mark Gurney <jmg at funkthat.com> wrote:
> Marko Zec wrote this message on Fri, Jul 17, 2020 at 12:03 +0200:
...
> > #define IFQ_DRV_IS_EMPTY(ifq) \
> >     (((ifq)->ifq_drv_len == 0) && ((ifq)->ifq_len == 0))
> > 
> > So, if per altq(9) the contract is that with IFQ_DRV_* the ifq_drv_*
> > fields should be protected by some caller-provided mechanism, while
> > the other ifq_* fields will be implictly protected by ifq_mtx, how
> > can accessing ifw_len without holding ifq_mtx in the above example
> > be safe?  
> 
> Reading is safe when you aren't modifying it, and only using it to
> inform if you should recheck w/ a lock...
> 
> This way a driver can do:
> 	if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
> 		mtx_lock(sc->sc_mtx);
> 		for (;;) {
> 			IFQ_DRV_DEQUEUE(&ifp->if_snd, m);
> 			if (m == NULL)
> 				break;
> 			sendpkt(m);
> 		}
> 		mtx_unlock(sc->sc_mtx);
> 	}
> 
> which saves an expensive lock/unlock op when there are no packets
> in the queue...

The above snippet is fine even if IFQ_DRV_IS_EMPTY() returns 0 during a
race with another thread: per altq(9) the subsequent (then properly
locked) IFQ_DRV_DEQUEUE() might find out that the queue is actually
empty, and bail out if (m == NULL).

But what if IFQ_DRV_IS_EMPTY() returns 1 due to a race with another
thread which has just executed IFQ_ENQUEUE() (invisible to the first
thread due to lack of synchronization), and therefore leaves the mbuf in
the queue, instead of dequeuing and processing it?

Marko


More information about the freebsd-net mailing list