uma: reuse VM cache buckets when replacement allocation fails

[vinicius_ferrao.net.br]

Summary

This change was a byproduct of the work we are doing to add NFS over RDMA (client and server) to FreeBSD.

During large NFS over RDMA reads, we found that the completion CPU can run out of page-cache bucket objects while other CPUs retain unused ones.

These allocations use M_NOVM, so UMA cannot allocate new backing pages to satisfy them. When replacement allocation fails, the fallback releases pages individually and takes the VM domain lock for each page.

This can cause a severe slowdown.

This patch reuses a full bucket when replacement allocation fails. It drains a full free bucket, or releases half the entries from a full allocation bucket. Keeping the other half avoids repeatedly draining and refilling the bucket on CPUs that also allocate.

I don't know much about UMA, to be honest, and I don't know if this is the right approach to the problem. Since it changes the allocator, it could have unintended effects elsewhere.

But anyway, the point here is that it helped in our tests and those carried out by NVIDIA. There may still be effects on other workloads that I haven't considered, as I already said. I'd appreciate your review here.

Also, one final remark: I kept bzero() for the clearing code. However, in recent reviews I've been asked to use memset() instead, so I can update that here too if needed.

Test Plan

The test plan is based on benchmarks and controlled testing with debug and release kernels, with INVARIANTS and WITNESS enabled in the debug kernels.

On KVM VMs and also on bare metal hardware, we ran 8 readers, each reading a separate 2 GiB file from tmpfs. This was the original NVIDIA test for the server, made by Wafa Hamzah.

For the KVM results below, we compared freshly booted unpatched and patched kernels with vm.pgcache_zone_max_pcpu=0.

We also ran 20 rounds of these workloads to see how much the results varied:

Workload	Unpatched (GB/s)	Patched (GB/s)
8 readers, 1 MiB	4.7-5.1	6.7-7.0
8 readers, 512 KiB	3.7-4.3	5.5-5.7
4 readers, 1 MiB	4.2-4.7	6.3-6.7

These tests could reproduce the issue on different systems, both inside KVM and on bare metal.

During the tests, we collected vmstat -z snapshots. The before/after counters from one 8-reader, 1 MiB run on each kernel showed:

Counter increase	Unpatched	Patched
128 Bucket failures	8,334,726	36,672
Busy vm pgcache requests	4,194,941	4,194,914

With that, we could achieve a reduction in bucket failures of 99%, with almost identical page-cache activity.

We could not find any increase in the other zones' failure counters in those snapshots.

[markj]

Where exactly are you allocating pages? Is there some reason you can't just have a dedicated UMA zone which provides buffers for RDMA (or just using mbuf zones), rather than (ab)using vm_page_alloc()?

My initial reaction to this patch is that I'd rather avoid adding complexity to UMA for the benefit of a specific consumer, if it can be avoided.

[vinicius_ferrao.net.br]

In D61234#1404193, @markj wrote:

Where exactly are you allocating pages? Is there some reason you can't just have a dedicated UMA zone which provides buffers for RDMA (or just using mbuf zones), rather than (ab)using vm_page_alloc()?

My initial reaction to this patch is that I'd rather avoid adding complexity to UMA for the benefit of a specific consumer, if it can be avoided.

Hi Mark,

The pages are allocated through the existing NFS read path:

nfsvno_read()
  -> nfsrv_createiovec_extpgs()
  -> mb_alloc_ext_plus_pages()
  -> vm_page_alloc_noobj()

I'm already using M_EXTPG mbufs.

RDMA transport maps and frees pages through m_freem() after completion.

The idea for UMA zone for is an option I didn't knew... maybe that's why Konstantin asked to mention you here.

I will check it.

Thanks.

[markj]

In D61234#1404241, @vinicius_ferrao.net.br wrote:

In D61234#1404193, @markj wrote:

Where exactly are you allocating pages? Is there some reason you can't just have a dedicated UMA zone which provides buffers for RDMA (or just using mbuf zones), rather than (ab)using vm_page_alloc()?

My initial reaction to this patch is that I'd rather avoid adding complexity to UMA for the benefit of a specific consumer, if it can be avoided.

Hi Mark,

The pages are allocated through the existing NFS read path:

nfsvno_read()
  -> nfsrv_createiovec_extpgs()
  -> mb_alloc_ext_plus_pages()
  -> vm_page_alloc_noobj()

I'm already using M_EXTPG mbufs.

RDMA transport maps and frees pages through m_freem() after completion.

The idea for UMA zone for is an option I didn't knew... maybe that's why Konstantin asked to mention you here.

I will check it.

Thanks.

It seems to me that we should not treat these buffers differently from all other mbuf cluster types, just because they are unmapped. mbuf zones have limits, accounting, etc., and the fact that mb_alloc_ext_plus_pages() bypasses this is a hack, IMHO. You can define a "secondary" zone (see uma_zsecond_create()) for zone_jumbop; MJUMPAGESIZE is always equal to PAGE_SIZE in practice. The secondary zone can allocate page-sized buffers from zone_jumbop, and resolve that to a paddr with pmap_kextract() in the init function. Add a compile-time guard for thew new zone, conditional on PMAP_HAS_DMAP and MJUMPAGESIZE == PAGE_SIZE.

It's just an idea, maybe there are some problems with it. But this UMA change could easily have undesirable second-order effects and it makes me uneasy. In the long term I'd actually like to remove the per-CPU page cache, at least from the noobj page allocator. That cache exacerbates physical memory fragmentation, and individual consumers which care about high-frequency page allocations should leverage UMA directly.

[glebius]

sys/vm/uma_core.c

1352

Sorry for not reviewing the gist of the change yet. I would suggest to use unsigned type for this count argument.

Note that ub_cnt should also be unsigned. Its current signedness is basically a leftover from pre-cae33c142913, 2003. The signedness of ub_cnt can be fixed in a separate commit.

[vinicius_ferrao.net.br]

sys/vm/uma_core.c

1352

That is fine @glebius

@markj suggested some ideias that will probably defeat the purpose of this change.

If there’s something here that you guys want to salvage, we can change. But I think this will be closed soon.

Thanks.