Re: [PATCH 4/4] hugetlb: add support for gigantic page allocation at runtime

[Andrew Morton]

On Mon, 7 Apr 2014 14:49:35 -0400 Luiz Capitulino <lcapitulino@xxxxxxxxxx> wrote:

> > > ---
> > >  arch/x86/include/asm/hugetlb.h |  10 +++
> > >  mm/hugetlb.c                   | 177 ++++++++++++++++++++++++++++++++++++++---
> > >  2 files changed, 176 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/hugetlb.h b/arch/x86/include/asm/hugetlb.h
> > > index a809121..2b262f7 100644
> > > --- a/arch/x86/include/asm/hugetlb.h
> > > +++ b/arch/x86/include/asm/hugetlb.h
> > > @@ -91,6 +91,16 @@ static inline void arch_release_hugepage(struct page *page)
> > >  {
> > >  }
> > >  
> > > +static inline int arch_prepare_gigantic_page(struct page *page)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +static inline void arch_release_gigantic_page(struct page *page)
> > > +{
> > > +}
> > > +
> > > +
> > >  static inline void arch_clear_hugepage_flags(struct page *page)
> > >  {
> > >  }
> > 
> > These are defined only on arch/x86, but called in generic code.
> > Does it cause build failure on other archs?
> 
> Hmm, probably. The problem here is that I'm unable to test this
> code in other archs. So I think the best solution for the first
> merge is to make the build of this feature conditional to x86_64?
> Then the first person interested in making this work in other
> archs add the generic code. Sounds reasonable?

These functions don't actually do anything so if and when other
architectures come along to implement this feature, their developers
won't know what you were thinking when you added them.  So how about
some code comments to explain their roles and responsibilities?

Or just delete them altogether and let people add them (or something
similar) if and when the need arises.  It's hard to tell when one lacks
telepathic powers, sigh.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/