Re: [PATCH mm-unstable v1 13/20] media: videobuf-dma-sg: remove FOLL_FORCE usage

2022-11-23 Thread Hans Verkuil 
On 16/11/2022 11:26, David Hildenbrand wrote:
> GUP now supports reliable R/O long-term pinning in COW mappings, such
> that we break COW early. MAP_SHARED VMAs only use the shared zeropage so
> far in one corner case (DAXFS file with holes), which can be ignored
> because GUP does not support long-term pinning in fsdax (see
> check_vma_flags()).
> 
> Consequently, FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM is no longer required
> for reliable R/O long-term pinning: FOLL_LONGTERM is sufficient. So stop
> using FOLL_FORCE, which is really only for ptrace access.
> 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: David Hildenbrand 

Acked-by: Hans Verkuil 

Looks good!

Hans

> ---
>  drivers/media/v4l2-core/videobuf-dma-sg.c | 14 +-
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
> b/drivers/media/v4l2-core/videobuf-dma-sg.c
> index f75e5eedeee0..234e9f647c96 100644
> --- a/drivers/media/v4l2-core/videobuf-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
> @@ -151,17 +151,16 @@ static void videobuf_dma_init(struct videobuf_dmabuf 
> *dma)
>  static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
>   int direction, unsigned long data, unsigned long size)
>  {
> + unsigned int gup_flags = FOLL_LONGTERM;
>   unsigned long first, last;
> - int err, rw = 0;
> - unsigned int flags = FOLL_FORCE;
> + int err;
>  
>   dma->direction = direction;
>   switch (dma->direction) {
>   case DMA_FROM_DEVICE:
> - rw = READ;
> + gup_flags |= FOLL_WRITE;
>   break;
>   case DMA_TO_DEVICE:
> - rw = WRITE;
>   break;
>   default:
>   BUG();
> @@ -177,14 +176,11 @@ static int videobuf_dma_init_user_locked(struct 
> videobuf_dmabuf *dma,
>   if (NULL == dma->pages)
>   return -ENOMEM;
>  
> - if (rw == READ)
> - flags |= FOLL_WRITE;
> -
>   dprintk(1, "init user [0x%lx+0x%lx => %lu pages]\n",
>   data, size, dma->nr_pages);
>  
> - err = pin_user_pages(data & PAGE_MASK, dma->nr_pages,
> -  flags | FOLL_LONGTERM, dma->pages, NULL);
> + err = pin_user_pages(data & PAGE_MASK, dma->nr_pages, gup_flags,
> +  dma->pages, NULL);
>  
>   if (err != dma->nr_pages) {
>   dma->nr_pages = (err >= 0) ? err : 0;

Re: [PATCH mm-unstable v1 13/20] media: videobuf-dma-sg: remove FOLL_FORCE usage

2022-11-16 Thread Daniel Vetter 
On Wed, Nov 16, 2022 at 11:26:52AM +0100, David Hildenbrand wrote:
> GUP now supports reliable R/O long-term pinning in COW mappings, such
> that we break COW early. MAP_SHARED VMAs only use the shared zeropage so
> far in one corner case (DAXFS file with holes), which can be ignored
> because GUP does not support long-term pinning in fsdax (see
> check_vma_flags()).
> 
> Consequently, FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM is no longer required
> for reliable R/O long-term pinning: FOLL_LONGTERM is sufficient. So stop
> using FOLL_FORCE, which is really only for ptrace access.
> 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: David Hildenbrand 

I looked at this a while ago when going through some of the follow_pfn
stuff, so

Reviewed-by: Daniel Vetter 

> ---
>  drivers/media/v4l2-core/videobuf-dma-sg.c | 14 +-
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
> b/drivers/media/v4l2-core/videobuf-dma-sg.c
> index f75e5eedeee0..234e9f647c96 100644
> --- a/drivers/media/v4l2-core/videobuf-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
> @@ -151,17 +151,16 @@ static void videobuf_dma_init(struct videobuf_dmabuf 
> *dma)
>  static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
>   int direction, unsigned long data, unsigned long size)
>  {
> + unsigned int gup_flags = FOLL_LONGTERM;
>   unsigned long first, last;
> - int err, rw = 0;
> - unsigned int flags = FOLL_FORCE;
> + int err;
>  
>   dma->direction = direction;
>   switch (dma->direction) {
>   case DMA_FROM_DEVICE:
> - rw = READ;
> + gup_flags |= FOLL_WRITE;
>   break;
>   case DMA_TO_DEVICE:
> - rw = WRITE;
>   break;
>   default:
>   BUG();
> @@ -177,14 +176,11 @@ static int videobuf_dma_init_user_locked(struct 
> videobuf_dmabuf *dma,
>   if (NULL == dma->pages)
>   return -ENOMEM;
>  
> - if (rw == READ)
> - flags |= FOLL_WRITE;
> -
>   dprintk(1, "init user [0x%lx+0x%lx => %lu pages]\n",
>   data, size, dma->nr_pages);
>  
> - err = pin_user_pages(data & PAGE_MASK, dma->nr_pages,
> -  flags | FOLL_LONGTERM, dma->pages, NULL);
> + err = pin_user_pages(data & PAGE_MASK, dma->nr_pages, gup_flags,
> +  dma->pages, NULL);
>  
>   if (err != dma->nr_pages) {
>   dma->nr_pages = (err >= 0) ? err : 0;
> -- 
> 2.38.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH mm-unstable v1 13/20] media: videobuf-dma-sg: remove FOLL_FORCE usage

2022-11-16 Thread David Hildenbrand 
GUP now supports reliable R/O long-term pinning in COW mappings, such
that we break COW early. MAP_SHARED VMAs only use the shared zeropage so
far in one corner case (DAXFS file with holes), which can be ignored
because GUP does not support long-term pinning in fsdax (see
check_vma_flags()).

Consequently, FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM is no longer required
for reliable R/O long-term pinning: FOLL_LONGTERM is sufficient. So stop
using FOLL_FORCE, which is really only for ptrace access.

Cc: Mauro Carvalho Chehab 
Signed-off-by: David Hildenbrand 
---
 drivers/media/v4l2-core/videobuf-dma-sg.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index f75e5eedeee0..234e9f647c96 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -151,17 +151,16 @@ static void videobuf_dma_init(struct videobuf_dmabuf *dma)
 static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
int direction, unsigned long data, unsigned long size)
 {
+   unsigned int gup_flags = FOLL_LONGTERM;
unsigned long first, last;
-   int err, rw = 0;
-   unsigned int flags = FOLL_FORCE;
+   int err;
 
dma->direction = direction;
switch (dma->direction) {
case DMA_FROM_DEVICE:
-   rw = READ;
+   gup_flags |= FOLL_WRITE;
break;
case DMA_TO_DEVICE:
-   rw = WRITE;
break;
default:
BUG();
@@ -177,14 +176,11 @@ static int videobuf_dma_init_user_locked(struct 
videobuf_dmabuf *dma,
if (NULL == dma->pages)
return -ENOMEM;
 
-   if (rw == READ)
-   flags |= FOLL_WRITE;
-
dprintk(1, "init user [0x%lx+0x%lx => %lu pages]\n",
data, size, dma->nr_pages);
 
-   err = pin_user_pages(data & PAGE_MASK, dma->nr_pages,
-flags | FOLL_LONGTERM, dma->pages, NULL);
+   err = pin_user_pages(data & PAGE_MASK, dma->nr_pages, gup_flags,
+dma->pages, NULL);
 
if (err != dma->nr_pages) {
dma->nr_pages = (err >= 0) ? err : 0;
-- 
2.38.1