Date: Wed, 22 May 2019 19:08:54 +0300 From: Slawa Olhovchenkov <slw@zxy.spb.ru> To: Lev Serebryakov <lev@FreeBSD.org> Cc: Alexander Motin <mav@FreeBSD.org>, Mark Johnston <markj@freebsd.org>, freebsd-fs@freebsd.org, freebsd-hackers@freebsd.org Subject: Re: Commit r345200 (new ARC reclamation threads) looks suspicious to me - second potential problem Message-ID: <20190522160854.GD47119@zxy.spb.ru> In-Reply-To: <94d051a3-3427-7a5b-efe7-169cff2265d3@FreeBSD.org> References: <369cb1e9-f36a-a558-6941-23b9b811825a@FreeBSD.org> <20190520164202.GA2130@spy> <28c7430b-fb7c-6472-5c1b-fa3ff63a9e73@FreeBSD.org> <94d051a3-3427-7a5b-efe7-169cff2265d3@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, May 22, 2019 at 06:50:10PM +0300, Lev Serebryakov wrote:
> On 22.05.2019 18:19, Alexander Motin wrote:
>
> >>> But looks like `arc_kmem_reap_soon()` is synchronous on FreeBSD! So,
> >>> this `delay()` looks very wrong. Am I right?
> >
> > Why is it wrong?
> One second pause after synchronous operation to wait it completion?
>
> There one more questionable piece of code:
>
> 6936 static void
> 6937 arc_lowmem(void *arg __unused, int howto __unused)
> 6938 {
> ....
> 6947 arc_reduce_target_size(to_free);
> 6948
> 6949 mutex_enter(&arc_adjust_lock);
> 6950 arc_adjust_needed = B_TRUE;
> 6951 zthr_wakeup(arc_adjust_zthr);
>
>
> 4587 static void
> 4588 arc_reduce_target_size(int64_t to_free)
> 4589 {
> ...
> 4612 if (asize > arc_c) {
> 4613 DTRACE_PROBE2(arc__shrink_adjust, uint64_t, asize,
> 4614 uint64_t, arc_c);
> 4615 /* See comment in arc_adjust_cb_check() on why
> lock+flag */
> 4616 mutex_enter(&arc_adjust_lock);
> 4617 arc_adjust_needed = B_TRUE;
> 4618 mutex_exit(&arc_adjust_lock);
> 4619 zthr_wakeup(arc_adjust_zthr);
> 4620 }
> 4621 }
>
> Looks like lock/flag/wakeup sequence (which is now very cheap — mutexes
> are not cheap, and this mutex could become contended in low-memory
> situation) could be called twice.
>
> Looks like `arc_reduce_target_size()` should return boolean value and
> unconditional signalling in `arc_lowmem()` should become conditional.
No, for FreeBSD case arc_lowmem() don't be conditional from this
condition in this maner and need completly differnt set of ananlys
factors and do different...
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190522160854.GD47119>