Commit r345200 (new ARC reclamation threads) looks suspicious to me - second potential problem
Alexander Motin
mav at FreeBSD.org
Wed May 22 16:07:33 UTC 2019
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
More information about the freebsd-hackers
mailing list