From owner-freebsd-fs@freebsd.org Fri May 31 15:12:15 2019 Return-Path: Delivered-To: freebsd-fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id F194015BF78B; Fri, 31 May 2019 15:12:14 +0000 (UTC) (envelope-from slw@zxy.spb.ru) Received: from zxy.spb.ru (zxy.spb.ru [195.70.199.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 691DD7074E; Fri, 31 May 2019 15:12:13 +0000 (UTC) (envelope-from slw@zxy.spb.ru) Received: from slw by zxy.spb.ru with local (Exim 4.86 (FreeBSD)) (envelope-from ) id 1hWjCM-000I1A-A0; Fri, 31 May 2019 18:12:02 +0300 Date: Fri, 31 May 2019 18:12:02 +0300 From: Slawa Olhovchenkov To: Ian Lepore Cc: lev@FreeBSD.org, freebsd-fs@freebsd.org, freebsd-hackers@FreeBSD.org, Alexander Motin Subject: Re: Commit r345200 (new ARC reclamation threads) looks suspicious to me. Message-ID: <20190531151202.GG47119@zxy.spb.ru> References: <55989579-a228-498e-2842-453cad6f315f@FreeBSD.org> <174f71126ca39907370a8904c07546b712ad91b9.camel@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <174f71126ca39907370a8904c07546b712ad91b9.camel@freebsd.org> User-Agent: Mutt/1.5.24 (2015-08-30) X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: slw@zxy.spb.ru X-SA-Exim-Scanned: No (on zxy.spb.ru); SAEximRunCond expanded to false X-Rspamd-Queue-Id: 691DD7074E X-Spamd-Bar: +++ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [3.61 / 15.00]; ARC_NA(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_SPAM_SHORT(0.83)[0.827,0]; MIME_GOOD(-0.10)[text/plain]; RCVD_TLS_LAST(0.00)[]; DMARC_NA(0.00)[zxy.spb.ru]; AUTH_NA(1.00)[]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_SPAM_MEDIUM(0.93)[0.928,0]; IP_SCORE(0.00)[country: RU(0.01)]; MX_GOOD(-0.01)[zxy.spb.ru]; NEURAL_SPAM_LONG(0.96)[0.961,0]; R_SPF_NA(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:5495, ipnet:195.70.192.0/19, country:RU]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_COUNT_TWO(0.00)[2] X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 May 2019 15:12:15 -0000 On Mon, May 20, 2019 at 10:10:16AM -0600, Ian Lepore wrote: > On Mon, 2019-05-20 at 18:38 +0300, Lev Serebryakov wrote: > > I'm looking at last commit to > > 'sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c' (r345200) and > > have one question. > > > > Is it Ok for two threads to communicate via simple global variable? > > Now > > new code has (line 315): > > > > static boolean_t arc_adjust_needed = B_FALSE; > > > > And after that some threads run code like this: > > > > mutex_enter(&arc_adjust_lock); > > arc_adjust_needed = B_TRUE; > > mutex_exit(&arc_adjust_lock); > > zthr_wakeup(arc_adjust_zthr); > > > > And thread `arc_adjust_zthr` has code like this (line 4874): > > > > return (arc_adjust_needed); > > > > This variable is not atomic. It is not updated and/or read in atomic > > way. What code gives guarantees that `arc_adjust_zthr` will detect > > this > > change? I don't see any. Am I wrong? > > The arc_adjust_needed variable is the gating condition associated with > a condition variable and lock. It is only read or changed while > holding a lock, and the acquiring and releasing of that lock provides > the needed memory barriers. In this case, the association with the > condition variable and lock is somewhat obscured by the way the zthread > timer stuff works. The arc_adjust_cb_check() function is called from > line 193 of contrib/opensolaris/uts/common/fs/zfs/zthr.c, and that's > where you'll find the code that makes it clear this is an idiomatic > condition variable pattern. What about arc_no_grow, for example? arc_no_grow set in arc_reap_cb_check(), called from arc_reap_zthr thread and in arc_lowmem(). arc_no_grow test in arc_adapt(), called from arc_read()/arc_get_data_impl() called from many unsynced thread. How synced visibility of this varibale?