linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-perf-users@vger.kernel.org
Subject: [PATCH] [suggestion] mm/gup: avoid IS_ERR_OR_NULL
Date: Fri, 19 May 2023 11:39:13 +0200	[thread overview]
Message-ID: <20230519093953.10972-1-arnd@kernel.org> (raw)

From: Arnd Bergmann <arnd@arndb.de>

While looking at an unused-variable warning, I noticed a new interface coming
in that requires the use of IS_ERR_OR_NULL(), which tends to indicate bad
interface design and is usually surprising to users.

Change get_user_page_vma_remote() to return -EIO when no pages were
found and adapt the callers to match.

Fixes: eca1a00155df ("mm/gup: remove vmas parameter from get_user_pages_remote()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I see the real bug is already fixed, but this seemed worth pointing out still.
Not sure if this is the best way to handle the return types here, but the version
in linux-next doesn't look great either.
---
 arch/arm64/kernel/mte.c | 4 ++--
 include/linux/mm.h      | 2 +-
 kernel/events/uprobes.c | 5 ++++-
 mm/memory.c             | 2 +-
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 4c5ef9b20065..6983ba35ce16 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -434,8 +434,8 @@ static int __access_remote_tags(struct mm_struct *mm, unsigned long addr,
 		struct page *page = get_user_page_vma_remote(mm, addr,
 							     gup_flags, &vma);
 
-		if (IS_ERR_OR_NULL(page)) {
-			err = page == NULL ? -EIO : PTR_ERR(page);
+		if (IS_ERR(page)) {
+			err = PTR_ERR(page);
 			break;
 		}
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 42ff3e04c006..4bb172e4818c 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2397,7 +2397,7 @@ static inline struct page *get_user_page_vma_remote(struct mm_struct *mm,
 	if (got < 0)
 		return ERR_PTR(got);
 	if (got == 0)
-		return NULL;
+		return ERR_PTR(-EIO);
 
 	vma = vma_lookup(mm, addr);
 	if (WARN_ON_ONCE(!vma)) {
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index cac3aef7c6f7..9cf2d4ba760e 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -474,7 +474,10 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm,
 		gup_flags |= FOLL_SPLIT_PMD;
 	/* Read the page with vaddr into memory */
 	old_page = get_user_page_vma_remote(mm, vaddr, gup_flags, &vma);
-	if (IS_ERR_OR_NULL(old_page))
+	if (old_page == ERR_PTR(-EIO))
+		return 0;
+
+	if (IS_ERR(old_page))
 		return PTR_ERR(old_page);
 
 	ret = verify_opcode(old_page, vaddr, &opcode);
diff --git a/mm/memory.c b/mm/memory.c
index 8358f3b853f2..f9a81278e76d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5604,7 +5604,7 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 		struct page *page = get_user_page_vma_remote(mm, addr,
 							     gup_flags, &vma);
 
-		if (IS_ERR_OR_NULL(page)) {
+		if (IS_ERR(page)) {
 #ifndef CONFIG_HAVE_IOREMAP_PROT
 			break;
 #else
-- 
2.39.2


             reply	other threads:[~2023-05-19  9:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-19  9:39 Arnd Bergmann [this message]
2023-05-19 14:51 ` [PATCH] [suggestion] mm/gup: avoid IS_ERR_OR_NULL Lorenzo Stoakes
2023-05-19 15:09   ` Arnd Bergmann
2023-05-19 15:30     ` Lorenzo Stoakes
2023-05-19 22:17   ` Jason Gunthorpe
2023-05-20  5:19     ` Lorenzo Stoakes
2023-05-20  8:25       ` Jason Gunthorpe
2023-05-20  9:12         ` Lorenzo Stoakes
2023-05-27  9:52           ` Jason Gunthorpe
2023-05-28 15:13             ` Lorenzo Stoakes
2023-05-28 16:22               ` Jason Gunthorpe
2023-05-28 23:07                 ` John Hubbard

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=20230519093953.10972-1-arnd@kernel.org \
    --to=arnd@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=will@kernel.org \
    /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).