expose free space histograms in /proc

[cleverca22]

Motivation and Context

this allows graphing zdb -MM without any performance costs

Description

adds a new kstat in /proc with free space fragmentation
example output:

[root@nixos:~]# cat /proc/spl/kstat/zfs/vm/fragmentation
normal{power="9",pool="vm"} 14
normal{power="10",pool="vm"} 75
normal{power="11",pool="vm"} 16
normal{power="12",pool="vm"} 37

How Has This Been Tested?

booting a vm and comparing the output to zdb -MM vm

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)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS 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.

[ryao]

These really should be destroyed in the reverse order that they were initialized, but that is a pre-existing problem that merits its own patch.

[ryao]

This looks good in my cursory review.

[rincebrain]

"fragmentation" seems like an awkward name, to me, for this /proc entry. Maybe something about it being the free space (or the histogram thereof)?

[ghost]

I don't think we need the pool name repeated on every line when it is already known. Is that formatted for influxdb? Nope not quite.

[ryao]

@freqlabs It is formatted for prometheus.

[ghost]

I'd prefer a format that is easier to manipulate for anyone not using prometheus.

[cleverca22]

the pool name was repeated, so it could easily just be dumped right into prometheus

but if the prometheus format is being removed, and its just 9 14, then the pool name isnt really needed, something has to convert it to prometheus, and can add the pool name back in as it does so

@rincebrain what about just histogram maybe? or metaclass_histogram?
i could also break it up, and so normal_histogram and special_histogram, which is sort of needed if the prometheus metric names are going away

[cleverca22]

[root@nixos:/proc/spl/kstat/zfs/vm]# ls fragmentation_*
fragmentation_dedup	    fragmentation_log	  fragmentation_special
fragmentation_embedded_log  fragmentation_normal

[root@nixos:/proc/spl/kstat/zfs/vm]# cat fragmentation_normal 
9 19
10 49
11 24
12 38
13 21
14 17
15 12
16 2
17 2
27 15

[root@nixos:/proc/spl/kstat/zfs/vm]# 

removed all of the redundant info, so its just a list of numbers now
also changed it to have one metaclass per file, and filled in all of them

open to suggestions on renaming the stat files

[ghost]

I missed it because the checkstyle workflow didn't run and it's hard to judge whitespace on github, but this doesn't pass cstyle. Generally try to match the style of the existing code, make cstyle will be more specific.

[cleverca22]

fixed the code to agree with cstyle

[ghost]

Please also sign off on the commit with your next update: https://github.com/openzfs/zfs/blob/master/.github/CONTRIBUTING.md#signed-off-by

[ghost]

All the commits need to be signed off, and the last two should be squashed into one.

[behlendorf]

@cleverca22 if you're still interested in pursuing this change please address the outstanding feedback and rebase this PR on the latest master.

[cleverca22]

i do want to get this merged, i'll take a look at rebasing and addressing the comments when i get some free time

i have been running this branch on 3 machines since i made it, and need to update them anyways

[behlendorf]

@cleverca22 great. We can tacklie this in parts if you want to open a new PR with just a singed off version of c0bd9ca. Then this can be squashed rebased on top of that change.

[amotin]

The checkstyle is unhappy: error: missing "Signed-off-by". Please add it to the commit message.

[behlendorf]

One last addition and this should be good. Can you rebase it and force update the PR while making the change.

[behlendorf]

A 6th metaslab class called spa_special_embedded_log_class was added by be1e991. Please add this one in too.

[behlendorf]

@ryan-moeller recently added allocation class metrics as pool properties in #18238. They don't include the free space histograms, but the other key metrics are there including fragmentation. Providing the full histograms in /proc I think is still useful but let's align the names with the pool properties. I'd suggest.

class_normal_free_histogram
class_special_free_histogram
class_dedup_free_histogram
class_log_free_histogram
class_elog_free_histogram
class_special_elog_free_histogram

I'd also suggest adding a header to the kstat output. That's easy enough to strip off for tools automatically consuming this output and helpful for us humans.