Date: Thu, 16 Jul 2020 07:49:17 +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: <20200716074917.04445daa@x23> In-Reply-To: <20200716072622.5fa35ba2@x23> References: <20200715232624.GR4213@funkthat.com> <20200716072622.5fa35ba2@x23>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 16 Jul 2020 07:26:22 +0200 Marko Zec <zec@fer.hr> wrote: > On Wed, 15 Jul 2020 16:26:25 -0700 > John-Mark Gurney <jmg@funkthat.com> wrote: > > > I happen to be looking at the implementation of IFQ_DRV_PREPEND, and > > unlike IFQ_DRV_DEQUEUE, it doesn't obtain the lock when manipulating > > the ifq. I took a brief look at a few drivers, and it looks like > > some of them expect that _PREPEND lock the Q like _DEQUEUE does. > > > > This is likely not an issue often, since it's an error path that > > likely rarely happens, but we should fix it. > > > > Should we just add the IFQ_LOCK/_UNLOCK to the macro? > > > > Comments or thoughts? > > I also have a hard time understanding the semantics of IFQ_DRV_* > macros: per altq(9) they appear to be variants of their IFQ_ > counterparts which are intended to be protected by some other > mechanism rather than IFQ_LOCK, but in some instances this isn't the > case, as they do grab the IFQ_LOCK. Hmm it's not that unclear after looking better: ifq_drv_* parts of struct ifaltq should be protected by some external mechanisms when using IFQ_DRV_ macros, and ifq_ parts are still protected by ifq_mtx. So it doesn't look like IFQ_LOCK/_UNLOCK should be added to IFQ_DRV_PREPEND()...
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200716074917.04445daa>