Add L2ARC arcstats for MFU/MRU buffers and buffer content type #10743

gamanakis · 2020-08-19T18:45:27Z

Motivation and Context

Currently the ARC state (MFU/MRU) of cached L2ARC buffer and their
content type is unknown. Knowing this information may prove beneficial
in adjusting the L2ARC caching policy.

This commit adds L2ARC arcstats that display the aligned size
(in bytes) of L2ARC buffers according to their content type
(data/metadata) and according to their ARC state (MRU/MFU or
prefetch). It also expands the existing evict_l2_eligible arcstat to
differentiate between MFU and MRU buffers.

Description

L2ARC caches buffers from the MRU and MFU lists of ARC. Upon caching a
buffer, its ARC state (MRU/MFU) is stored in the L2 header
(b_arcs_state). The l2_m{f,r}u_asize arcstats reflect the aligned size
(in bytes) of L2ARC buffers according to their ARC state (based on
b_arcs_state). We also account for the case where an L2ARC and ARC
cached MRU or MRU_ghost buffer transitions to MFU. The l2_prefetch_asize
reflects the alinged size (in bytes) of L2ARC buffers that were cached
while they had the prefetch flag set in ARC. This is dynamically updated
as the prefetch flag of L2ARC buffers changes.

Persistent L2ARC:
When committing an L2ARC buffer to a log block (L2ARC metadata) its
b_arcs_state and prefetch flag is also stored. If the buffer changes
its ARC state or prefetch flag this is reflected in the above arcstats.
However, the L2ARC metadata cannot currently be updated to reflect this
change.
Example: L2ARC caches an MRU buffer. L2ARC metadata and arcstats count
this as an MRU buffer. The buffer transitions to MFU. The arcstats are
updated to reflect this. Upon pool re-import or on/offlining the L2ARC
device the arcstats are cleared and the buffer will now be counted as an
MRU buffer, as the L2ARC metadata were not updated.

Bug fix:

If l2arc_noprefetch is set, arc_read_done clears the L2CACHE flag of
an ARC buffer. However, prefetches may be issued in a way that
arc_read_done() is bypassed. Instead, move the related code in
l2arc_write_eligible() to account for those cases too.

Also add a test and update manpages for l2arc_mfuonly module parameter,
and update the manpages and code comments for l2arc_noprefetch.
Closes #10464.

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

codecov · 2020-08-19T19:13:26Z

Codecov Report

Merging #10743 into master will increase coverage by 1.33%.
The diff coverage is 88.70%.

@@            Coverage Diff             @@
##           master   #10743      +/-   ##
==========================================
+ Coverage   79.04%   80.38%   +1.33%     
==========================================
  Files         395      293     -102     
  Lines      125233    84770   -40463     
==========================================
- Hits        98993    68141   -30852     
+ Misses      26240    16629    -9611

Flag	Coverage Δ
#kernel	`80.38% <88.70%> (+1.26%)`	⬆️
#user	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
module/zfs/arc.c	`88.18% <88.70%> (-1.44%)`	⬇️
module/icp/asm-x86_64/aes/aeskey.c	`0.00% <0.00%> (-100.00%)`	⬇️
module/icp/algs/modes/gcm_generic.c	`11.11% <0.00%> (-88.89%)`	⬇️
module/zfs/vdev_raidz_math_sse2.c	`14.39% <0.00%> (-84.85%)`	⬇️
module/icp/algs/aes/aes_impl_generic.c	`0.82% <0.00%> (-69.55%)`	⬇️
module/icp/algs/skein/skein_block.c	`31.94% <0.00%> (-68.06%)`	⬇️
module/icp/algs/aes/aes_impl_x86-64.c	`40.00% <0.00%> (-60.00%)`	⬇️
module/os/linux/spl/spl-err.c	`36.11% <0.00%> (-44.45%)`	⬇️
module/icp/algs/edonr/edonr.c	`45.72% <0.00%> (-42.59%)`	⬇️
module/zfs/ddt_zap.c	`57.14% <0.00%> (-38.32%)`	⬇️
... and 299 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e7fe49...45152dc. Read the comment docs.

richardelling · 2020-08-20T19:35:32Z

I don't see where this deals with the case of a buffer changing from MRU to MFU. Indeed, it could have been written to L2 when it was MRU, then it transitions to MFU, so it should be accounted in L2 as MFU.

richardelling · 2020-08-20T19:42:46Z

