re: CVS commit: src/sys

[matthew green <mrg%eterna.com.au@localhost>]

"Maxime Villard" writes:
> Module Name:	src
> Committed By:	maxv
> Date:		Mon Jul 27 09:24:28 UTC 2015
> 
> Modified Files:
> 	src/sys/kern: subr_kmem.c
> 	src/sys/uvm: files.uvm
> Removed Files:
> 	src/sys/uvm: uvm_kmguard.c uvm_kmguard.h
> 
> Log Message:
> Several changes and improvements in KMEM_GUARD:
>  - merge uvm_kmguard.{c,h} into subr_kmem.c. It is only user there, and
>    makes it more consistent. Also, it allows us to enable KMEM_GUARD
>    without enabling DEBUG.
>  - rename uvm_kmguard_XXX to kmem_guard_XXX, for consistency
>  - improve kmem_guard_alloc() so that it supports allocations bigger than
>    PAGE_SIZE
>  - remove the canary value, and use directly the kmem header as underflow
>    pattern.
>  - fix some comments
> 
> (The UAF fifo is disabled for the moment; we actually need to register
> the va and its size, and add a weight support not to consume too much
> memory.)

thanks for extending KMEM_GUARD beyond PAGE_SIZE.  this will be
quite helpful, if even now requiring a lot more memory when
using kmguard :)

was this change presented for review anywhere before commit?

i'm not convinced that moving directly into subr_kmem.c is the
right thing or the only way to solve the problem of depending
on DEBUG.  could you separate them again, please?

the failure mode in the new kmem_guard_alloc() is also expensive,
in that instead of knowing how to unwind itself, it calls into
UVM to figure out all the details (which involves pmap_extract()
and a bunch more accounting.)  it would probably be best to
replace this with a call to uvm_pglistalloc() in the new version,
and then you'll never have a partial allocation failure to deal
with and you only make a single call to allocate these pages.

also, about removing the canary -- can you explain this a little
more, i'm not sure i understand this part.

thanks.


.mrg.