sched_lock && thread_lock()

[Bruce Evans]

On Tue, 22 May 2007, Bruce Evans wrote:

> 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 */

4 more translation errors breaking 8 counters altogether (v_vnodepgsin
is broken twice):

% Index: kern_fork.c
% ===================================================================
% RCS file: /home/ncvs/src/sys/kern/kern_fork.c,v
% retrieving revision 1.270
% retrieving revision 1.271
% diff -u -2 -r1.270 -r1.271
% --- kern_fork.c	4 Apr 2007 09:11:32 -0000	1.270
% +++ kern_fork.c	18 May 2007 07:10:45 -0000	1.271
% @@ -666,18 +666,18 @@
% 
%  	if (flags == (RFFDG | RFPROC)) {
% -		atomic_add_int(&cnt.v_forks, 1);
% -		atomic_add_int(&cnt.v_forkpages, p2->p_vmspace->vm_dsize +
% +		VMCNT_ADD(forks, 1);
% +		VMCNT_ADD(forkpages, p2->p_vmspace->vm_dsize +
%  		    p2->p_vmspace->vm_ssize);
%  	} else if (flags == (RFFDG | RFPROC | RFPPWAIT | RFMEM)) {
% -		atomic_add_int(&cnt.v_vforks, 1);
    		                      ^^^^^^
% -		atomic_add_int(&cnt.v_vforkpages, p2->p_vmspace->vm_dsize +
    		                      ^^^^^^^^^^
% +		VMCNT_ADD(forks, 1);
    		          ^^^^^
% +		VMCNT_ADD(forkpages, p2->p_vmspace->vm_dsize +
    		          ^^^^^^^^^
%  		    p2->p_vmspace->vm_ssize);
%  	} else if (p1 == &proc0) {
% -		atomic_add_int(&cnt.v_kthreads, 1);
% -		atomic_add_int(&cnt.v_kthreadpages, p2->p_vmspace->vm_dsize +
% +		VMCNT_ADD(kthreads, 1);
% +		VMCNT_ADD(kthreadpages, p2->p_vmspace->vm_dsize +
%  		    p2->p_vmspace->vm_ssize);
%  	} else {
% -		atomic_add_int(&cnt.v_rforks, 1);
% -		atomic_add_int(&cnt.v_rforkpages, p2->p_vmspace->vm_dsize +
% +		VMCNT_ADD(rforks, 1);
% +		VMCNT_ADD(rforkpages, p2->p_vmspace->vm_dsize +
%  		    p2->p_vmspace->vm_ssize);
%  	}

% 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
% @@ -1158,6 +1159,6 @@
%  	auio.uio_td = (struct thread *) 0;
%  	error = VOP_WRITE(vp, &auio, ioflags, curthread->td_ucred);
% -	cnt.v_vnodeout++;
    	           ^^^
% -	cnt.v_vnodepgsout += ncount;
    	              ^^^
% +	VMCNT_ADD(vnodein, 1);
    	               ^^
% +	VMCNT_ADD(vnodepgsin, ncount);
    	                  ^^
% 
%  	if (error) {

> 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.

Another problem with using PCU_LAZY_INC() is that all counters might need
to be exported to userland by emulators, but only the vm_meter sysctls
understand PCPU_LAZY_INC().  At least the following counters are currently
exported by at least the linprocfs emulator:

 	v_intr, v_swtch, v_vnodepgsin, v_vnodepgsout

linprocfs accesses all these counters using VMCNT_GET(), but VMCNT_GET()
doesn't work for counters in the pcpu.  v_intr is updated by PCPU_LAZY_INC()
so it is broken in linprocfs.  The others aren't yet updated by
PCPU_LAZY_INC() so they aren't broken in linprocfs (except the above
bugs break v_vnodepgs*).

Apart from these problems and bugs, PCPU_LAZY_INC() should be used for all
of the counters visible in the above patches.

Bruce