Page MenuHomeFreeBSD

Unwire pages when cleaning up the kcov state
ClosedPublic

Authored by andrew on Feb 19 2019, 7:33 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 28, 9:08 AM
Unknown Object (File)
Jan 10 2024, 12:04 PM
Unknown Object (File)
Jan 2 2024, 10:49 PM
Unknown Object (File)
Dec 20 2023, 1:41 AM
Unknown Object (File)
Nov 27 2023, 8:23 AM
Unknown Object (File)
Nov 22 2023, 7:42 PM
Unknown Object (File)
Nov 22 2023, 7:42 PM
Unknown Object (File)
Nov 22 2023, 7:42 PM
Subscribers

Details

Summary

Without this we leak physical pages. This can lead to the kernel locking
up as it has no available physical memory to back allocations.

Diff Detail

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

Event Timeline

sys/kern/kern_kcov.c
382 ↗(On Diff #54084)

The object is newly allocated, so there's no reason to use vm_page_grab() - use vm_page_alloc() instead.

383 ↗(On Diff #54084)

VM_ALLOC_ZERO is an advisory flag; the caller is responsible for zeroing the page if the allocator could not supply a zero-filled page.

407 ↗(On Diff #54084)

Use vm_page_unwire_noq() here. The pages are about to be freed, so there's no reason for them to participate in LRU. AFAIK the page daemon doesn't really expect to find pages from an OBJT_PHYS object in the queues, though I don't think they will cause any problems so long as they are not marked dirty.

sys/kern/kern_kcov.c
383 ↗(On Diff #54084)

Well, this is the reason to use _grab and not _alloc. _grab handles !PG_ZERO transparently.

Intent was that non-vm callers would prefer _grab.

sys/kern/kern_kcov.c
383 ↗(On Diff #54084)

Ah, I forgot. Sorry for the noise.

sys/kern/kern_kcov.c
55 ↗(On Diff #54087)

I think

#include <vm/vm_param.h>

is missing here. At least I get on amd64 compile errors like

--- kern_kcov.o ---
../../../kern/kern_kcov.c:406:4: error: use of undeclared identifier 'PA_LOCK_COUNT'
                        vm_page_lock(info->m[i]);
                        ^
../../../vm/vm_page.h:313:35: note: expanded from macro 'vm_page_lock'
#define vm_page_lock(m)         mtx_lock(vm_page_lockptr((m)))
                                         ^

without it.

sys/kern/kern_kcov.c
55 ↗(On Diff #54087)

Yes. This is extracted from a larger patch, so I missed this header is needed.

Use vm_object_reference rather than manual tracking of the mmap state

Using this version, the memory lockup observed does not occur anymore. Tested with running kcovtrace /usr/bin/uptime in a loop.

sys/kern/kern_kcov.c
126 ↗(On Diff #54131)

Why do you need the array ? You have the object where all your pages are put, so you can iterate over the pages of the object.

Use vm_page_lookup to find the page

sys/kern/kern_kcov.c
410 ↗(On Diff #54138)

You can do a lookup on pindex 0, then just use vm_page_next() rather than a separate lookup for each one.

sys/kern/kern_kcov.c
362 ↗(On Diff #54138)

This can be vm_page_t m;

381 ↗(On Diff #54138)

Then remove this line.

384 ↗(On Diff #54138)

m = vm_page_grab(...

390 ↗(On Diff #54138)

Remove this line.

409 ↗(On Diff #54138)
	TAILQ_FOREACH(m, &object->memq, listq) {
410 ↗(On Diff #54138)

This lookup is not needed then.

sys/kern/kern_kcov.c
381 ↗(On Diff #54138)

Will this work with pmap_qenter? It takes an array of vm pages.

sys/kern/kern_kcov.c
381 ↗(On Diff #54138)

How many pages usually the code set up ?
You can pmap_qenter(m, 1) at the time of grab. The cost is an IPI per the page.

sys/kern/kern_kcov.c
381 ↗(On Diff #54138)

The buffer can be up to 128MB, although syzkaller normally uses 2MB buffers.

sys/kern/kern_kcov.c
381 ↗(On Diff #54138)

For 128MB, you are going to malloc() 256K linear buffer, which is some kind of strength. For 2M buffers it is 511 extra IPIs, while the allocation is 4K, which is more reasonable.

If you are definitely need to support 128M, I would say that per-page allocation and mapping is the only route.

pmap_qenter each page
Use vm_page_next

kib added inline comments.
sys/kern/kern_kcov.c
408 ↗(On Diff #54143)

You do not need the counter.

This revision is now accepted and ready to land.Feb 20 2019, 6:10 PM

Latest version tested again with running kcovtrace /usr/bin/uptime in a loop and in addition with running syzkaller. Both work fine.

This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state Needs Review.Feb 20 2019, 10:41 PM
This revision was automatically updated to reflect the committed changes.