From: Jason Gunthorpe <jgg@nvidia.com>
To: John Hubbard <jhubbard@nvidia.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux-MM <linux-mm@kvack.org>, Peter Xu <peterx@redhat.com>,
Alex Williamson <alex.williamson@redhat.com>,
Andrea Arcangeli <aarcange@redhat.com>,
David Hildenbrand <david@redhat.com>, Jan Kara <jack@suse.cz>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: Weird code with change "mm/gup: clean up follow_pfn_pte() slightly"
Date: Thu, 3 Feb 2022 09:01:37 -0400 [thread overview]
Message-ID: <20220203130137.GZ1786498@nvidia.com> (raw)
In-Reply-To: <6d38ed2a-72cb-3eb6-5af1-caee61d94005@nvidia.com>
On Thu, Feb 03, 2022 at 12:38:33AM -0800, John Hubbard wrote:
> On 2/2/22 22:27, Lukas Bulwahn wrote:
> > Dear John,
> >
> > Your change "mm/gup: clean up follow_pfn_pte() slightly" (see Link),
> > visible in linux-next as commit 05fef840b5c6 ("mm/gup: clean up
> > follow_pfn_pte() slightly"), is somehow weird.
>
> Well. That sounds like something to be avoided. :)
>
> >
> > In the new branch if (pages), you set page = ERR_PTR(-EFAULT) and goto
> > out. However, at the label out, the value of page is not used, but the
> > return uses the variables i and ret.
>
> Yes, I think that the complaint is accurate. The intent of this code is
> to return either number of pages so far (i) or ret (which should be zero
> in this case), because we are just stopping early, rather than calling
> this an actual error.
IIRC GUP shouldn't return 0, it should return an error code, not zero.
Jason
next prev parent reply other threads:[~2022-02-03 13:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-03 6:27 Weird code with change "mm/gup: clean up follow_pfn_pte() slightly" Lukas Bulwahn
2022-02-03 8:38 ` John Hubbard
2022-02-03 13:01 ` Jason Gunthorpe [this message]
2022-02-03 20:44 ` John Hubbard
2022-02-04 0:45 ` Jason Gunthorpe
2022-02-04 0:59 ` John Hubbard
2022-02-04 1:06 ` Jason Gunthorpe
2022-02-04 1:22 ` John Hubbard
2022-02-04 1:26 ` Jason Gunthorpe
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=20220203130137.GZ1786498@nvidia.com \
--to=jgg@nvidia.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=alex.williamson@redhat.com \
--cc=david@redhat.com \
--cc=jack@suse.cz \
--cc=jhubbard@nvidia.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lukas.bulwahn@gmail.com \
--cc=peterx@redhat.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.