Skip to content

Commit

Permalink
Increase arc_c_min to allow safe operation of arc_adapt()
Browse files Browse the repository at this point in the history
ZoL had lowered the minimum ARC size to 4MiB to better accommodate tiny
systems such as the raspberry pi, however, as of addition of large block
support, the arc_adapt() function depends on arc_c being >= 32MiB (2 *
SPA_MAXBLOCKSIZE).

This patch raises the minimum ARC size to 32MiB and adds a VERIFY test
to arc_adapt() for future-proofing.
  • Loading branch information
dweeezil authored and behlendorf committed Jun 5, 2015
1 parent dad646c commit af184e8
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions module/zfs/arc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3088,8 +3088,8 @@ arc_adapt_thread(void)
zfs_arc_max != arc_c_max)
arc_c_max = zfs_arc_max;

if (zfs_arc_min > 0 &&
zfs_arc_min < arc_c_max &&
if (zfs_arc_min >= 2ULL << SPA_MAXBLOCKSHIFT &&
zfs_arc_min <= arc_c_max &&
zfs_arc_min != arc_c_min)
arc_c_min = zfs_arc_min;

Expand Down Expand Up @@ -3355,6 +3355,7 @@ arc_adapt(int bytes, arc_state_t *state)
* If we're within (2 * maxblocksize) bytes of the target
* cache size, increment the target cache size
*/
VERIFY3U(arc_c, >=, 2ULL << SPA_MAXBLOCKSHIFT);
if (arc_size > arc_c - (2ULL << SPA_MAXBLOCKSHIFT)) {

This comment has been minimized.

Copy link
@dweeezil

dweeezil Jun 7, 2015

Author

This needs to be >= arc_c - (2ULL ...

This comment has been minimized.

Copy link
@behlendorf

behlendorf Jun 8, 2015

Owner

Are you sure? This was pulled verbatim from your original patch. The idea was just to catch the case where it would under flow due to arc_c_min being to low.

This comment has been minimized.

Copy link
@dweeezil

dweeezil Jun 8, 2015

Author

On a system on which arc_c_min is 32MiB, if arc_c ever falls to exactly that value, it will never grow again due to the ">" operator. I got this wrong in my original patch. I suppose the alternative would be to floor arc_c_min a bit higher but I think the ">=" logic makes more sense..

This comment has been minimized.

Copy link
@behlendorf

behlendorf Jun 8, 2015

Owner

Ahh I see. So what exactly are you proposing.

This comment has been minimized.

Copy link
@dweeezil

dweeezil Jun 8, 2015

Author

Change the operator on line 3359 to >=.

atomic_add_64(&arc_c, (int64_t)bytes);
if (arc_c > arc_c_max)
Expand Down Expand Up @@ -4826,8 +4827,8 @@ arc_init(void)
spl_register_shrinker(&arc_shrinker);
#endif

/* set min cache to zero */
arc_c_min = 4<<20;
/* set min cache to allow safe operation of arc_adapt() */
arc_c_min = 2ULL << SPA_MAXBLOCKSHIFT;
/* set max to 1/2 of all memory */
arc_c_max = arc_c * 4;

Expand All @@ -4837,7 +4838,8 @@ arc_init(void)
*/
if (zfs_arc_max > 64<<20 && zfs_arc_max < physmem * PAGESIZE)
arc_c_max = zfs_arc_max;
if (zfs_arc_min > 0 && zfs_arc_min <= arc_c_max)
if (zfs_arc_min >= 2ULL << SPA_MAXBLOCKSHIFT &&
zfs_arc_min <= arc_c_max)
arc_c_min = zfs_arc_min;

arc_c = arc_c_max;
Expand Down

0 comments on commit af184e8

Please sign in to comment.