Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 22 Nov 2014 11:25:27 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Mateusz Guzik <mjguzik@gmail.com>
Cc:        freebsd-fs@freebsd.org
Subject:   Re: atomic v_usecount and v_holdcnt
Message-ID:  <20141122092527.GT17068@kib.kiev.ua>
In-Reply-To: <20141122002812.GA32289@dft-labs.eu>
References:  <20141122002812.GA32289@dft-labs.eu>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Nov 22, 2014 at 01:28:12AM +0100, Mateusz Guzik wrote:
> The idea is that we don't need an interlock as long as we don't
> transition either counter 1->0 or 0->1.
> 
> Patch itself is more of a PoC, so I didn't rename vholdl & friends just
> yet.
> 
> It helps during lookups with same vnodes since the interlock which was
> taken twice served as a serializatin point and this effect is now
> reduced.
> 
> There are other places which can avoid VI_LOCK + vget scheme.
> 
> Patch below survived make -j 40 buildworld, poudriere with 40 workers
> etc on a 2 package(s) x 10 core(s) x 2 SMT threads machine with and
> without debugs (including DEBUG_VFS_LOCKS).
> 
> Perf difference:
> 
> in a crap microbenchmark of 40 threads doing a stat on
> /foo/bar/baz/quux${N}, where each thread stats a separate file I got
> over 4 times speed up on tmpfs.
> 
> Comments?

I already said that something along the lines of the patch should work.
In fact, you need vnode lock when hold count changes between 0 and 1,
and probably the same for use count.

Some notes about the patch.

mtx_owned() braces are untolerable ugliness. You should either pass a
boolean flag (preferred), or create locked/unlocked versions of the
functions.

Similarly, I dislike vget_held().  Add a flag to vget(), see LK_EATTR_MASK
in sys/lockmgr.h.

Could there be consequences of not taking vnode interlock and passing
LK_INTERLOCK to vn_lock() in vget() ?

Taking interlock when vnode lock is already owned is probably fine and
does not add to contention. I mean that making VI_OWEINACT so loose
breaks the VOP_INACTIVE() contract.



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