Page MenuHomeFreeBSD

vm_fault: Stop specifying VM_ALLOC_ZERO
ClosedPublic

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

Details

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
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj requested review of this revision.Sep 21 2021, 2:45 AM
This revision is now accepted and ready to land.Sep 21 2021, 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.

This revision was automatically updated to reflect the committed changes.
markj marked 2 inline comments as done.