All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Baolin Wang <baolin.wang@linux.alibaba.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: ziy@nvidia.com, shy828301@gmail.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] mm: migrate: Correct the hugetlb migration stats
Date: Tue, 23 Nov 2021 11:25:06 -0800	[thread overview]
Message-ID: <3e6dcac6-c947-5f94-cd94-b59a8247dbcf@oracle.com> (raw)
In-Reply-To: <71816b8f-93e5-5a2a-e616-d52a1c4d354c@linux.alibaba.com>

On 11/15/21 22:03, Baolin Wang wrote:
> 
> 
> On 2021/11/16 12:21, Andrew Morton wrote:
>> On Sun,  7 Nov 2021 16:57:26 +0800 Baolin Wang <baolin.wang@linux.alibaba.com> wrote:
>>
>>> Correct the migration stats for hugetlb with using compound_nr() instead
>>> of thp_nr_pages(),
>>
>> It would be helpful to explain why using thp_nr_pages() was wrong.
> 
> Sure. Using thp_nr_pages() to get the number of subpages for a hugetlb is incorrect, since the number of subpages in te hugetlb is not always HPAGE_PMD_NR.
> 

Correct.  However, prior to this patch the return value from thp_nr_pages
was never used for hugetlb pages; only THP.  So, this really did not have any
bad side effects prior to this patch that I can see.

>> And to explain the end user visible effects of this bug so we can
> 
> Actually not also user visible effect, but also hugetlb migration stats in kernel are incorrect. For he end user visible effects, like I described in patch 1,  the syscall move_pages() can return a non-migrated number larger than the number of pages the users tried to migrate, when a THP page is failed to migrate. This is confusing for users.
> 

It looks like hugetlb pages were never taken into account when originally
defining the migration stats.  In the documentation (page_migration.rst) it
only talks about Normal and THP pages.  It does not mention how hugetlb pages
are counted.

Currently, hugetlb pages count as 'a single page' in the stats
PGMIGRATE_SUCCESS/FAIL.  Correct?  After this change we will increment these
stats by the number of sub-pages.  Correct?

I 'think' this is OK since the behavior is not really defined today.  But, we
are changing user visible output.

Perhaps we should go ahead and document the hugetlb behavior when making these
changes?
-- 
Mike Kravetz

  reply	other threads:[~2021-11-23 19:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-07  8:57 [PATCH 0/3] Improve the migration stats Baolin Wang
2021-11-07  8:57 ` [PATCH 1/3] mm: migrate: Fix the return value of migrate_pages() Baolin Wang
2021-11-09 18:10   ` Zi Yan
2021-11-23 18:46   ` Mike Kravetz
2021-11-24 10:30     ` Baolin Wang
2021-11-24 18:46       ` Mike Kravetz
2021-11-07  8:57 ` [PATCH 2/3] mm: migrate: Correct the hugetlb migration stats Baolin Wang
2021-11-16  4:21   ` Andrew Morton
2021-11-16  6:03     ` Baolin Wang
2021-11-23 19:25       ` Mike Kravetz [this message]
2021-11-24 10:47         ` Baolin Wang
2021-11-24 19:05           ` Mike Kravetz
2021-11-25  5:50             ` Baolin Wang
2021-11-07  8:57 ` [PATCH 3/3] mm: compaction: Fix the migration stats in trace_mm_compaction_migratepages() Baolin Wang

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=3e6dcac6-c947-5f94-cd94-b59a8247dbcf@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shy828301@gmail.com \
    --cc=ziy@nvidia.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.