Remove PG_CACHED-related fields from struct vm_cnt.

[alc]

Summary

struct vm_cnt's v_cache_count and v_tcached fields are no longer used. More specifically, they are always zero because the code that incremented and decremented them no longer exists. Remove them and the remaining read accesses to them. Increment __FreeBSD_version to reflect the removal of these user-visible fields.

[alc]

Mark, this change reduced the size of struct vm_cnt without adding any spare fields. However, I thought that you encountered an assertion failure when you previously attempted to reduce the size of struct vm_cnt. Do you recall the specifics of the assertion failure?

[kib]

In D8583#178158, @alc wrote:

Mark, this change reduced the size of struct vm_cnt without adding any spare fields. However, I thought that you encountered an assertion failure when you previously attempted to reduce the size of struct vm_cnt. Do you recall the specifics of the assertion failure?

struct vmmeter is embedded into struct pcpu, and counters(9) implementation asserts statically that padded pcpu is page-sized. This was the most likely cause of the failure you mention.

That said, struct vmmeter is very surprising in that it does not participate in userspace ABI. I was quite surprised when I realized this some time ago. In other words, I do not see an urgent reason to bump version, but also the bumps are usually voluntary. Also note that changes to struct vmmeter are fine to MFC.

[alc]

In D8583#178174, @kib wrote:

That said, struct vmmeter is very surprising in that it does not participate in userspace ABI. I was quite surprised when I realized this some time ago. In other words, I do not see an urgent reason to bump version, but also the bumps are usually voluntary. Also note that changes to struct vmmeter are fine to MFC.

My rationale for the version bump was that there are likely to be "top"-like applications in the ports tree that report the count of PG_CACHED pages. Or, should I not worry about this possibility?

[markj]

In D8583#178174, @kib wrote:

In D8583#178158, @alc wrote:

Mark, this change reduced the size of struct vm_cnt without adding any spare fields. However, I thought that you encountered an assertion failure when you previously attempted to reduce the size of struct vm_cnt. Do you recall the specifics of the assertion failure?

struct vmmeter is embedded into struct pcpu, and counters(9) implementation asserts statically that padded pcpu is page-sized. This was the most likely cause of the failure you mention.

No, though I've seen that failure before too. I had wanted to keep the sysctls under an #ifndef BURN_BRIDGES to make it easier to switch between kernels with and without PG_CACHE support, since vmstat and top will exit with an error if they can't find an expected sysctl. Now that these changes are going into HEAD, I don't think it's necessary to do this anymore.

In D8583#178218, @alc wrote:

In D8583#178174, @kib wrote:

That said, struct vmmeter is very surprising in that it does not participate in userspace ABI. I was quite surprised when I realized this some time ago. In other words, I do not see an urgent reason to bump version, but also the bumps are usually voluntary. Also note that changes to struct vmmeter are fine to MFC.

My rationale for the version bump was that there are likely to be "top"-like applications in the ports tree that report the count of PG_CACHED pages. Or, should I not worry about this possibility?

There are certainly applications in ports that read these sysctls, so I think a __FreeBSD_version bump is warranted.

[kib]

In D8583#178218, @alc wrote:

My rationale for the version bump was that there are likely to be "top"-like applications in the ports tree that report the count of PG_CACHED pages. Or, should I not worry about this possibility?

Such application would issue sysctl("vm.stats.vm.v_cache_count") and get ENOENT. Checking the version at compile-time is wrong, and checking version to avoid sysctl is IMO more work than directly getting ENOENT. IMO.

[markj]

In D8583#178237, @kib wrote:

In D8583#178218, @alc wrote:

My rationale for the version bump was that there are likely to be "top"-like applications in the ports tree that report the count of PG_CACHED pages. Or, should I not worry about this possibility?

Such application would issue sysctl("vm.stats.vm.v_cache_count") and get ENOENT. Checking the version at compile-time is wrong, and checking version to avoid sysctl is IMO more work than directly getting ENOENT. IMO.

Patches to remove such calls will be applied based on a __FreeBSD_version predicate.

[kib]

In D8583#178238, @markj wrote:

Patches to remove such calls will be applied based on a __FreeBSD_version predicate.

Which is really wrong, since it encodes the interface of the host where the app is compiled, into the binary. E.g. the kernels of the ports builders machines are not updated that often, as result binaries with old version compiled into them will be delivered via pkg(8).

[markj]

In D8583#178242, @kib wrote:

In D8583#178238, @markj wrote:

Patches to remove such calls will be applied based on a __FreeBSD_version predicate.

Which is really wrong, since it encodes the interface of the host where the app is compiled, into the binary. E.g. the kernels of the ports builders machines are not updated that often, as result binaries with old version compiled into them will be delivered via pkg(8).

Indeed, but consider that there are ports which will not even compile after this change without a patch. (devel/sigar, for instance, appears to copy code from vmstat.) They are already implicitly encoding this information; the version bump just makes it easier to maintain patches.