When traversing vm_map entries, don't look up the same neighbor twice

[dougm]

Summary

Instead of looking up a predecessor or successor to the current map entry, when that entry has been seen already, keep the already-looked-up value in a variable and use that instead of looking it up again.

Define a standard naming convention for map entry iterators in vm_map.c and apply it.

[dougm]

Add parens around returned value in vm_map_entry_first.

[dougm]

This change reduces the size of mac_process.o and vm_map.o by 56 and 656 bytes, respectively, on the amd64 machine I'm testing on.

[markj]

It would be nice if the loop variable names were more consistent: in some places we use prev_entry/entry, in others it's temp_entry/current or entry/current, etc.. I think I prefer prev_entry/entry, FWIW. Anyway, that does not block this patch.

sys/vm/vm_map.c

2575 ↗

(On Diff #64362)

Why do we need an opening paren here?

3003 ↗

(On Diff #64362)

first_entry is a bit of a misnomer now, perhaps it should become prev_entry.

3290 ↗

(On Diff #64362)

Same comment about changing to prev_entry.

[dougm]

Rename variables in vm_map loops, so that the fixed starting point is called 'first_entry', the iterator is called 'entry', and the iterator trailing it, if any, is called 'prev_entry'.

[dougm]

Fix a transcription error.

[dougm]

Rewrite a couple of lines to that prev_entry isn't, briefly, after entry.

[alc]

sys/vm/vm_map.c

1430 ↗

(On Diff #64426)

Unsorted.

[dougm]

Update to account for r354785. Address reviewer comments.

[dougm]

Use the FOREACH macros for iterating over the old map in vm_fork. Reformat a comment to fit in 80 columns.

[markj]

sys/vm/vm_map.c

2678 ↗

(On Diff #64500)

I suspect that some versions of GCC will complain that prev_entry may be left initialized while handling the modify_map case below, even though it is initialized correctly in that case.

2853 ↗

(On Diff #64500)

These assignments could be placed in a for-loop header.

sys/vm/vm_map.h

426 ↗

(On Diff #64500)

You have an extra newline in the function below. The newline is now optional per style(9), but we should be consistent within the same file.

[alc]

sys/vm/vm_map.c

3399–3402 ↗

(On Diff #64500)

I would find this clearer if it were written with entry and next_entry, not prev_entry.

[alc]

sys/vm/vm_map.c

2969–2970 ↗

(On Diff #64500)

Again, I think that this would be clearer if it were written with entry and next_entry.

3286–3287 ↗

(On Diff #64500)

Initializing prev_entry and entry in the same order in both the then and else clause will make this easier to read.

[dougm]

Accept reviewer suggestions. Where the introduction of "next_entry" suggests that a for loop could replace a while loop, do so. Where that looks tricky because of a mid-loop continue (wire/unwire), make sure the continue case sets next_entry too. Observe that this all makes the 'first_iteration' state variable redundant, and eliminate it.

[dougm]

Address small style violations from an in-person review.

[alc]

sys/vm/vm_map.c

3121 ↗

(On Diff #64607)

unsorted

3230 ↗

(On Diff #64607)

indentation

[dougm]

Address reviewer feedback.

[alc]

sys/vm/vm_map.c

2971 ↗

(On Diff #64607)

unnecessary parens

[alc]

sys/vm/vm_map.c

3743 ↗

(On Diff #64608)

need spaces around the bitwise or.

[dougm]

Address online and offline comments.

[dougm]

Undo some overzealous style-fixing.

[dougm]

In all the loops over map entries where the iterator wasn't called 'entry' and the last several versions of this patch renamed it to 'entry', revert the name back to what is was before, mostly limiting this patch to its original purpose and leaving a renaming patch for another day.

[alc]

sys/vm/vm_map.c

2690 ↗

(On Diff #64613)

This assignment would fit at the end of the previous line.

[alc]

While I like the suggestion that we use a consistent variable naming schema, I felt like those style changes were overwhelming the functional changes. So, I asked Doug to separate the functional and stylistic changes.