Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 7 Feb 2001 21:26:00 +0000 (GMT)
From:      Terry Lambert <tlambert@primenet.com>
To:        bp@butya.kz (Boris Popov)
Cc:        freebsd-arch@FreeBSD.ORG, freebsd-fs@FreeBSD.ORG
Subject:   Re: vnode interlock API
Message-ID:  <200102072126.OAA24284@usr08.primenet.com>
In-Reply-To: <Pine.BSF.4.21.0102061638280.82511-100000@lion.butya.kz> from "Boris Popov" at Feb 06, 2001 05:00:03 PM

next in thread | previous in thread | raw e-mail | index | archive | help
> 	So, I suggest to introduce two macro definitions which will hide
> implementation details for interlocks:
> 
> #define VI_LOCK(vp)		mtx_enter(&(vp)->v_interlock, MTX_DEF)
> #define VI_UNLOCK(vp)		mtx_exit(&(vp)->v_interlock, MTX_DEF)
> 
> 	for RELENG_4 they will look like this:
> 
> #define VI_LOCK(vp)		simple_lock(&(vp)->v_interlock)
> #define VI_UNLOCK(vp)		simple_unlock(&(vp)->v_interlock)
> 
> 	Any comments, suggestions ?

1)	Macros are good; interfaces are better.  I've consistantly
	recommended that the NFS cookie interface be rewritten to
	not require cookies, even though the FreeBSD/NetBSD/OpenBSD
	differences _could_ be masked with macros.  The issue is
	one of binary vs. source compatability.

2)	If you are going to wrap vnode handling, it would probably
	be a good idea to wrap it using the same approach that
	another OS uses, instead of being gratuitously different
	in naming.  I would suggest using the Solaris names, but I
	will admit that doing that depends  heavily on the semantics
	being the same (I think they would be).  Worst case, pick an
	OS with the same semantics; if there are none, this may be
	an opportunity to learn from other OSs _why_ they don't have
	the same semantics.

3)	It seems to mee that the additional parameter of MTX_DEF is
	gratuitous, and tries to stretch mutex semantics further
	than they should be stretched.  I personally would have no
	problem with the conversion of simple_{un}lock() into the
	equivalent mtx_*() calls.  Even if the MTX_DEF can not be
	murdered without a large public outcry, using this as the
	the default demantic for the simple_*() equivalents isn't
	really a bad idea, in my book, and could be done with
	inline wrappers.  Best case, one could apply the WITNESS
	code to debugging 4.x problems, with some work.

4)	You need to wrap the calls with "{ ... }"; this is because
	it may be useful in the future to institute turnstile or
	single wakeup semantics, and converting the macro into a
	single statement instead of a statement block would mean
	a potentially large amount of work would be needed to cope
	with the change later, whereas, you seem to plan to already
	need to touch all those spots now.  Again, the Solaris SMP
	vnode lock management macros are, I think, a good example
	(or at least they were, six years ago, when Solaris faced
	the same problem).

I have other comments, but these are the four most important ones,
IMO, and I've been making a conscious effort to not clutter arguments
by giving more detail than people seem to want to hear before they
overflow and tune out.  8-).


					Terry Lambert
					terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-fs" in the body of the message




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