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 <mst@xxxxxxxxxx> 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
