Eliminate ARC's distinction between data and metadata for eviction

[adamdmoss]

Describe the feature would like to see added to OpenZFS

The ARC should treat data and metadata equally (at least for the purposes of eviction)

How will this feature improve OpenZFS?

I believe it should simplify the ARC and help to eliminate issues such as #10508 . The eviction code, for example, quite imperfectly jumps through hoops to keep the eviction balance between ARC_BUFC_DATA and ARC_BUFC_METADATA arc lists 'fair' in a slightly doomed manner which I believe would just 'come out in the wash' if they were not distinct in the first place.

Additional context

I seem to recall some Illumos engineers saying their version of ZFS does this and it turned out to be a good win. [citation needed]
That anecdote is the strongest argument I have, to be honest, though having spent a lot of time in the ARC code it's a tempting direction if only for the sake of simplicity.

I imagine there was (or seemed like) a good reason to treat data and metadata separately in the first place though; not sure if that theory has been proven-out.

Downsides:

• There may be devil in the details to keep supporting primarycache=metadata.
• Code churn
• OpenZFS's test harness more or less exercises ARC robustness but I don't think we have anything (automated or otherwise) to prove that an ARC change is a perf win/loss, short of throwing it out to the world and letting people try it via a potentially very long release/feedback/response loop... but that's a bigger issue.

[ryao]

I remember the Solaris guys saying that they adopted this and it was a win. It is a little surprising that we have not done it yet.

[rincebrain]

Oracle Solaris, I believe, did indeed do this among other ARC changes, though I don't think there's a particularly good list of what those are outside their walls.

This looks interesting, though.

[devZer0]

how can this be a win for people who have a metadata centric workload ?

i know several examples where "too early metadata eviction" is a big problem, leading to unecessary disk excercise and runtime enlargement by factors larger then 20

[ryao]

Instead of putting a hard limit on metadata in ARC, you could have metadata consume more space up to the full ARC size depending on what the algorithm determines is appropriate. Thus, it should be a win in metadata centric workloads.

[devZer0]

if it works depends what's zfs's notion of metadata centric workload.

to have rsync backup run nightly on your pool is a metadata centric workload for me. so is garbage collection in proxmox backup server - but will zfs have the same understanding of that ?

if mru/mfu is being used to decide what's important to keep cached , zfs may oversee that it's better to keep a directory/file metadata entry in cache, even if only used once a day.

zfs keeps anything in cache what is needed frequently and what was used recently - that's different to keeping in cache what's providing the best performance benefit from the end-user-perspective and what's causing the smallest number of harddisk's head seeks....

[ryao]

ZFS has a separate ARC Meta Limit that keeps it from using space in ARC for metadata when it thinks the space should go to metadata rather than data.

[adamdmoss]

I whipped up a simplest-possible-thing at https://github.com/adamdmoss/zfs/tree/arclists - not extensively tested, YMMV etc.

[ryao]

@adamdmoss It needs some cleanup:

module/zfs/arc.c:468:12: error: 'zfs_arc_meta_adjust_restarts' defined but not used [-Werror=unused-variable]
 static int zfs_arc_meta_adjust_restarts = 4096;
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
module/zfs/arc.c:467:12: error: 'zfs_arc_meta_strategy' defined but not used [-Werror=unused-variable]
 static int zfs_arc_meta_strategy = ARC_STRATEGY_META_BALANCED;
            ^~~~~~~~~~~~~~~~~~~~~
module/zfs/arc.c:466:12: error: 'zfs_arc_meta_prune' defined but not used [-Werror=unused-variable]
 static int zfs_arc_meta_prune = 10000;
            ^~~~~~~~~~~~~~~~~~

[ryao]

The kernel module parameters and their man page documentation should also be removed.

Once the warnings/errors are fixed, you could put it into a draft pull request so that the continuous integration infrastructure runs the test suite against it. It can take around 4 to 6 hours to get results (I forget exactly how long).

[adamdmoss]

There should only be warnings-as-errors, not errors. There are reasons I didn't make it a PR. :)

[adamdmoss]

I've gotten rid of those three now though!

[adamdmoss]

The kernel module parameters and their man page documentation should also be removed.

To be clear, I don't intend to do a PR - I was just prototyping out of interest to see how it behaved. If people are really excited I could always finish the job, though.