Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Jul 2020 23:54:38 +0200
From:      Marko Zec <zec@fer.hr>
To:        John-Mark Gurney <jmg@funkthat.com>
Cc:        freebsd-net@FreeBSD.org
Subject:   Re: IF_DRV_PREPEND unlocked?
Message-ID:  <20200717235438.1fee733b@x23>
In-Reply-To: <20200717185609.GX4213@funkthat.com>
References:  <20200715232624.GR4213@funkthat.com> <20200716072622.5fa35ba2@x23> <20200716074917.04445daa@x23> <20200716185629.GT4213@funkthat.com> <20200717120311.59377e0d@x23> <20200717185609.GX4213@funkthat.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 17 Jul 2020 11:56:09 -0700
John-Mark Gurney <jmg@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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200717235438.1fee733b>