or, to turn this around, when we evict a buffer it will be interesting to know if it was [MRU | MFU] and [in L2 | ineligible | eligible].
In other words, expand on the existing evict_* arcstats, because the question to be answered is "are we evicting a lot of MFU that is eligible for, but not in L2?"

richardelling · 2020-08-20T19:44:46Z

FYI, answering the above question also can expose problems with the very simplistic (too simplistic?) L2 eviction algorithm.

gamanakis · 2020-08-21T12:23:59Z

@richardelling your points are well taken.
Still working on it.

ghost · 2020-08-28T02:33:23Z

Could cmd/arcstat and cmd/arc_summary be updated as well?

gamanakis · 2020-08-28T15:52:29Z

The test is failing on all instances. Converted this PR to draft again, until I fix it.

ghost

Thanks for updating the commands. I had few nits reading through the rest of it, but I don't feel too strongly about them. I'm not familiar enough with the ARC/L2ARC accounting to give an explicit approval, but I'm certainly in favor of better observability in this area.

module/zfs/arc.c

tests/zfs-tests/tests/functional/l2arc_arcstats/cleanup.ksh

richardelling

Thanks for this George, the sooner we can get the metrics in the field, the sooner we can understand how the MRU/MFU and L2 interact.

FYI, stats collectors like node_exporter and telegraf will automatically pull in the new stats. When I can find some spare time, I'll add a grafana dashboard or two.

cmd/arcstat/arcstat.in

configure.ac

gamanakis · 2020-09-05T05:08:05Z

Thank you all for your comments!
0b849a6: Rebased to master, addressed suggestions. I moved the test to the directory l2arc. Perhaps we could also move the persist_l2arc tests here.

gamanakis · 2020-09-07T04:22:37Z

aff7dad: Change zdb logging from "state" to "ARC state" for clarity.

behlendorf

Looks good! Just one suggested optimization.

include/sys/arc_impl.h

gamanakis · 2020-09-10T16:50:40Z

55fcb2d: Fix from @behlendorf review, add a test case for l2arc_mfuonly and update the man page for l2arc_mfuonly in terms of what to expect if l2arc_noprefetch is 0.

fb9803e: Make the mfuonly test to read the l2 arcstats after re-importing the pool, explain why in the comments of the test, and document expected behaviour in the man page of l2arc_mfuonly.

10277e4: Nit in the comments of arcstats test.

gamanakis · 2020-09-11T03:07:33Z

22e263f: Update manpages and fix a code comment for l2arc_noprefetch in order to close #10464.

gamanakis · 2020-09-11T13:13:23Z

ade4860: Move persistent L2ARC tests to the unified L2ARC directory introduced in this PR.

Currently the ARC state (MFU/MRU) of cached L2ARC buffer and their content type is unknown. Knowing this information may prove beneficial in adjusting the L2ARC caching policy. This commit adds L2ARC arcstats that display the aligned size (in bytes) of L2ARC buffers according to their content type (data/metadata) and according to their ARC state (MRU/MFU or prefetch). It also expands the existing evict_l2_eligible arcstat to differentiate between MFU and MRU buffers. L2ARC caches buffers from the MRU and MFU lists of ARC. Upon caching a buffer, its ARC state (MRU/MFU) is stored in the L2 header (b_arcs_state). The l2_m{f,r}u_asize arcstats reflect the aligned size (in bytes) of L2ARC buffers according to their ARC state (based on b_arcs_state). We also account for the case where an L2ARC and ARC cached MRU or MRU_ghost buffer transitions to MFU. The l2_prefetch_asize reflects the alinged size (in bytes) of L2ARC buffers that were cached while they had the prefetch flag set in ARC. This is dynamically updated as the prefetch flag of L2ARC buffers changes. When buffers are evicted from ARC, if they are determined to be L2ARC eligible then their logical size is recorded in evict_l2_eligible_m{r,f}u arcstats according to their ARC state upon eviction. Persistent L2ARC: When commiting an L2ARC buffer to a log block (L2ARC metadata) its b_arcs_state and prefetch flag is also stored. If the buffer changes its arcstate or prefetch flag this is reflected in the above arcstats. However, the L2ARC metadata cannot currently be updated to reflect this change. Example: L2ARC caches an MRU buffer. L2ARC metadata and arcstats count this as an MRU buffer. The buffer transitions to MFU. The arcstats are updated to reflect this. Upon pool re-import or on/offlining the L2ARC device the arcstats are cleared and the buffer will now be counted as an MRU buffer, as the L2ARC metadata were not updated. Bug fix: - If l2arc_noprefetch is set, arc_read_done clears the L2CACHE flag of an ARC buffer. However, prefetches may be issued in a way that arc_read_done() is bypassed. Instead, move the related code in l2arc_write_eligible() to account for those cases too. Also add a test and update manpages for l2arc_mfuonly module parameter, and update the manpages and code comments for l2arc_noprefetch. Move persist_l2arc tests to l2arc. Signed-off-by: George Amanakis <gamanakis@gmail.com>

