On Tue, Mar 07, 2023 at 07:30:37AM +0000, Matthew Wilcox wrote:
> On Tue, Mar 07, 2023 at 07:55:32AM +0100, Hannes Reinecke wrote:
> > On 3/6/23 18:37, Matthew Wilcox wrote:
> > > On Mon, Mar 06, 2023 at 01:01:23PM +0100, Hannes Reinecke wrote:
> > > > - page = alloc_page(gfp | __GFP_ZERO | __GFP_HIGHMEM);
> > > > - if (!page)
> > > > + folio = folio_alloc(gfp | __GFP_ZERO, 0);
> > > > + if (!folio)
> > >
> > > Did you drop HIGHMEM support on purpose?
> >
> > No; I thought that folios would be doing that implicitely.
> > Will be re-adding.
>
> We can't ... not all filesystems want to allocate every folio from
> HIGHMEM. eg for superblocks, it often makes more sense to allocate the
> folio from lowmem than allocate it from highmem and keep it kmapped.
> The only GFP flag that folios force-set is __GFP_COMP because folios by
> definition are compound pages.
Just some historical information here. Some commit logs would seem
to make it seem that __GFP_HIGHMEM was added to support DAX, but it's
not the case. When DAX suport was removed from brd the __GFP_HIGHMEM
removed was removed by mistake, and later fixed through commit
f6b50160a06d4 ("brd: re-enable __GFP_HIGHMEM in brd_insert_page()").
But that doesn't tell us what the default were before.
To avoid future removals maybe we should document why we care?
See these commits:
9db5579be4bb5 ("rewrite rd") initial commit with highmem
cb9cd2ef0bbb4 ("rd: support XIP") forces highmem if XIP, but already set
a7a97fc9ff6c2 ("brd: rename XIP to DAX") renames the config to DAX
26defe34e48e1 ("fix brd allocation flags") tries to fix the confusion but makes it worse
And so commit f6b50160a06d4 ("brd: re-enable __GFP_HIGHMEM in brd_insert_page()")
seems to be correct in re-adding it, adn commit cb9cd2ef0bbb4 ("rd: support XIP")
made things confusing assuming we wanted only highmem when DAX was
enabled.
So we only want highmem just because it's been there since it was first added.
That's all.
Luis
