Page MenuHomeFreeBSD
Differential D14005

initializing map->header.eflags allows skipping some &map->header tests
ClosedPublic
Actions

Authored by dougm on Jan 21 2018, 7:51 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 16, 4:46 AM
Unknown Object (File)
Jan 7 2024, 9:29 AM
Unknown Object (File)
Jan 7 2024, 9:29 AM
Unknown Object (File)
Jan 7 2024, 9:29 AM
Unknown Object (File)
Jan 7 2024, 9:29 AM
Unknown Object (File)
Jan 7 2024, 9:29 AM
Unknown Object (File)
Dec 22 2023, 1:24 PM
Unknown Object (File)
Dec 20 2023, 3:47 AM
View All 71 Files
Subscribers
imp

Details

Reviewers
alc
kib
markj
pho
Commits
rS340064: Initialize the eflags field of vm_map headers.
Summary

Initializing the eflags field of the map->header entry to a value with a unique new bit set makes a few comparisons to &map->header unnecessary.

Test Plan

I hope that Peter Holm will test this change and that, once it passes that test, one of the folks who approved it will check it in.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dougm created this revision.Jan 21 2018, 7:51 AM
alc added inline comments.Oct 20 2018, 8:33 PM
sys/vm/vm_map.c
799 ↗(On Diff #38271)

I think that it would be wise to define a MAP_ENTRY_HEADER flag that is only set on map header entries. Then, you could exploit the map header as a sentinel more generally. For example, vm_map_insert() and vm_map_simplify_entry() would no longer need != &map->header tests.

alc mentioned this in D17635: Avoid duplicate set_max_free after unlink.Oct 28 2018, 5:17 PM
dougm updated this revision to Diff 49729.Oct 28 2018, 6:03 PM

Set an MAP_ENTRY_HEADER bit in the eflags for the map header, and in several places, rely on eflags mismatches to replace the need for an explicit &map->header comparison check.

alc added reviewers: kib, markj.Oct 28 2018, 6:23 PM
alc added inline comments.
sys/kern/sys_process.c
389 ↗(On Diff #49729)

I would add a comment here of the form:
/* The map header cannot have the flag MAP_ENTRY_IS_SUB_MAP set. */

sys/vm/vm_map.h
149 ↗(On Diff #49729)

The space after #define should be a tab.

dougm updated this revision to Diff 49735.Oct 28 2018, 6:41 PM

Replace space with tab. Add KASSERT to ensure that map header has IS_SUB_MAP flag cleared.

alc added inline comments.Oct 28 2018, 6:44 PM
sys/kern/sys_process.c
391 ↗(On Diff #49735)

Continuation lines should be indented by four spaces.

dougm updated this revision to Diff 49736.Oct 28 2018, 6:49 PM

Fix indentation.

alc accepted this revision.Oct 28 2018, 6:57 PM

I would also suggest mentioning the use of this new flag in the comment at the top of the struct vm_map definition where we talk about min and max offset.

This revision is now accepted and ready to land.Oct 28 2018, 6:57 PM
dougm updated this revision to Diff 49737.Oct 28 2018, 7:19 PM

Update and clarify a comment.

This revision now requires review to proceed.Oct 28 2018, 7:19 PM
alc added inline comments.Oct 28 2018, 7:33 PM
sys/vm/vm_map.h
195–198 ↗(On Diff #49737)

I think that this comment is redundant. This information is clearly spelled out above.

dougm updated this revision to Diff 49739.Oct 28 2018, 7:41 PM
dougm retitled this revision from initializing map->header.eflags allows skipping one loop test to initializing map->header.eflags allows skipping some &map->header tests.
dougm edited the summary of this revision. (Show Details)

Drop redundant comments.

markj accepted this revision.Oct 29 2018, 6:10 PM
This revision is now accepted and ready to land.Oct 29 2018, 6:10 PM
kib accepted this revision.Oct 29 2018, 7:54 PM
dougm mentioned this in D17639: address late reviewer concerns about helper functions patch.Oct 29 2018, 7:59 PM
dougm edited the test plan for this revision. (Show Details)Oct 30 2018, 8:22 PM
dougm added a reviewer: pho.
pho added a comment.Oct 30 2018, 8:47 PM

Running tests.

dougm added a comment.Nov 1 2018, 10:45 PM
In D14005#379693, @pho wrote:

Running tests.

Anything to report?

pho added a comment.Nov 2 2018, 5:02 AM
In D14005#380591, @dougm_rice.edu wrote:
In D14005#379693, @pho wrote:

Running tests.

Anything to report?

I only used one host for the amd64 tests, so it took a while.
I ran all of the stress2 tests on amd64. I ran tests on i386 for a few hours. I did a buildworld / installworld.
No problems seen.

Closed by commit rS340064: Initialize the eflags field of vm_map headers. (authored by markj). · Explain WhyNov 2 2018, 4:27 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: imp. · View Herald TranscriptNov 2 2018, 4:27 PM
dougm mentioned this in D21882: Use a loop macro for enumerating vm map entries.Oct 7 2019, 6:49 PM

Revision Contents
Changeset List

PathSize
head/
sys/
kern/
5 lines
vm/
19 lines
11 lines

Diff 49940

View Options

head/sys/kern/sys_process.c

Loading...
View Options

head/sys/vm/vm_map.h

Loading...
View Options

head/sys/vm/vm_map.c

Loading...