linux-kernel - Re: [RFC PATCH] mm, compaction: allow compaction for GFP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Oct 2016 08:50:19 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Vlastimil Babka <vbabka@...e.cz>
Cc:     Mel Gorman <mgorman@...e.de>, Joonsoo Kim <js1304@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] mm, compaction: allow compaction for GFP_NOFS
 requests

On Fri 07-10-16 07:27:37, Vlastimil Babka wrote:
[...]
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > index badb92bf14b4..07254a73ee32 100644
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -834,6 +834,13 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> >  		    page_count(page) > page_mapcount(page))
> >  			goto isolate_fail;
> > 
> > +		/*
> > +		 * Only allow to migrate anonymous pages in GFP_NOFS context
> > +		 * because those do not depend on fs locks.
> > +		 */
> > +		if (!(cc->gfp_mask & __GFP_FS) && page_mapping(page))
> > +			goto isolate_fail;
> 
> Unless page can acquire a page_mapping between this check and migration, I
> don't see a problem with allowing this.

It can be become swapcache but I guess this should be OK. We do not
allow to get here with GFP_NOIO and migrating swapcache pages in NOFS
mode should be OK AFAICS.

> But make sure you don't break kcompactd and manual compaction from /proc, as
> they don't currently set cc->gfp_mask. Looks like until now it was only used
> to determine direct compactor's migratetype which is irrelevant in those
> contexts.

OK, I see. This is really subtle. One way to go would be to provide a
fake gfp_mask for them. How does the following look to you?
---
diff --git a/mm/compaction.c b/mm/compaction.c
index 557c165b63ad..d1d90e96ef4b 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1779,6 +1779,7 @@ static void compact_node(int nid)
 		.mode = MIGRATE_SYNC,
 		.ignore_skip_hint = true,
 		.whole_zone = true,
+		.gfp_mask = GFP_KERNEL,
 	};
 
 
@@ -1904,6 +1905,7 @@ static void kcompactd_do_work(pg_data_t *pgdat)
 		.classzone_idx = pgdat->kcompactd_classzone_idx,
 		.mode = MIGRATE_SYNC_LIGHT,
 		.ignore_skip_hint = true,
+		.gfp_mask = GFP_KERNEL,
 
 	};
 	trace_mm_compaction_kcompactd_wake(pgdat->node_id, cc.order,
-- 
Michal Hocko
SUSE Labs