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. 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.
