All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zach O'Keefe" <zokeefe@google.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, Yang Shi <shy828301@gmail.com>
Subject: [PATCH mm-unstable] mm/khugepaged: don't check pmd value twice in collapse_pte_mapped_thp()
Date: Mon, 26 Sep 2022 20:38:54 -0700	[thread overview]
Message-ID: <20220927033854.477018-1-zokeefe@google.com> (raw)

During the v3 -> v4 merge of series "mm: add file/shmem support to
MADV_COLLAPSE", a line deletion was accidentally dropped, which left
collapse_pte_mapped_thp() in state:

---8<---
	<mmap_lock held exclusively>

	result = find_pmd_or_thp_or_none(mm, haddr, &pmd);

	<other checks that don't change "result" unless fail>

(*)	if (find_pmd_or_thp_or_none(mm, haddr, &pmd) != SCAN_SUCCEED)
	result = find_pmd_or_thp_or_none(mm, haddr, &pmd);

	switch (result) {
		case SCAN_SUCCEED:
---8<---

Where (*) is the line that should have been deleted (note that this is
still legal C).  Since the selftests are still passing, this mistake
highlights the fact that the second (intended) find_pmd_or_thp_or_none()
isn't necessary: the first one is called with mmap_lock held
exclusively, we don't drop the lock in this function, and "result" isn't
changed between assignment and use in the switch-statement (except for
failure paths which either return early or skip to "drop_hpage" label).

Remove the second (intended) find_pmd_or_thp_or_none() check, and the
misplaced find_pmd_or_thp_or_none() if-statement line.

Fixes: c27451af51cc ("mm/madvise: add file and shmem support to MADV_COLLAPSE")
Signed-off-by: Zach O'Keefe <zokeefe@google.com>
---

Andrew, apologies about letting this sneak through (both the needless check,
and the trivial merge error).  Please consider taking into mm-unstable to
clean things up there.  Will try to catch these better in the future. Thank you.

 mm/khugepaged.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 6d2dfd96608a..c7699fabf302 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1463,8 +1463,6 @@ int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
 		goto drop_hpage;
 	}
 
-	if (find_pmd_or_thp_or_none(mm, haddr, &pmd) != SCAN_SUCCEED)
-	result = find_pmd_or_thp_or_none(mm, haddr, &pmd);
 	switch (result) {
 	case SCAN_SUCCEED:
 		break;
-- 
2.37.3.998.g577e59143f-goog



                 reply	other threads:[~2022-09-27  3:38 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20220927033854.477018-1-zokeefe@google.com \
    --to=zokeefe@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-mm@kvack.org \
    --cc=shy828301@gmail.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.