Page MenuHomeFreeBSD
Differential D21882

Use a loop macro for enumerating vm map entries
ClosedPublic
Actions

Authored by dougm on Oct 3 2019, 8:00 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 13, 3:38 AM
Unknown Object (File)
Fri, Nov 1, 2:20 AM
Unknown Object (File)
Oct 21 2024, 4:09 PM
Unknown Object (File)
Oct 7 2024, 11:38 AM
Unknown Object (File)
Oct 5 2024, 9:43 PM
Unknown Object (File)
Oct 2 2024, 3:15 AM
Unknown Object (File)
Sep 30 2024, 4:04 AM
Unknown Object (File)
Sep 30 2024, 3:02 AM
View All 98 Files
Subscribers
gnn
imp
pho

Details

Reviewers
alc
markj
Commits
rS353298: Define macro VM_MAP_ENTRY_FOREACH for enumerating the entries in a vm_map.
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

dougm created this revision.Oct 3 2019, 8:00 AM
dougm updated this revision to Diff 62874.Oct 3 2019, 3:47 PM

Correct an off-by-one error.

pho added a comment.Oct 3 2019, 4:01 PM

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

dougm updated this revision to Diff 62898.Oct 4 2019, 4:09 AM

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

pho added a comment.Oct 5 2019, 6:26 PM

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

dougm added a reviewer: markj.Oct 6 2019, 6:54 PM
markj accepted this revision.Oct 7 2019, 2:44 PM

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
dougm added inline comments.Oct 7 2019, 3:05 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.

markj added inline comments.Oct 7 2019, 3:08 PM
sys/vm/vm_object.c
2387

Looks like it should be fine to remove.

alc added a comment.Oct 7 2019, 5:55 PM
In D21882#478825, @markj wrote:

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.

alc added inline comments.Oct 7 2019, 6:08 PM
sys/kern/sys_process.c
387

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?

dougm added inline comments.Oct 7 2019, 6:49 PM
sys/kern/sys_process.c
387

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

dougm updated this revision to Diff 63020.Oct 8 2019, 2:51 AM
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
dougm added inline comments.Oct 8 2019, 6:54 AM
sys/kern/sys_process.c
387

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
Closed by commit rS353298: Define macro VM_MAP_ENTRY_FOREACH for enumerating the entries in a vm_map. (authored by dougm). · Explain Why
This revision was automatically updated to reflect the committed changes.
dougm added a commit: rS353298: Define macro VM_MAP_ENTRY_FOREACH for enumerating the entries in a vm_map..
Herald added subscribers: gnn, imp. · View Herald TranscriptOct 8 2019, 7:14 AM

Revision Contents
Changeset List

PathSize
sys/
compat/
linprocfs/
3 lines
dev/
hwpmc/
2 lines
fs/
procfs/
3 lines
tmpfs/
3 lines
kern/
3 lines
6 lines
8 lines
security/
mac/
2 lines
vm/
2 lines
4 lines
12 lines
3 lines
8 lines

Diff 62874

View Options

sys/compat/linprocfs/linprocfs.c

Loading...
View Options

sys/dev/hwpmc/hwpmc_mod.c

Loading...
View Options

sys/fs/procfs/procfs_map.c

Loading...
View Options

sys/fs/tmpfs/tmpfs_vfsops.c

Loading...
View Options

sys/kern/imgact_elf.c

Loading...
View Options

sys/kern/kern_proc.c

Loading...
View Options

sys/kern/sys_process.c

Loading...
View Options

sys/security/mac/mac_process.c

Loading...
View Options

sys/vm/swap_pager.c

Loading...
View Options

sys/vm/vm_map.h

Loading...
View Options

sys/vm/vm_object.c

Loading...
View Options

sys/vm/vm_pageout.c

Loading...
View Options

sys/vm/vm_swapout.c

Loading...