Re: Possible broken MM code in dell-laptop.c?

Darren Hart
2015-06-18T21:15:16Z

On Wed, Jun 17, 2015 at 09:19:39AM +0200, Michal Hocko wrote:
> On Tue 16-06-15 20:43:34, Darren Hart wrote:
> [...]
> > Michal - thanks for the context.
> >
> > I'm surprised by your recommendation to use __free_page() out here in platform
> > driver land.
> >
> > I'd also prefer that the driver consistently free the same address to avoid
> > confusion.
> >
> > For these reasons, free_page((unsigned long)buffer) seems like the better
> > option.
> >
> > Can you elaborate on why you feel __free_page() is a better choice?
>
> Well the allocation uses alloc_page and __free_page is the freeing
> counterpart so it is natural to use it if the allocated page is
> available. Which is the case here.
>
> Anyway the code can be cleaned up by using __get_free_page for the
> allocation, then you do not have to care about the struct page and get
> the address right away without an additional code. free_page would be a
> natural freeing path.
> __get_free_page would be even a better API because it enforces that
> the allocation is not from the highmem - which the driver already does
> by not using __GFP_HIGHMEM.
>

Thank you Michal, I guess I'm just tripping over an API with mismatched __ and
no __ prefix paired calls. Thanks for the clarification.

Pali, I'm fine with any of these options - it sounds as though __get_free_page()
may be a general improvement.

--
Darren Hart
Intel Open Source Technology Center