Re: [RFC 00/11] Remove 'order' argument from many mm functions

Ira Weiny
2019-05-29T21:43:30Z

On Thu, May 09, 2019 at 11:29:02AM -0700, Matthew Wilcox wrote:
> On Thu, May 09, 2019 at 04:48:39PM +0000, Weiny, Ira wrote:
> > > On Wed, May 08, 2019 at 06:58:09PM -0700, Ira Weiny wrote:
> > > > On Mon, May 06, 2019 at 09:05:58PM -0700, Matthew Wilcox wrote:
> > > > > It's possible to save a few hundred bytes from the kernel text by
> > > > > moving the 'order' argument into the GFP flags. I had the idea
> > > > > while I was playing with THP pagecache (notably, I didn't want to add an
> > > 'order'
> > > > > parameter to pagecache_get_page())
> > > ...
> > > > > Anyway, this is just a quick POC due to me being on an aeroplane for
> > > > > most of today. Maybe we don't want to spend five GFP bits on this.
> > > > > Some bits of this could be pulled out and applied even if we don't
> > > > > want to go for the main objective. eg rmqueue_pcplist() doesn't use
> > > > > its gfp_flags argument.
> > > >
> > > > Over all I may just be a simpleton WRT this but I'm not sure that the
> > > > added complexity justifies the gain.
> > >
> > > I'm disappointed that you see it as added complexity. I see it as reducing
> > > complexity. With this patch, we can simply pass GFP_PMD as a flag to
> > > pagecache_get_page(); without it, we have to add a fifth parameter to
> > > pagecache_get_page() and change all the callers to pass '0'.
> >
> > I don't disagree for pagecache_get_page().
> >
> > I'm not saying we should not do this. But this seems odd to me.
> >
> > Again I'm probably just being a simpleton...
>
> This concerns me, though. I see it as being a simplification, but if
> other people see it as a complication, then it's not. Perhaps I didn't
> take the patches far enough for you to see benefit? We have quite the
> thicket of .*alloc_page.* functions, and I can't keep them all straight.
> Between taking, or not taking, the nodeid, the gfp mask, the order, a VMA
> and random other crap; not to mention the NUMA vs !NUMA implementations,
> this is crying out for simplification.

Was there a new version of this coming?

Sorry perhaps I dropped the ball here by not replying?

Ira

>
> It doesn't help that I screwed up the __get_free_pages patch. I should
> have grepped and realised that we had over 200 callers and it's not
> worth changing them all as part of this patchset.
>