From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by kanga.kvack.org (Postfix) with ESMTP id 4207B8E0001 for ; Tue, 18 Dec 2018 16:49:59 -0500 (EST) Received: by mail-ed1-f70.google.com with SMTP id f31so10762679edf.17 for ; Tue, 18 Dec 2018 13:49:59 -0800 (PST) Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id l37sor10028319edb.2.2018.12.18.13.49.57 for (Google Transport Security); Tue, 18 Dec 2018 13:49:57 -0800 (PST) Date: Tue, 18 Dec 2018 21:49:56 +0000 From: Wei Yang Subject: Re: [PATCH v2] mm, page_isolation: remove drain_all_pages() in set_migratetype_isolate() Message-ID: <20181218214956.svedrxevycbgwsuk@master> Reply-To: Wei Yang References: <20181214023912.77474-1-richard.weiyang@gmail.com> <20181218204656.4297-1-richard.weiyang@gmail.com> <58509504-4c30-3385-6eda-72c2abad60e7@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <58509504-4c30-3385-6eda-72c2abad60e7@redhat.com> Sender: owner-linux-mm@kvack.org List-ID: To: David Hildenbrand Cc: Wei Yang , linux-mm@kvack.org, akpm@linux-foundation.org, mhocko@suse.com, osalvador@suse.de On Tue, Dec 18, 2018 at 10:14:25PM +0100, David Hildenbrand wrote: >On 18.12.18 21:46, Wei Yang wrote: >> Below is a brief call flow for __offline_pages() and >> alloc_contig_range(): >> >> __offline_pages()/alloc_contig_range() >> start_isolate_page_range() >> set_migratetype_isolate() >> drain_all_pages() >> drain_all_pages() >> >> Current logic is: isolate and drain pcp list for each pageblock and >> drain pcp list again. This is not necessary and we could just drain pcp >> list once after isolate this whole range. >> >> The reason is start_isolate_page_range() will set the migrate type of >> a range to MIGRATE_ISOLATE. After doing so, this range will never be >> allocated from Buddy, neither to a real user nor to pcp list. >> >> Since drain_all_pages() is zone based, by reduce times of >> drain_all_pages() also reduce some contention on this particular zone. >> >> Signed-off-by: Wei Yang > >Yes, as far as I can see, when a MIGRATE_ISOLATE page gets freed, it >will not go onto the pcp list again. > >However, start_isolate_page_range() is also called via >alloc_contig_range(). Are you sure we can effectively drop the >drain_all_pages() for that call path? > alloc_contig_range() does following now: - isolate page range - do reclaim and migration - drain lru - drain pcp list If step 2 fails, it will not drain lru and pcp list. I don't see we have to drain pcp list before step 2. And after this change, it will save some effort if step 2 fails. >> >> --- >> v2: adjust changelog with MIGRATE_ISOLATE effects for the isolated range >> --- >> mm/page_isolation.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/mm/page_isolation.c b/mm/page_isolation.c >> index 43e085608846..f44c0e333bed 100644 >> --- a/mm/page_isolation.c >> +++ b/mm/page_isolation.c >> @@ -83,8 +83,6 @@ static int set_migratetype_isolate(struct page *page, int migratetype, >> } >> >> spin_unlock_irqrestore(&zone->lock, flags); >> - if (!ret) >> - drain_all_pages(zone); >> return ret; >> } >> >> > > >-- > >Thanks, > >David / dhildenb -- Wei Yang Help you, Help me