Re: [PATCH v2 2/4] mm: fail get_vaddr_frames() for filesystem-dax mappings

[Dan Williams]

On Mon, Nov 27, 2017 at 8:15 AM, Jan Kara  wrote:
> On Tue 14-11-17 11:56:39, Dan Williams wrote:
>> Until there is a solution to the dma-to-dax vs truncate problem it is
>> not safe to allow V4L2, Exynos, and other frame vector users to create
>> long standing / irrevocable memory registrations against filesytem-dax
>> vmas.
>>
>> Cc: Inki Dae 
>> Cc: Seung-Woo Kim 
>> Cc: Joonyoung Shim 
>> Cc: Kyungmin Park 
>> Cc: Mauro Carvalho Chehab 
>> Cc: linux-media@vger.kernel.org
>> Cc: Jan Kara 
>> Cc: Mel Gorman 
>> Cc: Vlastimil Babka 
>> Cc: Andrew Morton 
>> Cc: 
>> Fixes: 3565fce3a659 ("mm, x86: get_user_pages() for dax mappings")
>> Signed-off-by: Dan Williams 
>
> Makes sense. I'd just note that in principle get_vaddr_frames() is no more
> long-term than get_user_pages(). It is just so that all the users of
> get_vaddr_frames() currently want a long-term reference. Maybe could you
> add here also a comment that the vma_is_fsdax() check is there because all
> users of this function want a long term page reference? With that you can
> add:

Ok, will do.

> Reviewed-by: Jan Kara 

Thanks.

Re: [PATCH v2 2/4] mm: fail get_vaddr_frames() for filesystem-dax mappings

[Jan Kara]

On Tue 14-11-17 11:56:39, Dan Williams wrote:
> Until there is a solution to the dma-to-dax vs truncate problem it is
> not safe to allow V4L2, Exynos, and other frame vector users to create
> long standing / irrevocable memory registrations against filesytem-dax
> vmas.
> 
> Cc: Inki Dae 
> Cc: Seung-Woo Kim 
> Cc: Joonyoung Shim 
> Cc: Kyungmin Park 
> Cc: Mauro Carvalho Chehab 
> Cc: linux-media@vger.kernel.org
> Cc: Jan Kara 
> Cc: Mel Gorman 
> Cc: Vlastimil Babka 
> Cc: Andrew Morton 
> Cc: 
> Fixes: 3565fce3a659 ("mm, x86: get_user_pages() for dax mappings")
> Signed-off-by: Dan Williams 

Makes sense. I'd just note that in principle get_vaddr_frames() is no more
long-term than get_user_pages(). It is just so that all the users of
get_vaddr_frames() currently want a long-term reference. Maybe could you
add here also a comment that the vma_is_fsdax() check is there because all
users of this function want a long term page reference? With that you can
add:

Reviewed-by: Jan Kara 

Honza

> ---
>  mm/frame_vector.c |4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/mm/frame_vector.c b/mm/frame_vector.c
> index 72ebec18629c..d2fdbeaadc8b 100644
> --- a/mm/frame_vector.c
> +++ b/mm/frame_vector.c
> @@ -52,6 +52,10 @@ int get_vaddr_frames(unsigned long start, unsigned int 
> nr_frames,
>   ret = -EFAULT;
>   goto out;
>   }
> +
> + if (vma_is_fsdax(vma))
> + return -EOPNOTSUPP;
> +
>   if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
>   vec->got_ref = true;
>   vec->is_pfns = false;
> 
-- 
Jan Kara 
SUSE Labs, CR

[PATCH v2 2/4] mm: fail get_vaddr_frames() for filesystem-dax mappings

[Dan Williams]

Until there is a solution to the dma-to-dax vs truncate problem it is
not safe to allow V4L2, Exynos, and other frame vector users to create
long standing / irrevocable memory registrations against filesytem-dax
vmas.

Cc: Inki Dae 
Cc: Seung-Woo Kim 
Cc: Joonyoung Shim 
Cc: Kyungmin Park 
Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Cc: Jan Kara 
Cc: Mel Gorman 
Cc: Vlastimil Babka 
Cc: Andrew Morton 
Cc: 
Fixes: 3565fce3a659 ("mm, x86: get_user_pages() for dax mappings")
Signed-off-by: Dan Williams 
---
 mm/frame_vector.c |4 
 1 file changed, 4 insertions(+)

diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 72ebec18629c..d2fdbeaadc8b 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -52,6 +52,10 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
ret = -EFAULT;
goto out;
}
+
+   if (vma_is_fsdax(vma))
+   return -EOPNOTSUPP;
+
if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
vec->got_ref = true;
vec->is_pfns = false;