Date: Tue, 02 Jul 2013 19:48:35 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: Konstantin Belousov <kib@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: should_yield problem [Was: svn commit: r251322 - head/sys/kern] Message-ID: <51D30463.50608@FreeBSD.org> In-Reply-To: <201306031736.r53Hain5093431@svn.freebsd.org> References: <201306031736.r53Hain5093431@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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=<value optimized out>, 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=<value optimized out>, uap=<value optimized out>) 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? <roger.pau@citrix.com> > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?51D30463.50608>