Re: [RFC] Tagging of vmalloc pages for supporting the pmalloc allocator

[Jerome Glisse]

On Mon, Aug 07, 2017 at 05:13:00PM +0300, Igor Stoppa wrote:
> 
> 
> On 07/08/17 16:31, Jerome Glisse wrote:
> > On Mon, Aug 07, 2017 at 02:26:21PM +0300, Igor Stoppa wrote:
> 
> [...]
> 
> >> I'll add a vm_area field as you advised.
> >>
> >> Is this something I could send as standalone patch?
> > 
> > Note that vmalloc() is not the only thing that use vmalloc address
> > space. There is also vmap() and i know one set of drivers that use
> > vmap() and also use the mapping field of struct page namely GPU
> > drivers.
> 
> Ah, yes, you mentioned this.
> 
> > So like i said previously i would store a flag inside vm_struct to
> > know if page you are looking at are pmalloc or not.
> 
> And I was planning to follow your advice, using one of the flags.
> But ...
> 
> > Again do you
> > need to store something per page ? Would storing it per vm_struct
> > not be enough ?
> 
> ... there was this further comment, about speeding up the access to
> vm_area, which seemed good from performance perspective.
> 
> ---8<--------------8<--------------8<--------------8<--------------8<---
> On 03/08/17 14:48, Michal Hocko wrote:
> > On Thu 03-08-17 13:11:45, Igor Stoppa wrote:
> 
> [...]
> 
> >> But, to reply more specifically to your advice, yes, I think I could
> >> add a flag to vm_struct and then retrieve its value, for the address
> >> being processed, by passing through find_vm_area().
> >
> > ... and you can store vm_struct pointer to the struct page there and
> > you won't need to do the slow find_vm_area. I haven't checked very
> > closely but this should be possible in principle. I guess other
> > callers might benefit from this as well.
> ---8<--------------8<--------------8<--------------8<--------------8<---
> 
> I do not strictly need to modify the page struct, but it seems it might
> harm performance, if it is added on the path of hardened usercopy.
> 
> I have an updated version of the old proposal:
> 
> * put a magic number in the private field, during initialization of
> pmalloc pages
> 
> * during hardened usercopy verification, when I have to assess if a page
> is of pmalloc type, compare the private field against the magic number
> 
> * if and only if the private field matches the magic number, then invoke
> find_vm_area(), so that the slowness affects only a possibly limited
> amount of false positives.

This all sounds good to me.

Jérôme