On Wed, Feb 13, 2019 at 03:41:02PM +0100, Jan Kara wrote: > On Tue 12-02-19 10:34:54, Matthew Wilcox wrote: > > Transparent Huge Pages are currently stored in i_pages as pointers to > > consecutive subpages. This patch changes that to storing consecutive > > pointers to the head page in preparation for storing huge pages more > > efficiently in i_pages. > > > > Large parts of this are "inspired" by Kirill's patch > > https://lore.kernel.org/lkml/20170126115819.58875-2-kirill.shutemov@xxxxxxxxxxxxxxx/ > > > > Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxx> > > I like the idea! Thanks! It's a step towards reducing the overhead of huge pages in the page cache from (on x86) 520 pointers to a mere 8. Still not as good as hugetlbfs, but I'm working on that too. > > - pages[ret] = page; > > + pages[ret] = find_subpage(page, xas.xa_index); > > if (++ret == nr_pages) { > > *start = page->index + 1; > > goto out; > > } > > So this subtly changes the behavior because now we will be returning in > '*start' a different index. So you should rather use 'pages[ret]->index' > instead. You're right, I made a mistake there. However, seeing this: https://lore.kernel.org/lkml/20190110030838.84446-1-yuzhao@xxxxxxxxxx/ makes me think that I should be using xa_index + 1 there. > Otherwise the patch looks good to me so feel free to add: > > Acked-by: Jan Kara <jack@xxxxxxx> > > after fixing these two. Thanks!
