From owner-svn-src-all@freebsd.org Fri Aug 14 11:32:58 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 56F199B8E2E; Fri, 14 Aug 2015 11:32:58 +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 9894D1674; Fri, 14 Aug 2015 11:32:56 +0000 (UTC) (envelope-from avg@FreeBSD.org) 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 OAA14478; Fri, 14 Aug 2015 14:32:54 +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 1ZQDEE-0002kP-Gt; Fri, 14 Aug 2015 14:32:54 +0300 Subject: Re: svn commit: r286623 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs To: Alexander Motin , src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org References: <201508110918.t7B9IpGA039354@repo.freebsd.org> From: Andriy Gapon X-Enigmail-Draft-Status: N1110 Message-ID: <55CDD1AA.40008@FreeBSD.org> Date: Fri, 14 Aug 2015 14:31:54 +0300 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <201508110918.t7B9IpGA039354@repo.freebsd.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Aug 2015 11:32:58 -0000 On 11/08/2015 12:18, Alexander Motin wrote: > Author: mav > Date: Tue Aug 11 09:18:51 2015 > New Revision: 286623 > URL: https://svnweb.freebsd.org/changeset/base/286623 > > Log: > Remove extra lock, that IMO only creates potential problems now. > > Modified: > head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c In my tree I dropped arc_reclaim_thr_lock from arc_lowmem(). Because of that I had to drop msleep() as well. I encountered the following deadlock. Thread X does I/O through ARC, it has got an ARC hash lock, it needs to allocate some memory, but the system has grown low on memory, so the thread has to wait on the page daemon to page some memory. The ARC eviction thread is already running, it holds arc_reclaim_thr_lock and it got blocked on the hash lock held by thread X. The page daemon thread invokes the low memory hook and enters arc_lowmem() where it gets blocked on arc_reclaim_thr_lock() and thus no pages can be paged out. I would prefer to have the pageout path free of indefinite waiting. > Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c > ============================================================================== > --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Tue Aug 11 09:00:27 2015 (r286622) > +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Tue Aug 11 09:18:51 2015 (r286623) > @@ -4773,7 +4773,6 @@ arc_tempreserve_space(uint64_t reserve, > return (0); > } > > -static kmutex_t arc_lowmem_lock; > #ifdef _KERNEL > static eventhandler_tag arc_event_lowmem = NULL; > > @@ -4781,8 +4780,6 @@ static void > arc_lowmem(void *arg __unused, int howto __unused) > { > > - /* Serialize access via arc_lowmem_lock. */ > - mutex_enter(&arc_lowmem_lock); > mutex_enter(&arc_reclaim_thr_lock); > needfree = 1; > DTRACE_PROBE(arc__needfree); > @@ -4793,12 +4790,9 @@ arc_lowmem(void *arg __unused, int howto > * here from ARC itself and may hold ARC locks and thus risk a deadlock > * with ARC reclaim thread. > */ > - if (curproc == pageproc) { > - while (needfree) > - msleep(&needfree, &arc_reclaim_thr_lock, 0, "zfs:lowmem", 0); > - } > + if (curproc == pageproc) > + msleep(&needfree, &arc_reclaim_thr_lock, 0, "zfs:lowmem", 0); > mutex_exit(&arc_reclaim_thr_lock); > - mutex_exit(&arc_lowmem_lock); > } > #endif > > @@ -4809,7 +4803,6 @@ arc_init(void) > > mutex_init(&arc_reclaim_thr_lock, NULL, MUTEX_DEFAULT, NULL); > cv_init(&arc_reclaim_thr_cv, NULL, CV_DEFAULT, NULL); > - mutex_init(&arc_lowmem_lock, NULL, MUTEX_DEFAULT, NULL); > > /* Convert seconds to clock ticks */ > arc_min_prefetch_lifespan = 1 * hz; > @@ -5049,7 +5042,6 @@ arc_fini(void) > > ASSERT0(arc_loaned_bytes); > > - mutex_destroy(&arc_lowmem_lock); > #ifdef _KERNEL > if (arc_event_lowmem != NULL) > EVENTHANDLER_DEREGISTER(vm_lowmem, arc_event_lowmem); > -- Andriy Gapon