Re: [PATCH v4 RESEND 1/2] thp, dax: add thp_get_unmapped_area for pmd mappings

["Kani, Toshimitsu" <toshi.kani@hpe.com>]

On Mon, 2016-08-29 at 12:34 -0700, Dan Williams wrote:
> On Mon, Aug 29, 2016 at 12:11 PM, Toshi Kani <toshi.kani@hpe.com>
> wrote:
> > 
> > When CONFIG_FS_DAX_PMD is set, DAX supports mmap() using pmd page
> > size.  This feature relies on both mmap virtual address and FS
> > block (i.e. physical address) to be aligned by the pmd page size.
> > Users can use mkfs options to specify FS to align block
> > allocations. However, aligning mmap address requires code changes
> > to existing applications for providing a pmd-aligned address to
> > mmap().
> > 
> > For instance, fio with "ioengine=mmap" performs I/Os with mmap()
> > [1]. It calls mmap() with a NULL address, which needs to be changed
> > to provide a pmd-aligned address for testing with DAX pmd mappings.
> > Changing all applications that call mmap() with NULL is
> > undesirable.
> > 
> > Add thp_get_unmapped_area(), which can be called by filesystem's
> > get_unmapped_area to align an mmap address by the pmd size for
> > a DAX file.  It calls the default handler, mm->get_unmapped_area(),
> > to find a range and then aligns it for a DAX file.
> > 
> > The patch is based on Matthew Wilcox's change that allows adding
> > support of the pud page size easily.
 :
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Great!

> ...with one minor nit:
> 
> 
> > 
> >  include/linux/huge_mm.h |    7 +++++++
> >  mm/huge_memory.c        |   43
> > +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 50 insertions(+)
> > 
> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> > index 6f14de4..4fca526 100644
> > --- a/include/linux/huge_mm.h
> > +++ b/include/linux/huge_mm.h
> > @@ -87,6 +87,10 @@ extern bool is_vma_temporary_stack(struct
> > vm_area_struct *vma);
> > 
> >  extern unsigned long transparent_hugepage_flags;
> > 
> > +extern unsigned long thp_get_unmapped_area(struct file *filp,
> > +               unsigned long addr, unsigned long len, unsigned
> > long pgoff,
> > +               unsigned long flags);
> > +
> >  extern void prep_transhuge_page(struct page *page);
> >  extern void free_transhuge_page(struct page *page);
> > 
> > @@ -169,6 +173,9 @@ void put_huge_zero_page(void);
> >  static inline void prep_transhuge_page(struct page *page) {}
> > 
> >  #define transparent_hugepage_flags 0UL
> > +
> > +#define thp_get_unmapped_area  NULL
> 
> Lets make this:
> 
> static inline unsigned long thp_get_unmapped_area(struct file *filp,
>                unsigned long addr, unsigned long len, unsigned long
> pgoff,
>                unsigned long flags)
> {
>     return 0;
> }
> 
> ...to get some type checking in the CONFIG_TRANSPARENT_HUGEPAGE=n
> case.
> 

Per get_unmapped_area() in mm/mmap.c, I think we need to set it to NULL
when we do not override current->mm->get_unmapped_area.

Thanks!
-Toshi