Page MenuHomeFreeBSD

Use a loop macro for enumerating vm map entries
ClosedPublic

Authored by dougm on Oct 3 2019, 8:00 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 9 2024, 6:57 AM
Unknown Object (File)
Jan 16 2024, 1:43 AM
Unknown Object (File)
Jan 7 2024, 9:21 AM
Unknown Object (File)
Jan 7 2024, 9:20 AM
Unknown Object (File)
Jan 7 2024, 9:20 AM
Unknown Object (File)
Jan 7 2024, 9:16 AM
Unknown Object (File)
Jan 7 2024, 9:16 AM
Unknown Object (File)
Jan 7 2024, 9:16 AM
Subscribers

Details

Summary

There are many source files that iterate over the entries of a vm_map. A change to the implementation of that iteration would require modifying all of them. Define a VME_FOREACH macro that encapsulates the current implementation, so that other files can be less sensitive to the changes in that implementation. Use the macro everywhere.

Test Plan

I can install the modified kernel and use it to compile a kernel.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Correct an off-by-one error.

I ran tests on D21882.62858.diff for 5 1/2 hours without seeing any problems. I'll move on to 62874.

Combine two loops in ptrace_vm_entry into one, so that "entry->next" doesn't have to appear.

I ran tests on D21882.62898 for more than a day. No problems seen.

I would consider calling it VM_MAP_ENTRY_FOREACH(): it is a bit long, but there doesn't seem to be much risk of hitting the 80 column limit, and VME_FOREACH() seems a bit terse given that the macro is used outside of the VM subsystem. The change looks fine to me either way, so please keep the name if you prefer.

sys/vm/vm_object.c
2387

What's the purpose of the counter?

This revision is now accepted and ready to land.Oct 7 2019, 2:44 PM
sys/vm/vm_object.c
2387

r6129 | dg | 1995-02-02 03:09:15 -0600 (Thu, 02 Feb 1995) | 48 lines

swap_pager.c:
Fixed long standing bug in freeing swap space during object collapses.
Fixed 'out of space' messages from printing out too often.
Modified to use new kmem_malloc() calling convention.
Implemented an additional stat in the swap pager struct to count the
amount of space allocated to that pager. This may be removed at some
point in the future.
Minimized unnecessary wakeups.

I'll remove the counter, if that's okay.

sys/vm/vm_object.c
2387

Looks like it should be fine to remove.

I would consider calling it VM_MAP_ENTRY_FOREACH(): it is a bit long, but there doesn't seem to be much risk of hitting the 80 column limit, and VME_FOREACH() seems a bit terse given that the macro is used outside of the VM subsystem. The change looks fine to me either way, so please keep the name if you prefer.

I agree.

sys/kern/sys_process.c
388

I'm not sure that I see the point of checking the header repeatedly for an erroneous submap flag. But more generally, I can't make any sense out of the submap handling. Has this changed in the last year or so?

sys/kern/sys_process.c
388

The origin of the KASSERT is https://reviews.freebsd.org/D14005

dougm marked 3 inline comments as done.

VME_ -> VM_MAP_ENTRY_
Remove purposeless counter.
Move KASSERT out of loop.

This revision now requires review to proceed.Oct 8 2019, 2:51 AM
sys/kern/sys_process.c
388

I'll change the "repeated" checking to make it one-time.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 8 2019, 7:14 AM
This revision was automatically updated to reflect the committed changes.