linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: William Kucharski <william.kucharski@oracle.com>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Andrea Arcangeli <aarcange@redhat.com>, Zi Yan <ziy@nvidia.com>,
	Yang Shi <yang.shi@linux.alibaba.com>,
	Ralph Campbell <rcampbell@nvidia.com>,
	John Hubbard <jhubbard@nvidia.com>, linux-mm <linux-mm@kvack.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCHv3, RESEND 0/8] thp/khugepaged improvements and CoW semantics
Date: Tue, 14 Apr 2020 20:47:18 -0600	[thread overview]
Message-ID: <8163D0B7-988F-4713-815E-577A641B5F64@oracle.com> (raw)
In-Reply-To: <20200413100447.20073-6-kirill.shutemov@linux.intel.com>

Quick comments, mostly nits:

[PATCH 5/6] khugepaged: Allow to collapse a page shared across fork


> +static bool is_refcount_suitable(struct page *page)
> +{
> +	int expected_refcount, refcount;
> +
> +	refcount = page_count(page);
> +	expected_refcount = total_mapcount(page);
> +	if (PageSwapCache(page))
> +		expected_refcount += compound_nr(page);
> +
> +	if (IS_ENABLED(CONFIG_DEBUG_VM) && expected_refcount > refcount) {
> +		pr_err("expected_refcount: %d, refcount: %d\n",
> +				expected_refcount, refcount);

I'd rather see this message reworded slightly as I prefer to know WHY a value
was unexpected directly from the error message:

		pr_err("expected_refcount (%d) > refcount (%d)\n",

[PATCHv3, RESEND 7/8] thp: Change CoW semantics for anon-THP

I really like the simplifications here.

[PATCHv3, RESEND 8/8] khugepaged: Introduce 'max_ptes_shared' tunable

As above, I'd like to see a tiny extra bit of specificity in the printf()s:

+static void collapse_max_ptes_shared()
+{

[ ... ]

+		if (check_huge(p))
+			success("OK");
+		else
+			fail("Fail");
+
+		printf("Trigger CoW in %d of %d...",

		printf("Trigger CoW on page %d of %d...",

+				hpage_pmd_nr - max_ptes_shared - 1, hpage_pmd_nr);
+		fill_memory(p, 0, (hpage_pmd_nr - max_ptes_shared - 1) * page_size);
+		if (!check_huge(p))
+			success("OK");
+		else
+			fail("Fail");
+
+		if (wait_for_scan("Do not collapse with max_ptes_shared exeeded", p))
+			fail("Timeout");
+		else if (!check_huge(p))
+			success("OK");
+		else
+			fail("Fail");
+
+		printf("Trigger CoW in %d of %d...",

		printf("Trigger CoW on page %d of %d...",

+				hpage_pmd_nr - max_ptes_shared, hpage_pmd_nr);
+		fill_memory(p, 0, (hpage_pmd_nr - max_ptes_shared) * page_size);
+		if (!check_huge(p))
+			success("OK");
+		else
+			fail("Fail");

Otherwise, for the khugepaged/THP series:

Reviewed-by: William Kucharski <william.kucharski@oracle.com>


  reply	other threads:[~2020-04-15  2:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-13 10:04 [PATCH 0/6] thp/khugepaged improvements and CoW semantics Kirill A. Shutemov
2020-04-13 10:04 ` [PATCH 1/6] khugepaged: Add self test Kirill A. Shutemov
2020-04-13 10:04 ` [PATCH 2/6] khugepaged: Do not stop collapse if less than half PTEs are referenced Kirill A. Shutemov
2020-04-13 10:04 ` [PATCH 3/6] khugepaged: Drain all LRU caches before scanning pages Kirill A. Shutemov
2020-04-13 10:04 ` [PATCH 4/6] khugepaged: Drain LRU add pagevec after swapin Kirill A. Shutemov
2020-04-13 10:04 ` [PATCH 5/6] khugepaged: Allow to collapse a page shared across fork Kirill A. Shutemov
2020-04-15  2:47   ` William Kucharski [this message]
2020-04-13 10:04 ` [PATCH 6/6] khugepaged: Allow to collapse PTE-mapped compound pages Kirill A. Shutemov
2020-04-13 12:49 ` [PATCH 0/6] thp/khugepaged improvements and CoW semantics Kirill A. Shutemov
2020-04-13 12:52 [PATCHv3, RESEND 0/8] " Kirill A. Shutemov

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=8163D0B7-988F-4713-815E-577A641B5F64@oracle.com \
    --to=william.kucharski@oracle.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=jhubbard@nvidia.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rcampbell@nvidia.com \
    --cc=yang.shi@linux.alibaba.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).