gamanakis · 2020-09-11T18:49:06Z

45152dc: Make the manpage for l2arc_noprefetch a bit more user-friendly.

behlendorf · 2020-09-12T15:29:28Z

This looks ready to go to me. @gamanakis if you could just confirm you're happy with the latest version we can get this merged.

gamanakis · 2020-09-12T16:14:31Z

@behlendorf I am happy with the current state. Thank you!

Currently the ARC state (MFU/MRU) of cached L2ARC buffer and their content type is unknown. Knowing this information may prove beneficial in adjusting the L2ARC caching policy. This commit adds L2ARC arcstats that display the aligned size (in bytes) of L2ARC buffers according to their content type (data/metadata) and according to their ARC state (MRU/MFU or prefetch). It also expands the existing evict_l2_eligible arcstat to differentiate between MFU and MRU buffers. L2ARC caches buffers from the MRU and MFU lists of ARC. Upon caching a buffer, its ARC state (MRU/MFU) is stored in the L2 header (b_arcs_state). The l2_m{f,r}u_asize arcstats reflect the aligned size (in bytes) of L2ARC buffers according to their ARC state (based on b_arcs_state). We also account for the case where an L2ARC and ARC cached MRU or MRU_ghost buffer transitions to MFU. The l2_prefetch_asize reflects the alinged size (in bytes) of L2ARC buffers that were cached while they had the prefetch flag set in ARC. This is dynamically updated as the prefetch flag of L2ARC buffers changes. When buffers are evicted from ARC, if they are determined to be L2ARC eligible then their logical size is recorded in evict_l2_eligible_m{r,f}u arcstats according to their ARC state upon eviction. Persistent L2ARC: When committing an L2ARC buffer to a log block (L2ARC metadata) its b_arcs_state and prefetch flag is also stored. If the buffer changes its arcstate or prefetch flag this is reflected in the above arcstats. However, the L2ARC metadata cannot currently be updated to reflect this change. Example: L2ARC caches an MRU buffer. L2ARC metadata and arcstats count this as an MRU buffer. The buffer transitions to MFU. The arcstats are updated to reflect this. Upon pool re-import or on/offlining the L2ARC device the arcstats are cleared and the buffer will now be counted as an MRU buffer, as the L2ARC metadata were not updated. Bug fix: - If l2arc_noprefetch is set, arc_read_done clears the L2CACHE flag of an ARC buffer. However, prefetches may be issued in a way that arc_read_done() is bypassed. Instead, move the related code in l2arc_write_eligible() to account for those cases too. Also add a test and update manpages for l2arc_mfuonly module parameter, and update the manpages and code comments for l2arc_noprefetch. Move persist_l2arc tests to l2arc. Reviewed-by: Ryan Moeller <freqlabs@FreeBSD.org> Reviewed-by: Richard Elling <Richard.Elling@RichardElling.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: George Amanakis <gamanakis@gmail.com> Closes openzfs#10743

gamanakis mentioned this pull request Aug 19, 2020

Introduce ZFS module parameter l2arc_mfuonly #10710

Merged

12 tasks

gamanakis force-pushed the l2arc_instrument branch 7 times, most recently from dbb3afb to 84230c6 Compare August 20, 2020 05:01

gamanakis force-pushed the l2arc_instrument branch 8 times, most recently from dd7182a to 3fe91aa Compare August 27, 2020 13:32

gamanakis marked this pull request as ready for review August 27, 2020 14:01

gamanakis changed the title ~~[DRAFT] Add L2ARC arcstats for MFU/MRU buffers and buffer content type~~ Add L2ARC arcstats for MFU/MRU buffers and buffer content type Aug 27, 2020

ghost added the Status: Code Review Needed Ready for review and testing label Aug 28, 2020

gamanakis force-pushed the l2arc_instrument branch from 3fe91aa to a6b8a47 Compare August 28, 2020 03:03

