LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Oscar Salvador <osalvador@suse.de>
To: David Hildenbrand <david@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Michal Hocko <mhocko@kernel.org>,
	Muchun Song <songmuchun@bytedance.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 2/7] mm,compaction: Let isolate_migratepages_{range,block} return error codes
Date: Thu, 15 Apr 2021 10:42:03 +0200
Message-ID: <YHf8W9fWVIWFpG1K@localhost.localdomain> (raw)
In-Reply-To: <2628d20e-3304-1a0c-5246-cfc2320cce8b@redhat.com>

On Wed, Apr 14, 2021 at 01:54:17PM +0200, David Hildenbrand wrote:
> >    * Isolate all pages that can be migrated from the range specified by
> >    * [low_pfn, end_pfn). The range is expected to be within same pageblock.
> > - * Returns zero if there is a fatal signal pending, otherwise PFN of the
> > - * first page that was not scanned (which may be both less, equal to or more
> > - * than end_pfn).
> > + * Returns errno, like -EAGAIN or -EINTR in case e.g signal pending or congestion,
> > + * or 0.
> > + * cc->migrate_pfn will contain the next pfn to scan (which may be both less,
> > + * equal to or more that end_pfn).
> 
> I failed to parse the last sentence -- e.g., using "both" and then listing
> three options. Also, s/than/than/? Can we simplify to
> 
> "cc->migrate_pfn will contain the next pfn to scan"

Ok, will simplify

> >   /**
> > @@ -1139,15 +1144,15 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> >    * @start_pfn: The first PFN to start isolating.
> >    * @end_pfn:   The one-past-last PFN.
> >    *
> > - * Returns zero if isolation fails fatally due to e.g. pending signal.
> > - * Otherwise, function returns one-past-the-last PFN of isolated page
> > - * (which may be greater than end_pfn if end fell in a middle of a THP page).
> > + * Returns errno, like -EAGAIN or -EINTR in case e.g signal pending or congestion,
> 
> errno is usually positive.
> 
> > + * or 0.
> 
> I'd be more specific here. Something like
> 
> "
> Returns 0 if isolation succeeded. Returns -EINTR if a fatal signal is
> pending. Returns -EAGAIN when contended and the caller should retry.
> 
> In any case, cc->migrate_pfn is set to one-past-the-last PFN that has been
> isolated.
> "

I will try to reword it. Although note that 0 does not mean the
isolation succeeded.
Compaction tracks whether we could isolate pages by means of
cc->nr_migratepages.

The thing is that alloc_contig_range and compaction code need different
things.
Compaction wants to isolate and migrate as much as it needs to in order to
form a higher-order page (or in case
if it is trying to compact the whole zone, it goes through the zone).
alloc_contig_range() needs to isolate the whole range we specified in
order to be able to migrate it and make it free for whoever wants to use
it.

Let us say that the interface between alloc_contig_range() and
compaction still needs some more love that I plan to work on when this goes
in.


-- 
Oscar Salvador
SUSE L3

  reply index

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-13 10:47 [PATCH v7 0/7] Make alloc_contig_range handle Hugetlb pages Oscar Salvador
2021-04-13 10:47 ` [PATCH v7 1/7] mm,page_alloc: Bail out earlier on -ENOMEM in alloc_contig_migrate_range Oscar Salvador
2021-04-13 17:00   ` Mike Kravetz
2021-04-13 10:47 ` [PATCH v7 2/7] mm,compaction: Let isolate_migratepages_{range,block} return error codes Oscar Salvador
2021-04-13 17:52   ` Mike Kravetz
2021-04-14 11:54   ` David Hildenbrand
2021-04-15  8:42     ` Oscar Salvador [this message]
2021-04-13 10:47 ` [PATCH v7 3/7] mm,hugetlb: Clear HPageFreed outside of the lock Oscar Salvador
2021-04-13 13:23   ` Michal Hocko
2021-04-13 21:19     ` Mike Kravetz
2021-04-14  6:04       ` Michal Hocko
2021-04-14  7:41         ` Oscar Salvador
2021-04-14  8:28           ` Michal Hocko
2021-04-14 10:01             ` Oscar Salvador
2021-04-14 10:03               ` Oscar Salvador
2021-04-14 10:32               ` Michal Hocko
2021-04-14 10:49                 ` Oscar Salvador
2021-04-14 11:09                   ` Michal Hocko
2021-04-14 12:02                     ` David Hildenbrand
2021-04-14 16:45                   ` Mike Kravetz
2021-04-13 10:47 ` [PATCH v7 4/7] mm,hugetlb: Split prep_new_huge_page functionality Oscar Salvador
2021-04-13 13:24   ` Michal Hocko
2021-04-13 13:26     ` Michal Hocko
2021-04-13 21:33   ` Mike Kravetz
2021-04-14  4:59     ` Oscar Salvador
2021-04-14 12:15       ` David Hildenbrand
2021-04-14 17:03       ` Mike Kravetz
2021-04-13 10:47 ` [PATCH v7 5/7] mm: Make alloc_contig_range handle free hugetlb pages Oscar Salvador
2021-04-13 13:40   ` Michal Hocko
2021-04-15  8:29     ` Oscar Salvador
2021-04-13 22:29   ` Mike Kravetz
2021-04-14  4:54     ` Oscar Salvador
2021-04-14 12:26   ` David Hildenbrand
2021-04-15  8:28     ` Oscar Salvador
2021-04-13 10:47 ` [PATCH v7 6/7] mm: Make alloc_contig_range handle in-use " Oscar Salvador
2021-04-13 22:48   ` Mike Kravetz
2021-04-14  4:52     ` Oscar Salvador
2021-04-14 17:07       ` Mike Kravetz
2021-04-13 10:47 ` [PATCH v7 7/7] mm,page_alloc: Drop unnecessary checks from pfn_range_valid_contig Oscar Salvador
2021-04-13 22:53   ` Mike Kravetz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YHf8W9fWVIWFpG1K@localhost.localdomain \
    --to=osalvador@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=mike.kravetz@oracle.com \
    --cc=songmuchun@bytedance.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git