Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Oscar Salvador <osalvador@suse.de>
To: Michal Hocko <mhocko@kernel.org>
Cc: Wei Yang <richard.weiyang@gmail.com>,
	akpm@linux-foundation.org, vbabka@suse.cz,
	pavel.tatashin@microsoft.com, rppt@linux.vnet.ibm.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mm, page_alloc: Fix has_unmovable_pages for HugePages
Date: Thu, 20 Dec 2018 15:21:27 +0100
Message-ID: <20181220142124.r34fnuv6b33luj5a@d104.suse.de> (raw)
In-Reply-To: <20181220134132.6ynretwlndmyupml@d104.suse.de>

On Thu, Dec 20, 2018 at 02:41:32PM +0100, Oscar Salvador wrote:
> On Thu, Dec 20, 2018 at 02:06:06PM +0100, Michal Hocko wrote:
> > You did want iter += skip_pages - 1 here right?
> 
> Bleh, yeah.
> I am taking vacation today so my brain has left me hours ago, sorry.
> Should be:
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 4812287e56a0..0634fbdef078 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -8094,7 +8094,7 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
>                                 goto unmovable;
>  
>                         skip_pages = (1 << compound_order(head)) - (page - head);
> -                       iter = round_up(iter + 1, skip_pages) - 1;
> +                       iter += skip_pages - 1;
>                         continue;
>                 }

On a second thought, I think it should not really matter.

AFAICS, we can have these scenarios:

1) the head page is the first page in the pabeblock
2) first page in the pageblock is not a head but part of a hugepage
3) the head is somewhere within the pageblock

For cases 1) and 3), iter will just get the right value and we will
break the loop afterwards.

In case 2), iter will be set to a value to skip over the remaining pages.

I am assuming that hugepages are allocated and packed together.

Note that I am not against the change, but I just wanted to see if there is
something I am missing.
-- 
Oscar Salvador
SUSE L3

  reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-17 22:51 Oscar Salvador
2018-12-17 23:07 ` Andrew Morton
2018-12-18  7:36   ` Michal Hocko
2018-12-18 21:46     ` Andrew Morton
2018-12-18 21:51   ` Oscar Salvador
2018-12-19 14:25 ` Wei Yang
2018-12-19 14:28   ` Wei Yang
2018-12-19 23:39   ` Oscar Salvador
2018-12-20  9:12     ` Michal Hocko
2018-12-20 12:49       ` Oscar Salvador
2018-12-20 13:06         ` Michal Hocko
2018-12-20 13:41           ` Oscar Salvador
2018-12-20 14:21             ` Oscar Salvador [this message]
2018-12-20 14:39               ` Michal Hocko
2018-12-20 15:37                 ` Oscar Salvador
2018-12-20 15:32               ` Wei Yang
2018-12-20 15:32                 ` Wei Yang
2018-12-20 15:52                 ` Oscar Salvador
2018-12-20 13:08         ` Wei Yang
2018-12-20 13:49           ` Oscar Salvador

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=20181220142124.r34fnuv6b33luj5a@d104.suse.de \
    --to=osalvador@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=pavel.tatashin@microsoft.com \
    --cc=richard.weiyang@gmail.com \
    --cc=rppt@linux.vnet.ibm.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

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

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

Example config snippet for mirrors

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


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