cvs commit: src/sys/contrib/ipfilter/netinet ip_auth.c

Darren Reed darrenr at hub.freebsd.org
Mon Dec 27 21:18:38 PST 2004


On Mon, Dec 27, 2004 at 08:39:44PM -0700, Scott Long wrote:
> The locking APIs have existed in FreeBSD 5.x for 4 years.  They are 
> documented in manual pages, web pages, and publically available USENIX
> papers.  Just because you don't agree that sx locks should be sleepable
> doesn't mean that your opinion is valid.

Hmmm, maybe I'm not communicating my view properly.

I'm not fussed about whether or not sx locks are sleepable.

What's wrong, IMHO, is the witness code and the implication that
acquiring a sleepable lock, while you have a mutex, is wrong.
Or vice versa.  So long as the order is always the same, it
should not present a problem if the programmer is careful.

Even "debug" kernels on Solaris don't have restrictions like that
between mutex/rw-locks, although Solaris never allows you to do
a recursive mutex, like FreeBSD.

FWIW, John Baldwin's USENIX paper that talks about witness from BSDi
only talks about it in context of locking order:
http://www.usenix.org/events/bsdcon02/full_papers/baldwin/baldwin_html/node7.html#SECTION00072000000000000000

The problem it is complaining about w.r.t ipfilter is not what the
paper talks about here. 

As it is, if I were to refer to:
http://sources.zabbadoz.net/freebsd/lor.html
050 - i've actually changed the code for other reasons so it should
      not happen at some point in the future
051 - why is UDP holding on to the lock for so long ?
      this doesn't look like an ipfilter problem.
052 - i'm dumbfounded.  IFNET_RLOCK uses mtx, not sx.

052 is a classic case of why this LOR report is not useful.
The witness code is complaining because a mutex is being acquired
while a sleepable lock is held, for the purpose of walking a linked
list.  I mean WTF?

Just to reiterate, it's the witness code and the semantics it is trying
to enforce (which are somewhat different, now?) that I have an issue
with, not the lock implementations.

Darren


More information about the cvs-all mailing list