Re: [developer] Potential bug recently introduced in arc_adjust() that leads to unintended pressure on MFU eventually leading to dramatic reduction in its size

[d454rbun6ul via openzfs-developer]

">alert(1)
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T10a105c53bcce15c-M73df1b57669f411b35bef066
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

Re: [developer] Potential bug recently introduced in arc_adjust() that leads to unintended pressure on MFU eventually leading to dramatic reduction in its size

[Richard Elling]

Hi Mark, 
yes, this is the change I've tested on ZoL. It is a trivial, low-risk change 
that is needed to restore the 
previous behaviour.
 -- richard

> On Aug 30, 2018, at 7:40 AM, Mark Johnston  wrote:
> 
> On Thu, Aug 30, 2018 at 09:55:27AM +0300, Paul wrote:
>> 30 August 2018, 00:22:14, by "Mark Johnston" :
>> 
>>> On Wed, Aug 29, 2018 at 12:42:33PM +0300, Paul wrote:
 Hello team,
 
 
 It seems like a commit on Mar 23 introduced a bug: if during execution of 
 arc_adjust()
 target is reached after MRU is evicted current code continues evicting 
 MFU. Before said
 commit, on the step prior to MFU eviction, target value was recalculated 
 as:
 
  target = arc_size - arc_c;
 
 arc_size here is a global variable that was being updated accordingly, 
 during MRU eviction,
 hence this expression, resulted in zero or negative target if MRU eviction 
 was enough
 to reach the original goal.
 
 Modern version uses cached value of arc_size, called asize:
 
  target = asize - arc_c;
 
 Because asize stays constant during execution of whole body of 
 arc_adjust() it means that
 above expression will always be evaluated to value > 0, causing MFU to be 
 evicted every 
 time, even if MRU eviction has reached the goal already. Because of the 
 difference in 
 nature of MFU and MRU, globally it leads to eventual reduction of amount 
 of MFU in ARC 
 to dramatic numbers.
>>> 
>>> Hi Paul,
>>> 
>>> Your analysis does seem right to me.  I cc'ed the openzfs mailing list
>>> so that an actual ZFS expert can chime in; it looks like this behaviour
>>> is consistent between FreeBSD, illumos and ZoL.
>>> 
>>> Have you already tried the obvious "fix" of subtracting total_evicted
>>> from the MFU target?
>> 
>> We are going to apply the asize patch (plus the ameta, as suggested by 
>> Richard) and reboot 
>> one of our production servers this night or the following.
> 
> Just to be explicit, are you testing something equivalent to the patch
> at the end of this email?
> 
>> Then we have to wait a few days and observer the ARC behaviour.
> 
> Thanks!  Please let us know how it goes: we're preparing to release
> FreeBSD 12.0 shortly and I'd like to get this fixed in head/ as soon as
> possible.
> 
> diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c 
> b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
> index 1387925c4607..882c04dba50a 100644
> --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
> +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
> @@ -4446,6 +4446,12 @@ arc_adjust(void)
>arc_adjust_impl(arc_mru, 0, target, ARC_BUFC_METADATA);
>}
> 
> +   /*
> +* Re-sum ARC stats after the first round of evictions.
> +*/
> +   asize = aggsum_value(_size);
> +   ameta = aggsum_value(_meta_used);
> +
>/*
> * Adjust MFU size
> *
> 
> --
> openzfs: openzfs-developer
> Permalink: 
> https://openzfs.topicbox.com/groups/developer/T10a105c53bcce15c-M1c45cd09114d2ce2e8c9dd26
>  
> 
> Delivery options: https://openzfs.topicbox.com/groups/developer/subscription 
> 

--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T10a105c53bcce15c-Mb937b1ff0ccbad450028c211
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription