Page MenuHomeFreeBSD

Replace many instances of VM_WAIT with blocking page allocation functions
ClosedPublic

Authored by jeff on Oct 30 2017, 11:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 22 2024, 10:17 PM
Unknown Object (File)
Dec 20 2023, 8:29 AM
Unknown Object (File)
Oct 23 2023, 11:17 AM
Unknown Object (File)
Sep 21 2023, 5:32 PM
Unknown Object (File)
Sep 11 2023, 4:46 AM
Unknown Object (File)
Aug 17 2023, 1:59 PM
Unknown Object (File)
May 29 2023, 4:09 AM
Unknown Object (File)
May 17 2023, 6:15 AM

Details

Summary

In order to support NUMA we will want an atomic allocate and sleep to provide tighter synchronization around allocation failures and sleeps. This patch moves most VM_WAIT consumers to use WAITOK/WAITFAIL and consequently eliminates a bunch of redundant code from callers.

Test Plan

pho has been testing with stress and an emphasis on memory starvation

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

All my notes are cosmetics/style.

amd64/amd64/uma_machdep.c
53

There is no reason to keep pflags variable, inline the call to malloc2vm_flags() into the arg.

arm64/arm64/uma_machdep.c
53

Same there.

powerpc/aim/mmu_oea64.c
1520

pflags is not needed there as well.

1525

No need for {}.

powerpc/aim/slb.c
492

Same.

powerpc/powerpc/uma_machdep.c
67

Again pflags and {}.

vm/swap_pager.c
1823

It is somewhat pity to allocate the item and free it only to retry the non-wait allocation immediately after. Perhaps I will rewrite this fragment after your commit goes in.

vm/vm_fault.c
1675

I do not think this would work. For busy, we could need to sleep anyway, while hold does not prevent the free. Also, the object might be collapsed and src page copied.

Might be the paging in progress count could be used there, but current simple handling of the low mem situation seems to be easiest.

vm/vm_page.c
3240

Please split the line and put spaces around '|'.

This revision is now accepted and ready to land.Oct 31 2017, 11:21 AM

Might be nice to define VM_ALLOC_WAITFLAGS to (VM_ALLOC_WAITOK|VM_ALLOC_NOWAIT|VM_ALLOC_WAITFAIL) and use it in places where you wish to clear all three.

vm_page_alloc(), vm_page_grab_pages() and vm_page_alloc_freelist() document the flags that they accept; those comments should be updated.

mips/mips/uma_machdep.c
55

Looks like this is missing parens? Also the constants are misnamed, there's no "PAGE_".

amd64/amd64/uma_machdep.c
53

I prefer sometimes to split and use temporaries for legibility.

mips/mips/uma_machdep.c
55

Thank you. I will also make universe before I commit.

vm/swap_pager.c
1823

You can rewrite if you like. I don't think it's a big deal though since it only happens after failure and it's really just manipulating a stack in the fast path of the allocator.

vm/vm_fault.c
1675

The problem will come when VM_WAIT needs to do something sane for NUMA.

Perhaps we could pass in the object that we're trying to allocate for so if it's not round-robin we sleep on the right domain.

Overall, I think that this change is a good one. It will be a lot easier to perform a NUMA-aware VM_WAIT with this change.

arm64/arm64/uma_machdep.c
55

The otherwise identical amd64 version above has dropped the unneeded braces.

mips/mips/uma_machdep.c
57

Please see my comment on vm_page_alloc_freelist() below. Essentially, in this new world order, the call to vm_page_reclaim_contig() should come from vm_page_alloc_freelist().

powerpc/aim/slb.c
493โ€“494

There is nothing wrong with your change here, but I wanted to express skepticism about the correctness of the existing code because it does not call vm_page_reclaim_contig().

powerpc/powerpc/uma_machdep.c
62

Can you please add the _NOOBJ flag here so that this version of uma_small_alloc() doesn't differ from the others in ways that don't reflect necessary machine-specific differences.

sparc64/sparc64/vm_machdep.c
406

Please drop the unnecessary braces so that these functions look the same except for required differences. And, move the _NOOBJ flag.

vm/vm_fault.c
1675

I think that this comment better explains the motivation for this change than the summary. Specifically, that VM_WAIT needs to know the NUMA allocation policy that was used by the failed allocation, and moving the VM_WAIT inside the allocation functions makes that easier.

vm/vm_page.c
1814

In this new world order, it would make sense for this function to call vm_page_reclaim_contig() rather than expecting the caller to do so. Doing so here (versus in the caller) has the advantage that we know whether we failed due a "global" shortage of pages or an absence within the correct range, so we can more selectively use vm_page_reclaim_contig().

1980

The same comment applies here as in vm_page_alloc_contig(). The difference is that this function would need to call a machine-dependent wrapper around vm_page_reclaim_contig() that understands the mapping from "free lists" to physical address ranges.

2573

The style(9) police will want a blank line here.

2582

It looks like there might be a stray space here.

3333โ€“3334

Please put spaces around the "|" operator.

vm/vm_radix.c
780

Please insert a blank line here.