LKML Archive on lore.kernel.org help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <email@example.com> To: Linus Torvalds <firstname.lastname@example.org> Cc: Al Viro <email@example.com>, Linux Kernel Mailing List <firstname.lastname@example.org>, stable <email@example.com>, firstname.lastname@example.org, linux-mm <email@example.com>, "Kirill A. Shutemov" <firstname.lastname@example.org>, Andrew Morton <email@example.com>, Huang Ying <firstname.lastname@example.org>, Jonathan Corbet <email@example.com>, Peter Zijlstra <firstname.lastname@example.org>, Thomas Gleixner <email@example.com>, Thorsten Leemhuis <firstname.lastname@example.org> Subject: Re: [PATCH] gup: return -EFAULT on access_ok failure Date: Fri, 6 Apr 2018 00:08:08 +0300 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <CA+55aFwEqnY_Z5T-5UUwbxNJfV5MmfV=-8r73xvBnA1tnU_d_w@mail.gmail.com> On Thu, Apr 05, 2018 at 11:43:27AM -0700, Linus Torvalds wrote: > On Thu, Apr 5, 2018 at 11:28 AM, Michael S. Tsirkin <firstname.lastname@example.org> wrote: > > > > to repeat what you are saying IIUC __get_user_pages_fast returns 0 if it can't > > pin any pages and that is by design. Returning 0 on error isn't usual I think > > so I guess this behaviour should we well documented. > > Arguably it happens elsewhere too, and not just in the kernel. > "read()" at past the end of a file is not an error, you'll just get 0 > for EOF. > > So it's not really "returning 0 on error". > > It really is simply returning the number of pages it got. End of > story. That number of pages can be smaller than the requested number > of pages, and _that_ is due to some error, but note how it can return > "5" on error too - you asked for 10 pages, but the error happened in > the middle! > > So the right way to check for error is to bverify that you get the > number of pages that you asked for. If you don't, something bad > happened. > > Of course, many users don't actually care about "I didn't get > everything". They only care about "did I get _something_". Then that 0 > ends up being the error case, but note how it depends on the caller. > > > What about get_user_pages_fast though? > > We do seem to special-case the first page there. I'm not sure it's a > good idea. But like the __get_user_pages_fast(), we seem to have users > that know about the particular semantics and depend on it. > > It's all ugly, I agree. > > End result: we can't just change semantics of either of them. > > At least not without going through every single user and checking that > they are ok with it. > > Which I guess I could be ok with. Maybe changing the semantics of > __get_user_pages_fast() is acceptable, if you just change it > *everywhere* (which includes not just he users, but also the couple of > architecture-specific versions of that same function that we have. > > Linus For now I sent a patchset 1. documenting current behaviour for __get_user_pages_fast. 2. fixing get_user_pages_fast for consistency. -- MST
next prev parent reply other threads:[~2018-04-05 21:08 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <email@example.com> 2018-04-05 1:53 ` [PATCH] gup: return -EFAULT on access_ok failure Michael S. Tsirkin 2018-04-05 2:40 ` Linus Torvalds 2018-04-05 14:17 ` Michael S. Tsirkin 2018-04-05 15:40 ` Linus Torvalds 2018-04-05 18:28 ` Michael S. Tsirkin 2018-04-05 18:43 ` Linus Torvalds 2018-04-05 19:34 ` Michael S. Tsirkin 2018-04-05 19:39 ` Chris Wilson 2018-04-05 21:08 ` Michael S. Tsirkin [this message] 2018-04-06 11:35 ` Alan Cox
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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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: linkBe 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).