From owner-svn-src-head@FreeBSD.ORG Tue Jul 2 16:49:35 2013 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id B8295E04; Tue, 2 Jul 2013 16:49:35 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 368AB1978; Tue, 2 Jul 2013 16:49:33 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id TAA24641; Tue, 02 Jul 2013 19:49:32 +0300 (EEST) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1Uu3lj-00057V-W8; Tue, 02 Jul 2013 19:49:32 +0300 Message-ID: <51D30463.50608@FreeBSD.org> Date: Tue, 02 Jul 2013 19:48:35 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130405 Thunderbird/17.0.5 MIME-Version: 1.0 To: Konstantin Belousov Subject: should_yield problem [Was: svn commit: r251322 - head/sys/kern] References: <201306031736.r53Hain5093431@svn.freebsd.org> In-Reply-To: <201306031736.r53Hain5093431@svn.freebsd.org> X-Enigmail-Version: 1.5.1 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Jul 2013 16:49:35 -0000 on 03/06/2013 20:36 Konstantin Belousov said the following: > Author: kib > Date: Mon Jun 3 17:36:43 2013 > New Revision: 251322 > URL: http://svnweb.freebsd.org/changeset/base/251322 > > Log: > Be more generous when donating the current thread time to the owner of > the vnode lock while iterating over the free vnode list. Instead of > yielding, pause for 1 tick. The change is reported to help in some > virtualized environments. Kostik, I've just run into a problem where word "generous" does not seem to be applicable at all unless I am mistaken about how the code works. There is a thread in vdropl() call that has a vnode interlock and want to take vnode_free_list_mtx. Then there is a thread that keeps cycling on the locked interlock while holding vnode_free_list_mtx: #0 cpustop_handler () at /usr/src/sys/amd64/amd64/mp_machdep.c:1391 #1 0xffffffff80cc8e1d in ipi_nmi_handler () at /usr/src/sys/amd64/amd64/mp_machdep.c:1373 #2 0xffffffff80cd6269 in trap (frame=0xffffff80002baf30) at /usr/src/sys/amd64/amd64/trap.c:211 #3 0xffffffff80cbf86f in nmi_calltrap () at /usr/src/sys/amd64/amd64/exception.S:501 #4 0xffffffff80924f22 in _mtx_trylock (m=0xfffffe00236f4c98, opts=0, file=, line=0) at atomic.h:160 #5 0xffffffff809d3e5a in mnt_vnode_next_active (mvp=0xffffff86c95e4958, mp=0xfffffe00118039a8) at /usr/src/sys/kern/vfs_subr.c:4813 #6 0xffffffff809daa86 in vfs_msync (mp=0xfffffe00118039a8, flags=2) at /usr/src/sys/kern/vfs_subr.c:3542 #7 0xffffffff809e06ef in sys_sync (td=, uap=) at /usr/src/sys/kern/vfs_syscalls.c:149 #8 0xffffffff80cd515a in amd64_syscall (td=0xfffffe019d966490, traced=0) at subr_syscall.c:135 #9 0xffffffff80cbf717 in Xfast_syscall () at /usr/src/sys/amd64/amd64/exception.S:387 Now the curious part: (kgdb) p td->td_swvoltick $4 = 0 (kgdb) p td->td_ru.ru_nvcsw $7 = 0 ticks happen to be in negative territory: (kgdb) p ticks $2 = -1946084020 Give this definition: int should_yield(void) { return (ticks - curthread->td_swvoltick >= hogticks); } We see that should_yield() is going to return false until ticks roll over to become positive again. And obviously with the thread spinning on VI_TRYLOCK() it's not going to get its td_swvoltick updated. I am not sure if the originally reported problem was also caused by should_yield() or if it was something else. But in either case I think that we should fix should_yield. Perhaps (ticks - curthread->td_swvoltick) should be cast to unsigned before comparison? > Submitted by: Roger Pau Monn? > Discussed with: jilles > Tested by: pho > MFC after: 2 weeks > > Modified: > head/sys/kern/vfs_subr.c > > Modified: head/sys/kern/vfs_subr.c > ============================================================================== > --- head/sys/kern/vfs_subr.c Mon Jun 3 17:36:26 2013 (r251321) > +++ head/sys/kern/vfs_subr.c Mon Jun 3 17:36:43 2013 (r251322) > @@ -4693,7 +4693,7 @@ restart: > if (mp_ncpus == 1 || should_yield()) { > TAILQ_INSERT_BEFORE(vp, *mvp, v_actfreelist); > mtx_unlock(&vnode_free_list_mtx); > - kern_yield(PRI_USER); > + pause("vnacti", 1); > mtx_lock(&vnode_free_list_mtx); > goto restart; > } > -- Andriy Gapon