Re: [PATCH RFC] mm: compaction: avoid migrating non-cma pages to a cma area

[Roman Gushchin]

On Tue, Apr 14, 2020 at 01:49:45PM +0200, Vlastimil Babka wrote:
> On 4/8/20 9:41 PM, Roman Gushchin wrote:
> > Compaction does treat cma pageblocks on pair with any movable
> > pageblocks. It means it can easily move non-cma pages into a cma zone.
> > 
> > It can create problems for the cma allocator.
> > 
> > The particular problem I'm looking at is related to btrfs metadata
> > pages, which are allocated without __GFP_MOVABLE, but beside that
> > are generic pagecache pages. In fact, they are sometimes movable
> > and sometimes not, depending on whether they are dirty and also
> > on the extent buffer reference counter.
>

Hello, Vlastimil!

Thank you for looking  into it.

> Hm I think I'd rather make such pages really unmovable (by a pin?) than deny the
> whole CMA area to compaction. Would it be feasible?

Well, it's an option too, however I'm not sure it's the best one.
First, because these pages can be moved quite often, making
them completely unmovable will make the compaction less efficient.
Second, because it's not only about these pages, but in general
about migrating pages into a cma area without a clear need.

As I wrote in the commit log, if a cma area is located close to end
of a node (which seems to be default at least on x86 without a movable
zone), compaction can fill it quite aggressively. And it might bring
some hot pages (e.g. executable pagecache pages), which will cause
cma allocation failures. I've seen something like this in our prod.

> 
> > Compaction moves them to the hugetlb_cma area, and then sometimes
> > the cma allocator fails to move them back from the cma area. It
> > results in failures of gigantic hugepages allocations.
> > 
> > Also in general cma areas are reserved close to the end of a zone,
> > and it's where compaction tries to migrate pages. It means
> > compaction will aggressively fill cma areas, which makes not much
> > sense.
> 
> So now the free page scanner will have to skip those areas, which is not much
> effective. But I suspect a worse problem in __compaction_suitable() which will
> now falsely report that there are enough free pages, so compaction will start
> but fail to do anytning. Minimally the __zone_watermark_ok() check there would
> have to lose ALLOC_CMA, but there might be other similar checks that would need
> adjusting.

A really good point! I've looked around for any other checks, but haven't found
anything. Please, find an updated version of the patch below.

> 
> And long-term what happens if the "CMA using ZONE_MOVABLE" approach is merged
> and there are not more CMA migratetypes to test? Might this change actually also
> avoid your issue, as said pages without __GFP_MOVABLE won't end up in a
> ZONE_MOVABLE?

Yeah, this is what I was thinking about. Basically I want to mimic this behavior
right now. Once this approach will be implemented and merged, it will happen
automatically: obviously, compaction won't move pages between different zones.

Thank you!

--