From: John Hubbard <jhubbard@nvidia.com> Date: 2020-05-22 04:15:11
Hi,
Note that I have only compile-tested this series, although that does
also include cross-compiling for a few other arches. I'm hoping that
this posting will lead to some run-time testing.
Also: the proposed fix does not have a "Fixes:" tag, nor does it
Cc stable. That's because the issue has been there since the dawn of
git history for the kernel. If it's gone unnoticed this long, then
there is clearly no need for the relatively fast track of putting it
into stable, IMHO. But please correct me if that's wrong.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Daniel Vetter <redacted>
Cc: Gustavo A. R. Silva <redacted>
Cc: Jani Nikula <redacted>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
John Hubbard (2):
video: fbdev: fix error handling for get_user_pages_fast()
video: fbdev: convert get_user_pages() --> pin_user_pages()
drivers/video/fbdev/pvr2fb.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
base-commit: 051143e1602d90ea71887d92363edd539d411de5
--
2.26.2
From: John Hubbard <jhubbard@nvidia.com> Date: 2020-05-22 04:15:13
Dealing with the return value of get_user_pages*() variants has a few
classic pitfalls, and this driver found one of them: the return value
might be zero, positive, or -errno. And if positive, it might be fewer
pages than were requested. And if fewer pages than requested, then
the caller should return (via put_page()) the pages that *were*
pinned.
This driver was doing that *except* that it had a problem with the
-errno case, which was being stored in an unsigned int, and which
would case an interesting mess if it ever happened: nr_pages would be
interpreted as a spectacularly huge unsigned value, rather than a
small negative value. Also, it was unnecessarily overriding a
potentially informative -errno, with -EINVAL, in some cases.
Instead: clamp the nr_pages to zero or positive, so that the error
handling works. And return the -errno value from get_user_pages*(),
unchanged, if we get one. And explain this with comments, seeing as
how it is error-prone.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Daniel Vetter <redacted>
Cc: Gustavo A. R. Silva <redacted>
Cc: Jani Nikula <redacted>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/video/fbdev/pvr2fb.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
From: John Hubbard <jhubbard@nvidia.com> Date: 2020-05-22 04:15:13
This code was using get_user_pages*(), in a "Case 2" scenario
(DMA/RDMA), using the categorization from [1]. That means that it's
time to convert the get_user_pages*() + put_page() calls to
pin_user_pages*() + unpin_user_pages() calls.
There is some helpful background in [2]: basically, this is a small
part of fixing a long-standing disconnect between pinning pages, and
file systems' use of those pages.
[1] Documentation/core-api/pin_user_pages.rst
[2] "Explicit pinning of user-space pages":
https://lwn.net/Articles/807108/
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Daniel Vetter <redacted>
Cc: Gustavo A. R. Silva <redacted>
Cc: Jani Nikula <redacted>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/video/fbdev/pvr2fb.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
From: Sam Ravnborg <hidden> Date: 2020-05-31 20:58:26
Hi John.
On Thu, May 21, 2020 at 09:15:04PM -0700, John Hubbard wrote:
Hi,
Note that I have only compile-tested this series, although that does
also include cross-compiling for a few other arches. I'm hoping that
this posting will lead to some run-time testing.
Also: the proposed fix does not have a "Fixes:" tag, nor does it
Cc stable. That's because the issue has been there since the dawn of
git history for the kernel. If it's gone unnoticed this long, then
there is clearly no need for the relatively fast track of putting it
into stable, IMHO. But please correct me if that's wrong.
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Daniel Vetter <redacted>
Cc: Gustavo A. R. Silva <redacted>
Cc: Jani Nikula <redacted>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Thanks, patches are now applied to drm-misc-next.
They will hit -next soon, but you will have to wait
until next (not the upcoming) merge window before they hit
mainline linux.
Sam
From: John Hubbard <jhubbard@nvidia.com> Date: 2020-05-31 21:06:56
On 2020-05-31 13:58, Sam Ravnborg wrote:
...
Thanks, patches are now applied to drm-misc-next.
They will hit -next soon, but you will have to wait
until next (not the upcoming) merge window before they hit
mainline linux.
Sam
Great! That will work out just fine.
thanks,
--
John Hubbard
NVIDIA