Date: Tue, 22 May 2007 16:45:19 +1000 (EST) From: Bruce Evans <bde@optusnet.com.au> To: Attilio Rao <attilio@freebsd.org> Cc: arch@freebsd.org Subject: Re: sched_lock && thread_lock() Message-ID: <20070522162819.N5249@besplex.bde.org> In-Reply-To: <20070521225032.C57233@delplex.bde.org> References: <20070520155103.K632@10.0.0.1> <20070521113648.F86217@besplex.bde.org> <20070520213132.K632@10.0.0.1> <4651CAB8.8070007@FreeBSD.org> <4651CE2F.8080908@FreeBSD.org> <20070521022847.D679@10.0.0.1> <20070521195811.G56785@delplex.bde.org> <4651FCB5.7070604@FreeBSD.org> <20070521225032.C57233@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 22 May 2007, Bruce Evans wrote:
> Bugs found while grepping for sched_locking of VMCNT fields:
> - v_trap is supposed to be updated by PCPU_LAZY_INC(), but subr_trap.c
> updates it by VMCNT_ADD(). The direct access used to be perfectly unlocked
> and non-atomic, since it occurs immediately after releasing sched_lock
> and just used "++". Now it is atomic and a micro-pessimization.
> - v_trap's name is now hidden by the macro so grepping for it doesn't work.
I'm still searching for a 1% performance loss in makeworld under UP
that occurred just after the VMCNT changes. I've only found this
harmless (?) bug so far:
% Index: vnode_pager.c
% ===================================================================
% RCS file: /home/ncvs/src/sys/vm/vnode_pager.c,v
% retrieving revision 1.232
% retrieving revision 1.233
% diff -u -2 -r1.232 -r1.233
% --- vnode_pager.c 14 Oct 2006 23:21:48 -0000 1.232
% +++ vnode_pager.c 18 May 2007 07:10:50 -0000 1.233
% @@ -910,6 +910,6 @@
% atomic_add_int(&runningbufspace, bp->b_runningbufspace);
%
% - cnt.v_vnodein++;
% - cnt.v_vnodepgsin += count;
^^^^^
% + VMCNT_ADD(vnodein, 1);
% + VMCNT_ADD(vnodepgsin, 1);
^
%
% /* do the input */
Related API problem: v_vnodein and v_vnodepgsin are not used in the kernel
except to increment them and to export them using sysctl, so they should
be incremented using PCPY_LAZY_INC(). However, PCPU_LAZY_INC() can only
do increments of 1.
The increments of +1 also complicate searching for the regression. After
removing `volatile', the VMCNT change should have no effect on the binary,
but it has an effect since increments of 1 are replaced by "movl $1,%reg;
addl %reg,mem" in i386 code, since the i386 atomic asms don't use incl
and the "ir" constraint isn't working as intended (this is with gcc-3.4.6).
The "ir" constraint should result in the "i" variant being chosen and
generate "addl $1,mem".
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070522162819.N5249>