gamanakis marked this pull request as draft August 28, 2020 15:51

gamanakis force-pushed the l2arc_instrument branch 2 times, most recently from d357d1c to 26b10bd Compare August 28, 2020 16:07

ghost reviewed Sep 3, 2020

View reviewed changes

module/zfs/arc.c Outdated Show resolved Hide resolved

module/zfs/arc.c Outdated Show resolved Hide resolved

tests/zfs-tests/tests/functional/l2arc_arcstats/cleanup.ksh Outdated Show resolved Hide resolved

richardelling approved these changes Sep 3, 2020

View reviewed changes

behlendorf reviewed Sep 4, 2020

View reviewed changes

cmd/arcstat/arcstat.in Show resolved Hide resolved

configure.ac Outdated Show resolved Hide resolved

gamanakis force-pushed the l2arc_instrument branch from e1ea357 to 0b849a6 Compare September 5, 2020 05:05

gamanakis force-pushed the l2arc_instrument branch from 0b849a6 to aff7dad Compare September 7, 2020 04:21

behlendorf approved these changes Sep 8, 2020

View reviewed changes

include/sys/arc_impl.h Show resolved Hide resolved

gamanakis force-pushed the l2arc_instrument branch from aff7dad to 55fcb2d Compare September 10, 2020 16:48

gamanakis force-pushed the l2arc_instrument branch 5 times, most recently from 60d556f to c9cd8cf Compare September 11, 2020 03:05

gamanakis force-pushed the l2arc_instrument branch 2 times, most recently from 22e263f to ade4860 Compare September 11, 2020 13:06

gamanakis force-pushed the l2arc_instrument branch from ade4860 to 45152dc Compare September 11, 2020 18:48

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 12, 2020

behlendorf merged commit 0853216 into openzfs:master Sep 14, 2020

hmenke mentioned this pull request Jul 23, 2021

[21.05] zfsUnstable: 2.1.0-rc5 -> 2.1.0 NixOS/nixpkgs#131238

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add L2ARC arcstats for MFU/MRU buffers and buffer content type #10743

Add L2ARC arcstats for MFU/MRU buffers and buffer content type #10743

gamanakis commented Aug 19, 2020 •

edited

Loading

codecov bot commented Aug 19, 2020 •

edited

Loading

richardelling commented Aug 20, 2020

richardelling commented Aug 20, 2020

richardelling commented Aug 20, 2020

gamanakis commented Aug 21, 2020

ghost commented Aug 28, 2020

gamanakis commented Aug 28, 2020

ghost left a comment

richardelling left a comment

gamanakis commented Sep 5, 2020 •

edited

Loading

gamanakis commented Sep 7, 2020

behlendorf left a comment

gamanakis commented Sep 10, 2020 •

edited

Loading

gamanakis commented Sep 11, 2020 •

edited

Loading

gamanakis commented Sep 11, 2020

gamanakis commented Sep 11, 2020

behlendorf commented Sep 12, 2020

gamanakis commented Sep 12, 2020

Add L2ARC arcstats for MFU/MRU buffers and buffer content type #10743

Add L2ARC arcstats for MFU/MRU buffers and buffer content type #10743

Conversation

gamanakis commented Aug 19, 2020 • edited Loading

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

codecov bot commented Aug 19, 2020 • edited Loading

Codecov Report

richardelling commented Aug 20, 2020

richardelling commented Aug 20, 2020

richardelling commented Aug 20, 2020

gamanakis commented Aug 21, 2020

ghost commented Aug 28, 2020

gamanakis commented Aug 28, 2020

ghost left a comment

Choose a reason for hiding this comment

richardelling left a comment

Choose a reason for hiding this comment

gamanakis commented Sep 5, 2020 • edited Loading

gamanakis commented Sep 7, 2020

behlendorf left a comment

Choose a reason for hiding this comment

gamanakis commented Sep 10, 2020 • edited Loading

gamanakis commented Sep 11, 2020 • edited Loading

gamanakis commented Sep 11, 2020

gamanakis commented Sep 11, 2020

behlendorf commented Sep 12, 2020

gamanakis commented Sep 12, 2020

gamanakis commented Aug 19, 2020 •

edited

Loading

codecov bot commented Aug 19, 2020 •

edited

Loading

gamanakis commented Sep 5, 2020 •

edited

Loading

gamanakis commented Sep 10, 2020 •

edited

Loading

gamanakis commented Sep 11, 2020 •

edited

Loading