Date: Wed, 22 May 2019 12:07:29 -0400 From: Alexander Motin <mav@FreeBSD.org> To: lev@FreeBSD.org, Mark Johnston <markj@freebsd.org> Cc: 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: <2a50e192-e672-7c87-178b-afd509a765df@FreeBSD.org> 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 22.05.2019 11:50, 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?
No. To rate-throttle them. This gives UMA a second to get back into
minimally steady state after we ripped all caches from it. As I have
told, we do not want to drain caches constantly in a tight loop, we want
more or less steady state.
> 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.
I don't think this is not a hot path to bother about it. arc_lowmem()
calls should be very rare.
--
Alexander Motin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2a50e192-e672-7c87-178b-afd509a765df>