Page MenuHomeFreeBSD

vm_fault: Stop specifying VM_ALLOC_ZERO
AcceptedPublic

Authored by markj on Tue, Sep 21, 2:45 AM.

Details

Reviewers
alc
kib
Summary

Now vm_page_alloc() and friends will unconditionally preserve PG_ZERO,
so there is no point in setting this flag.

Eliminate a local variable and add a comment explaining why we
prioritize the allocation when the process is doomed. I remember that
it took me some time to understand this when I first read the code long
ago.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 41614
Build 38503: arc lint + arc unit

Event Timeline

markj requested review of this revision.Tue, Sep 21, 2:45 AM
This revision is now accepted and ready to land.Tue, Sep 21, 12:45 PM

As an aside, I still think that vm_page_alloc() and other allocators which insert into an object should handle VM_ALLOC_ZERO by zeroing the page. The argument against that is that when used carelessly we end up zeroing pages with the object lock held. But:

  • Many calls come via vm_page_grab(), which does exactly that anyway. kmem_back_domain() also zeros pages with the kernel object lock held, though I'm not certain about whether this is required.
  • There are far fewer vm_page_alloc() calls now, making such pessimizations easier to spot.
  • It seems undesirable to handle VM_ALLOC_ZERO inconsistently between different page allocator interfaces.

Then might be, change the semantic of VM_ALLOC_ZERO to unconditionally zero page, and add e.g. VM_ALLOC_PGZERO which preserves PG_ZERO flag?

In D32036#724558, @kib wrote:

Then might be, change the semantic of VM_ALLOC_ZERO to unconditionally zero page, and add e.g. VM_ALLOC_PGZERO which preserves PG_ZERO flag?

Note that in D32033 and D32034 vm_page_alloc() and vm_page_alloc_contig() were changed to unconditionally preserve PG_ZERO. Alan suggested this in D28805. So there is no need for VM_ALLOC_PGZERO.

In D32036#724558, @kib wrote:

Then might be, change the semantic of VM_ALLOC_ZERO to unconditionally zero page, and add e.g. VM_ALLOC_PGZERO which preserves PG_ZERO flag?

Note that in D32033 and D32034 vm_page_alloc() and vm_page_alloc_contig() were changed to unconditionally preserve PG_ZERO. Alan suggested this in D28805. So there is no need for VM_ALLOC_PGZERO.

Then everything is already handled, isn't it?

In D32036#724570, @kib wrote:
In D32036#724558, @kib wrote:

Then might be, change the semantic of VM_ALLOC_ZERO to unconditionally zero page, and add e.g. VM_ALLOC_PGZERO which preserves PG_ZERO flag?

Note that in D32033 and D32034 vm_page_alloc() and vm_page_alloc_contig() were changed to unconditionally preserve PG_ZERO. Alan suggested this in D28805. So there is no need for VM_ALLOC_PGZERO.

Then everything is already handled, isn't it?

vm_page_alloc() and vm_page_alloc_contig() effectively ignore VM_ALLOC_ZERO with the current patch series.