All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-15 15:45 ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-15 15:45 UTC (permalink / raw)
  To: akpm, willy, mhocko, kirill.shutemov, vbabka, riel, sfr, rppt,
	peterz, linux, robin.murphy, iamjoonsoo.kim, treding, keescook,
	m.szyprowski, stefanr, hjc, heiko, airlied,
	oleksandr_andrushchenko, joro, pawel, kyungmin.park, mchehab,
	boris.ostrovsky, jgross
  Cc: linux-kernel, linux-mm, linux-arm-kernel, linux1394-devel,
	dri-devel, linux-rockchip, xen-devel, iommu, linux-media

Previouly drivers have their own way of mapping range of
kernel pages/memory into user vma and this was done by
invoking vm_insert_page() within a loop.

As this pattern is common across different drivers, it can
be generalized by creating a new function and use it across
the drivers.

vm_insert_range is the new API which will be used to map a
range of kernel memory/pages to user vma.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Reviewed-by: Matthew Wilcox <willy@infradead.org>
---
 include/linux/mm_types.h |  3 +++
 mm/memory.c              | 28 ++++++++++++++++++++++++++++
 mm/nommu.c               |  7 +++++++
 3 files changed, 38 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5ed8f62..15ae24f 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
 extern void tlb_finish_mmu(struct mmu_gather *tlb,
 				unsigned long start, unsigned long end);
 
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count);
+
 static inline void init_tlb_flush_pending(struct mm_struct *mm)
 {
 	atomic_set(&mm->tlb_flush_pending, 0);
diff --git a/mm/memory.c b/mm/memory.c
index 15c417e..da904ed 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
 }
 
 /**
+ * vm_insert_range - insert range of kernel pages into user vma
+ * @vma: user vma to map to
+ * @addr: target user address of this page
+ * @pages: pointer to array of source kernel pages
+ * @page_count: no. of pages need to insert into user vma
+ *
+ * This allows drivers to insert range of kernel pages they've allocated
+ * into a user vma. This is a generic function which drivers can use
+ * rather than using their own way of mapping range of kernel pages into
+ * user vma.
+ */
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count)
+{
+	unsigned long uaddr = addr;
+	int ret = 0, i;
+
+	for (i = 0; i < page_count; i++) {
+		ret = vm_insert_page(vma, uaddr, pages[i]);
+		if (ret < 0)
+			return ret;
+		uaddr += PAGE_SIZE;
+	}
+
+	return ret;
+}
+
+/**
  * vm_insert_page - insert single page into user vma
  * @vma: user vma to map to
  * @addr: target user address of this page
diff --git a/mm/nommu.c b/mm/nommu.c
index 749276b..d6ef5c7 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
 }
 EXPORT_SYMBOL(vm_insert_page);
 
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count)
+{
+	return -EINVAL;
+}
+EXPORT_SYMBOL(vm_insert_range);
+
 /*
  *  sys_brk() for the most part doesn't need the global kernel
  *  lock, except when an application is doing something nasty
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-15 15:45 ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-15 15:45 UTC (permalink / raw)
  To: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	willy-wEGCiKHe2LqWVfeAwA7xHQ, mhocko-IBi9RG/b67k,
	kirill.shutemov-VuQAYsv1563Yd54FQh9/CA, vbabka-AlSwsSmVLrQ,
	riel-ebMLmSuQjDVBDgjK7y7TUQ, sfr-3FnU+UHB4dNDw9hX6IcOSA,
	rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	peterz-wEGCiKHe2LqWVfeAwA7xHQ, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	robin.murphy-5wv7dgnIgG8, iamjoonsoo.kim-Hm3cg6mZ9cc,
	treding-DDmLM1+adcrQT0dZR+AlfA, keescook-F7+t8E8rja9g9hUCZPvPmw,
	m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
	stefanr-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB,
	hjc-TNX95d0MmH7DzftRWevZcw, heiko-4mtYJXux2i+zQB+pC5nmwQ,
	airlied-cv59FeDIM0c, oleksandr_andrushchenko-uRwfk40T5oI,
	joro-zLv9SwRftAIdnm+yROfE0A, pawel-FA/gS7QP4orQT0dZR+AlfA,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	boris.ostrovsky-QHcLZuEGTsvQT0dZR+AlfA, jgross-IBi9RG/b67k
  Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	xen-devel-GuqFBffKawuEi8DpZVb4nw,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-media-u79uwXL29TY76Z2rM5mHXA

Previouly drivers have their own way of mapping range of
kernel pages/memory into user vma and this was done by
invoking vm_insert_page() within a loop.

As this pattern is common across different drivers, it can
be generalized by creating a new function and use it across
the drivers.

vm_insert_range is the new API which will be used to map a
range of kernel memory/pages to user vma.

Signed-off-by: Souptick Joarder <jrdr.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
---
 include/linux/mm_types.h |  3 +++
 mm/memory.c              | 28 ++++++++++++++++++++++++++++
 mm/nommu.c               |  7 +++++++
 3 files changed, 38 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5ed8f62..15ae24f 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
 extern void tlb_finish_mmu(struct mmu_gather *tlb,
 				unsigned long start, unsigned long end);
 
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count);
+
 static inline void init_tlb_flush_pending(struct mm_struct *mm)
 {
 	atomic_set(&mm->tlb_flush_pending, 0);
diff --git a/mm/memory.c b/mm/memory.c
index 15c417e..da904ed 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
 }
 
 /**
+ * vm_insert_range - insert range of kernel pages into user vma
+ * @vma: user vma to map to
+ * @addr: target user address of this page
+ * @pages: pointer to array of source kernel pages
+ * @page_count: no. of pages need to insert into user vma
+ *
+ * This allows drivers to insert range of kernel pages they've allocated
+ * into a user vma. This is a generic function which drivers can use
+ * rather than using their own way of mapping range of kernel pages into
+ * user vma.
+ */
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count)
+{
+	unsigned long uaddr = addr;
+	int ret = 0, i;
+
+	for (i = 0; i < page_count; i++) {
+		ret = vm_insert_page(vma, uaddr, pages[i]);
+		if (ret < 0)
+			return ret;
+		uaddr += PAGE_SIZE;
+	}
+
+	return ret;
+}
+
+/**
  * vm_insert_page - insert single page into user vma
  * @vma: user vma to map to
  * @addr: target user address of this page
diff --git a/mm/nommu.c b/mm/nommu.c
index 749276b..d6ef5c7 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
 }
 EXPORT_SYMBOL(vm_insert_page);
 
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count)
+{
+	return -EINVAL;
+}
+EXPORT_SYMBOL(vm_insert_range);
+
 /*
  *  sys_brk() for the most part doesn't need the global kernel
  *  lock, except when an application is doing something nasty
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-15 15:45 ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-15 15:45 UTC (permalink / raw)
  To: linux-arm-kernel

Previouly drivers have their own way of mapping range of
kernel pages/memory into user vma and this was done by
invoking vm_insert_page() within a loop.

As this pattern is common across different drivers, it can
be generalized by creating a new function and use it across
the drivers.

vm_insert_range is the new API which will be used to map a
range of kernel memory/pages to user vma.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Reviewed-by: Matthew Wilcox <willy@infradead.org>
---
 include/linux/mm_types.h |  3 +++
 mm/memory.c              | 28 ++++++++++++++++++++++++++++
 mm/nommu.c               |  7 +++++++
 3 files changed, 38 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5ed8f62..15ae24f 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
 extern void tlb_finish_mmu(struct mmu_gather *tlb,
 				unsigned long start, unsigned long end);
 
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count);
+
 static inline void init_tlb_flush_pending(struct mm_struct *mm)
 {
 	atomic_set(&mm->tlb_flush_pending, 0);
diff --git a/mm/memory.c b/mm/memory.c
index 15c417e..da904ed 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
 }
 
 /**
+ * vm_insert_range - insert range of kernel pages into user vma
+ * @vma: user vma to map to
+ * @addr: target user address of this page
+ * @pages: pointer to array of source kernel pages
+ * @page_count: no. of pages need to insert into user vma
+ *
+ * This allows drivers to insert range of kernel pages they've allocated
+ * into a user vma. This is a generic function which drivers can use
+ * rather than using their own way of mapping range of kernel pages into
+ * user vma.
+ */
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count)
+{
+	unsigned long uaddr = addr;
+	int ret = 0, i;
+
+	for (i = 0; i < page_count; i++) {
+		ret = vm_insert_page(vma, uaddr, pages[i]);
+		if (ret < 0)
+			return ret;
+		uaddr += PAGE_SIZE;
+	}
+
+	return ret;
+}
+
+/**
  * vm_insert_page - insert single page into user vma
  * @vma: user vma to map to
  * @addr: target user address of this page
diff --git a/mm/nommu.c b/mm/nommu.c
index 749276b..d6ef5c7 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
 }
 EXPORT_SYMBOL(vm_insert_page);
 
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count)
+{
+	return -EINVAL;
+}
+EXPORT_SYMBOL(vm_insert_range);
+
 /*
  *  sys_brk() for the most part doesn't need the global kernel
  *  lock, except when an application is doing something nasty
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-15 15:45 ` Souptick Joarder
  (?)
@ 2018-11-15 18:13   ` Randy Dunlap
  -1 siblings, 0 replies; 73+ messages in thread
From: Randy Dunlap @ 2018-11-15 18:13 UTC (permalink / raw)
  To: Souptick Joarder, akpm, willy, mhocko, kirill.shutemov, vbabka,
	riel, sfr, rppt, peterz, linux, robin.murphy, iamjoonsoo.kim,
	treding, keescook, m.szyprowski, stefanr, hjc, heiko, airlied,
	oleksandr_andrushchenko, joro, pawel, kyungmin.park, mchehab,
	boris.ostrovsky, jgross
  Cc: linux-kernel, linux-mm, linux-arm-kernel, linux1394-devel,
	dri-devel, linux-rockchip, xen-devel, iommu, linux-media

On 11/15/18 7:45 AM, Souptick Joarder wrote:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)

Hi,

What is the opposite of vm_insert_range() or even of vm_insert_page()?
or is there no need for that?


> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma

s/no./number/

> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.
> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	unsigned long uaddr = addr;
> +	int ret = 0, i;
> +
> +	for (i = 0; i < page_count; i++) {
> +		ret = vm_insert_page(vma, uaddr, pages[i]);
> +		if (ret < 0)
> +			return ret;

For a non-trivial value of page_count:
Is it a problem if vm_insert_page() succeeds for several pages
and then fails?

> +		uaddr += PAGE_SIZE;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page


thanks.
-- 
~Randy

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-15 18:13   ` Randy Dunlap
  0 siblings, 0 replies; 73+ messages in thread
From: Randy Dunlap @ 2018-11-15 18:13 UTC (permalink / raw)
  To: Souptick Joarder, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	willy-wEGCiKHe2LqWVfeAwA7xHQ, mhocko-IBi9RG/b67k,
	kirill.shutemov-VuQAYsv1563Yd54FQh9/CA, vbabka-AlSwsSmVLrQ,
	riel-ebMLmSuQjDVBDgjK7y7TUQ, sfr-3FnU+UHB4dNDw9hX6IcOSA,
	rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	peterz-wEGCiKHe2LqWVfeAwA7xHQ, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	robin.murphy-5wv7dgnIgG8, iamjoonsoo.kim-Hm3cg6mZ9cc,
	treding-DDmLM1+adcrQT0dZR+AlfA, keescook-F7+t8E8rja9g9hUCZPvPmw,
	m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
	stefanr-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB,
	hjc-TNX95d0MmH7DzftRWevZcw, heiko-4mtYJXux2i+zQB+pC5nmwQ,
	airlied-cv59FeDIM0c, oleksandr_andrushchenko-uRwfk40T5oI,
	joro-zLv9SwRftAIdnm+yROfE0A, pawel-FA/gS7QP4orQT0dZR+AlfA,
	kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	boris.ostrovsky-QHcLZuEGTsvQT0dZR+AlfA, jgross-IBi9RG/b67k
  Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	xen-devel-GuqFBffKawuEi8DpZVb4nw,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-media-u79uwXL29TY76Z2rM5mHXA

On 11/15/18 7:45 AM, Souptick Joarder wrote:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)

Hi,

What is the opposite of vm_insert_range() or even of vm_insert_page()?
or is there no need for that?


> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma

s/no./number/

> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.
> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	unsigned long uaddr = addr;
> +	int ret = 0, i;
> +
> +	for (i = 0; i < page_count; i++) {
> +		ret = vm_insert_page(vma, uaddr, pages[i]);
> +		if (ret < 0)
> +			return ret;

For a non-trivial value of page_count:
Is it a problem if vm_insert_page() succeeds for several pages
and then fails?

> +		uaddr += PAGE_SIZE;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page


thanks.
-- 
~Randy

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-15 18:13   ` Randy Dunlap
  0 siblings, 0 replies; 73+ messages in thread
From: Randy Dunlap @ 2018-11-15 18:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/15/18 7:45 AM, Souptick Joarder wrote:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)

Hi,

What is the opposite of vm_insert_range() or even of vm_insert_page()?
or is there no need for that?


> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma

s/no./number/

> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.
> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	unsigned long uaddr = addr;
> +	int ret = 0, i;
> +
> +	for (i = 0; i < page_count; i++) {
> +		ret = vm_insert_page(vma, uaddr, pages[i]);
> +		if (ret < 0)
> +			return ret;

For a non-trivial value of page_count:
Is it a problem if vm_insert_page() succeeds for several pages
and then fails?

> +		uaddr += PAGE_SIZE;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page


thanks.
-- 
~Randy

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-15 15:45 ` Souptick Joarder
                   ` (2 preceding siblings ...)
  (?)
@ 2018-11-15 18:13 ` Randy Dunlap
  -1 siblings, 0 replies; 73+ messages in thread
From: Randy Dunlap @ 2018-11-15 18:13 UTC (permalink / raw)
  To: Souptick Joarder, akpm, willy, mhocko, kirill.shutemov, vbabka,
	riel, sfr, rppt, peterz, linux, robin.murphy, iamjoonsoo.kim,
	treding, keescook, m.szyprowski, stefanr, hjc, heiko, airlied,
	oleksandr_andrushchenko, joro, pawel, kyungmin.park, mchehab,
	boris.ostrovsky, jgross
  Cc: linux-rockchip, linux-kernel, dri-devel, xen-devel, linux-mm,
	iommu, linux1394-devel, linux-arm-kernel, linux-media

On 11/15/18 7:45 AM, Souptick Joarder wrote:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)

Hi,

What is the opposite of vm_insert_range() or even of vm_insert_page()?
or is there no need for that?


> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma

s/no./number/

> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.
> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	unsigned long uaddr = addr;
> +	int ret = 0, i;
> +
> +	for (i = 0; i < page_count; i++) {
> +		ret = vm_insert_page(vma, uaddr, pages[i]);
> +		if (ret < 0)
> +			return ret;

For a non-trivial value of page_count:
Is it a problem if vm_insert_page() succeeds for several pages
and then fails?

> +		uaddr += PAGE_SIZE;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page


thanks.
-- 
~Randy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16  5:30     ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-16  5:30 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Andrew Morton, Matthew Wilcox, Michal Hocko, Kirill A. Shutemov,
	vbabka, Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > Previouly drivers have their own way of mapping range of
> > kernel pages/memory into user vma and this was done by
> > invoking vm_insert_page() within a loop.
> >
> > As this pattern is common across different drivers, it can
> > be generalized by creating a new function and use it across
> > the drivers.
> >
> > vm_insert_range is the new API which will be used to map a
> > range of kernel memory/pages to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> > Reviewed-by: Matthew Wilcox <willy@infradead.org>
> > ---
> >  include/linux/mm_types.h |  3 +++
> >  mm/memory.c              | 28 ++++++++++++++++++++++++++++
> >  mm/nommu.c               |  7 +++++++
> >  3 files changed, 38 insertions(+)
>
> Hi,
>
> What is the opposite of vm_insert_range() or even of vm_insert_page()?
> or is there no need for that?

There is no opposite function of vm_insert_range() / vm_insert_page().
My understanding is, in case of any error, mmap handlers will return the
err to user process and user space will decide the next action. So next
time when mmap handler is getting invoked it will map from the beginning.
Correct me if I am wrong.
>
>
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 15c417e..da904ed 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >
> >  /**
> > + * vm_insert_range - insert range of kernel pages into user vma
> > + * @vma: user vma to map to
> > + * @addr: target user address of this page
> > + * @pages: pointer to array of source kernel pages
> > + * @page_count: no. of pages need to insert into user vma
>
> s/no./number/

I didn't get it ??
>
> > + *
> > + * This allows drivers to insert range of kernel pages they've allocated
> > + * into a user vma. This is a generic function which drivers can use
> > + * rather than using their own way of mapping range of kernel pages into
> > + * user vma.
> > + */
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     unsigned long uaddr = addr;
> > +     int ret = 0, i;
> > +
> > +     for (i = 0; i < page_count; i++) {
> > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > +             if (ret < 0)
> > +                     return ret;
>
> For a non-trivial value of page_count:
> Is it a problem if vm_insert_page() succeeds for several pages
> and then fails?

No, it will be considered as total failure and mmap handler will return
the err to user space.
>
> > +             uaddr += PAGE_SIZE;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> >   * vm_insert_page - insert single page into user vma
> >   * @vma: user vma to map to
> >   * @addr: target user address of this page
>
>
> thanks.
> --
> ~Randy

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16  5:30     ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-16  5:30 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux-MM,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Stephen Rothwell, oleksandr_andrushchenko-uRwfk40T5oI,
	Russell King - ARM Linux, Matthew Wilcox, airlied-cv59FeDIM0c,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	treding-DDmLM1+adcrQT0dZR+AlfA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Kees Cook,
	pawel-FA/gS7QP4orQT0dZR+AlfA, Rik van Riel,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Boris Ostrovsky,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, iamjoonsoo.kim-Hm3cg6mZ9cc,
	vbabka-AlSwsSmVLrQ, Juergen Gross, hjc-TNX95d0MmH7DzftRWevZcw,
	xen-devel-GuqFBffKawuEi8DpZVb4nw, Kyungmin

On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>
> On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > Previouly drivers have their own way of mapping range of
> > kernel pages/memory into user vma and this was done by
> > invoking vm_insert_page() within a loop.
> >
> > As this pattern is common across different drivers, it can
> > be generalized by creating a new function and use it across
> > the drivers.
> >
> > vm_insert_range is the new API which will be used to map a
> > range of kernel memory/pages to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Reviewed-by: Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> > ---
> >  include/linux/mm_types.h |  3 +++
> >  mm/memory.c              | 28 ++++++++++++++++++++++++++++
> >  mm/nommu.c               |  7 +++++++
> >  3 files changed, 38 insertions(+)
>
> Hi,
>
> What is the opposite of vm_insert_range() or even of vm_insert_page()?
> or is there no need for that?

There is no opposite function of vm_insert_range() / vm_insert_page().
My understanding is, in case of any error, mmap handlers will return the
err to user process and user space will decide the next action. So next
time when mmap handler is getting invoked it will map from the beginning.
Correct me if I am wrong.
>
>
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 15c417e..da904ed 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >
> >  /**
> > + * vm_insert_range - insert range of kernel pages into user vma
> > + * @vma: user vma to map to
> > + * @addr: target user address of this page
> > + * @pages: pointer to array of source kernel pages
> > + * @page_count: no. of pages need to insert into user vma
>
> s/no./number/

I didn't get it ??
>
> > + *
> > + * This allows drivers to insert range of kernel pages they've allocated
> > + * into a user vma. This is a generic function which drivers can use
> > + * rather than using their own way of mapping range of kernel pages into
> > + * user vma.
> > + */
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     unsigned long uaddr = addr;
> > +     int ret = 0, i;
> > +
> > +     for (i = 0; i < page_count; i++) {
> > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > +             if (ret < 0)
> > +                     return ret;
>
> For a non-trivial value of page_count:
> Is it a problem if vm_insert_page() succeeds for several pages
> and then fails?

No, it will be considered as total failure and mmap handler will return
the err to user space.
>
> > +             uaddr += PAGE_SIZE;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> >   * vm_insert_page - insert single page into user vma
> >   * @vma: user vma to map to
> >   * @addr: target user address of this page
>
>
> thanks.
> --
> ~Randy

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16  5:30     ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-16  5:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > Previouly drivers have their own way of mapping range of
> > kernel pages/memory into user vma and this was done by
> > invoking vm_insert_page() within a loop.
> >
> > As this pattern is common across different drivers, it can
> > be generalized by creating a new function and use it across
> > the drivers.
> >
> > vm_insert_range is the new API which will be used to map a
> > range of kernel memory/pages to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> > Reviewed-by: Matthew Wilcox <willy@infradead.org>
> > ---
> >  include/linux/mm_types.h |  3 +++
> >  mm/memory.c              | 28 ++++++++++++++++++++++++++++
> >  mm/nommu.c               |  7 +++++++
> >  3 files changed, 38 insertions(+)
>
> Hi,
>
> What is the opposite of vm_insert_range() or even of vm_insert_page()?
> or is there no need for that?

There is no opposite function of vm_insert_range() / vm_insert_page().
My understanding is, in case of any error, mmap handlers will return the
err to user process and user space will decide the next action. So next
time when mmap handler is getting invoked it will map from the beginning.
Correct me if I am wrong.
>
>
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 15c417e..da904ed 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >
> >  /**
> > + * vm_insert_range - insert range of kernel pages into user vma
> > + * @vma: user vma to map to
> > + * @addr: target user address of this page
> > + * @pages: pointer to array of source kernel pages
> > + * @page_count: no. of pages need to insert into user vma
>
> s/no./number/

I didn't get it ??
>
> > + *
> > + * This allows drivers to insert range of kernel pages they've allocated
> > + * into a user vma. This is a generic function which drivers can use
> > + * rather than using their own way of mapping range of kernel pages into
> > + * user vma.
> > + */
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     unsigned long uaddr = addr;
> > +     int ret = 0, i;
> > +
> > +     for (i = 0; i < page_count; i++) {
> > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > +             if (ret < 0)
> > +                     return ret;
>
> For a non-trivial value of page_count:
> Is it a problem if vm_insert_page() succeeds for several pages
> and then fails?

No, it will be considered as total failure and mmap handler will return
the err to user space.
>
> > +             uaddr += PAGE_SIZE;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> >   * vm_insert_page - insert single page into user vma
> >   * @vma: user vma to map to
> >   * @addr: target user address of this page
>
>
> thanks.
> --
> ~Randy

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-15 18:13   ` Randy Dunlap
  (?)
  (?)
@ 2018-11-16  5:30   ` Souptick Joarder
  -1 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-16  5:30 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, Matthew Wilcox, airlied,
	linux-arm-kernel, linux-rockchip, treding, linux-media,
	Kees Cook, pawel, Rik van Riel, iommu, rppt, Boris Ostrovsky,
	mchehab, iamjoonsoo.kim, vbabka, Juergen Gross

On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > Previouly drivers have their own way of mapping range of
> > kernel pages/memory into user vma and this was done by
> > invoking vm_insert_page() within a loop.
> >
> > As this pattern is common across different drivers, it can
> > be generalized by creating a new function and use it across
> > the drivers.
> >
> > vm_insert_range is the new API which will be used to map a
> > range of kernel memory/pages to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> > Reviewed-by: Matthew Wilcox <willy@infradead.org>
> > ---
> >  include/linux/mm_types.h |  3 +++
> >  mm/memory.c              | 28 ++++++++++++++++++++++++++++
> >  mm/nommu.c               |  7 +++++++
> >  3 files changed, 38 insertions(+)
>
> Hi,
>
> What is the opposite of vm_insert_range() or even of vm_insert_page()?
> or is there no need for that?

There is no opposite function of vm_insert_range() / vm_insert_page().
My understanding is, in case of any error, mmap handlers will return the
err to user process and user space will decide the next action. So next
time when mmap handler is getting invoked it will map from the beginning.
Correct me if I am wrong.
>
>
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 15c417e..da904ed 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >
> >  /**
> > + * vm_insert_range - insert range of kernel pages into user vma
> > + * @vma: user vma to map to
> > + * @addr: target user address of this page
> > + * @pages: pointer to array of source kernel pages
> > + * @page_count: no. of pages need to insert into user vma
>
> s/no./number/

I didn't get it ??
>
> > + *
> > + * This allows drivers to insert range of kernel pages they've allocated
> > + * into a user vma. This is a generic function which drivers can use
> > + * rather than using their own way of mapping range of kernel pages into
> > + * user vma.
> > + */
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     unsigned long uaddr = addr;
> > +     int ret = 0, i;
> > +
> > +     for (i = 0; i < page_count; i++) {
> > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > +             if (ret < 0)
> > +                     return ret;
>
> For a non-trivial value of page_count:
> Is it a problem if vm_insert_page() succeeds for several pages
> and then fails?

No, it will be considered as total failure and mmap handler will return
the err to user space.
>
> > +             uaddr += PAGE_SIZE;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> >   * vm_insert_page - insert single page into user vma
> >   * @vma: user vma to map to
> >   * @addr: target user address of this page
>
>
> thanks.
> --
> ~Randy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-16  5:30     ` Souptick Joarder
  (?)
@ 2018-11-16  6:40       ` Matthew Wilcox
  -1 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-16  6:40 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Randy Dunlap, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
	vbabka, Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
> On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > What is the opposite of vm_insert_range() or even of vm_insert_page()?
> > or is there no need for that?
> 
> There is no opposite function of vm_insert_range() / vm_insert_page().
> My understanding is, in case of any error, mmap handlers will return the
> err to user process and user space will decide the next action. So next
> time when mmap handler is getting invoked it will map from the beginning.
> Correct me if I am wrong.

The opposite function, I suppose, is unmap_region().

> > s/no./number/
> 
> I didn't get it ??

This is a 'sed' expression.  's' is the 'substitute' command; the /
is a separator, 'no.' is what you wrote, and 'number' is what Randy
is recommending instead.

> > > +     for (i = 0; i < page_count; i++) {
> > > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > > +             if (ret < 0)
> > > +                     return ret;
> >
> > For a non-trivial value of page_count:
> > Is it a problem if vm_insert_page() succeeds for several pages
> > and then fails?
> 
> No, it will be considered as total failure and mmap handler will return
> the err to user space.

I think what Randy means is "What happens to the inserted pages?" and
the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
label, which is an accurate name.

Thanks for looking, Randy.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16  6:40       ` Matthew Wilcox
  0 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-16  6:40 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, Randy Dunlap

On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
> On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > What is the opposite of vm_insert_range() or even of vm_insert_page()?
> > or is there no need for that?
> 
> There is no opposite function of vm_insert_range() / vm_insert_page().
> My understanding is, in case of any error, mmap handlers will return the
> err to user process and user space will decide the next action. So next
> time when mmap handler is getting invoked it will map from the beginning.
> Correct me if I am wrong.

The opposite function, I suppose, is unmap_region().

> > s/no./number/
> 
> I didn't get it ??

This is a 'sed' expression.  's' is the 'substitute' command; the /
is a separator, 'no.' is what you wrote, and 'number' is what Randy
is recommending instead.

> > > +     for (i = 0; i < page_count; i++) {
> > > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > > +             if (ret < 0)
> > > +                     return ret;
> >
> > For a non-trivial value of page_count:
> > Is it a problem if vm_insert_page() succeeds for several pages
> > and then fails?
> 
> No, it will be considered as total failure and mmap handler will return
> the err to user space.

I think what Randy means is "What happens to the inserted pages?" and
the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
label, which is an accurate name.

Thanks for looking, Randy.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16  6:40       ` Matthew Wilcox
  0 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-16  6:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
> On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > What is the opposite of vm_insert_range() or even of vm_insert_page()?
> > or is there no need for that?
> 
> There is no opposite function of vm_insert_range() / vm_insert_page().
> My understanding is, in case of any error, mmap handlers will return the
> err to user process and user space will decide the next action. So next
> time when mmap handler is getting invoked it will map from the beginning.
> Correct me if I am wrong.

The opposite function, I suppose, is unmap_region().

> > s/no./number/
> 
> I didn't get it ??

This is a 'sed' expression.  's' is the 'substitute' command; the /
is a separator, 'no.' is what you wrote, and 'number' is what Randy
is recommending instead.

> > > +     for (i = 0; i < page_count; i++) {
> > > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > > +             if (ret < 0)
> > > +                     return ret;
> >
> > For a non-trivial value of page_count:
> > Is it a problem if vm_insert_page() succeeds for several pages
> > and then fails?
> 
> No, it will be considered as total failure and mmap handler will return
> the err to user space.

I think what Randy means is "What happens to the inserted pages?" and
the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
label, which is an accurate name.

Thanks for looking, Randy.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-16  5:30     ` Souptick Joarder
                       ` (2 preceding siblings ...)
  (?)
@ 2018-11-16  6:40     ` Matthew Wilcox
  -1 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-16  6:40 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, Randy Dunlap

On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
> On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > What is the opposite of vm_insert_range() or even of vm_insert_page()?
> > or is there no need for that?
> 
> There is no opposite function of vm_insert_range() / vm_insert_page().
> My understanding is, in case of any error, mmap handlers will return the
> err to user process and user space will decide the next action. So next
> time when mmap handler is getting invoked it will map from the beginning.
> Correct me if I am wrong.

The opposite function, I suppose, is unmap_region().

> > s/no./number/
> 
> I didn't get it ??

This is a 'sed' expression.  's' is the 'substitute' command; the /
is a separator, 'no.' is what you wrote, and 'number' is what Randy
is recommending instead.

> > > +     for (i = 0; i < page_count; i++) {
> > > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > > +             if (ret < 0)
> > > +                     return ret;
> >
> > For a non-trivial value of page_count:
> > Is it a problem if vm_insert_page() succeeds for several pages
> > and then fails?
> 
> No, it will be considered as total failure and mmap handler will return
> the err to user space.

I think what Randy means is "What happens to the inserted pages?" and
the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
label, which is an accurate name.

Thanks for looking, Randy.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16  8:15         ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-16  8:15 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Randy Dunlap, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
	vbabka, Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Fri, Nov 16, 2018 at 12:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
> > On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > > On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > > What is the opposite of vm_insert_range() or even of vm_insert_page()?
> > > or is there no need for that?
> >
> > There is no opposite function of vm_insert_range() / vm_insert_page().
> > My understanding is, in case of any error, mmap handlers will return the
> > err to user process and user space will decide the next action. So next
> > time when mmap handler is getting invoked it will map from the beginning.
> > Correct me if I am wrong.
>
> The opposite function, I suppose, is unmap_region().
>
> > > s/no./number/
> >
> > I didn't get it ??
>
> This is a 'sed' expression.  's' is the 'substitute' command; the /
> is a separator, 'no.' is what you wrote, and 'number' is what Randy
> is recommending instead.

Ok. Will change it in v2.
>
> > > > +     for (i = 0; i < page_count; i++) {
> > > > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > > > +             if (ret < 0)
> > > > +                     return ret;
> > >
> > > For a non-trivial value of page_count:
> > > Is it a problem if vm_insert_page() succeeds for several pages
> > > and then fails?
> >
> > No, it will be considered as total failure and mmap handler will return
> > the err to user space.
>
> I think what Randy means is "What happens to the inserted pages?" and
> the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
> label, which is an accurate name.

Sorry for incorrect understanding of the question.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16  8:15         ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-16  8:15 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux-MM,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Stephen Rothwell, oleksandr_andrushchenko-uRwfk40T5oI,
	Russell King - ARM Linux,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	airlied-cv59FeDIM0c,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	treding-DDmLM1+adcrQT0dZR+AlfA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Kees Cook,
	pawel-FA/gS7QP4orQT0dZR+AlfA, Rik van Riel,
	rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Boris Ostrovsky,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, iamjoonsoo.kim-Hm3cg6mZ9cc,
	vbabka-AlSwsSmVLrQ, Juergen Gross, Randy Dunlap,
	hjc-TNX95d0MmH7DzftRWevZcw, xen-devel-GuqFBffKawuEi8DpZVb4nw,
	Kyungmin

On Fri, Nov 16, 2018 at 12:11 PM Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>
> On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
> > On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> > > On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > > What is the opposite of vm_insert_range() or even of vm_insert_page()?
> > > or is there no need for that?
> >
> > There is no opposite function of vm_insert_range() / vm_insert_page().
> > My understanding is, in case of any error, mmap handlers will return the
> > err to user process and user space will decide the next action. So next
> > time when mmap handler is getting invoked it will map from the beginning.
> > Correct me if I am wrong.
>
> The opposite function, I suppose, is unmap_region().
>
> > > s/no./number/
> >
> > I didn't get it ??
>
> This is a 'sed' expression.  's' is the 'substitute' command; the /
> is a separator, 'no.' is what you wrote, and 'number' is what Randy
> is recommending instead.

Ok. Will change it in v2.
>
> > > > +     for (i = 0; i < page_count; i++) {
> > > > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > > > +             if (ret < 0)
> > > > +                     return ret;
> > >
> > > For a non-trivial value of page_count:
> > > Is it a problem if vm_insert_page() succeeds for several pages
> > > and then fails?
> >
> > No, it will be considered as total failure and mmap handler will return
> > the err to user space.
>
> I think what Randy means is "What happens to the inserted pages?" and
> the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
> label, which is an accurate name.

Sorry for incorrect understanding of the question.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16  8:15         ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-16  8:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Nov 16, 2018 at 12:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
> > On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > > On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > > What is the opposite of vm_insert_range() or even of vm_insert_page()?
> > > or is there no need for that?
> >
> > There is no opposite function of vm_insert_range() / vm_insert_page().
> > My understanding is, in case of any error, mmap handlers will return the
> > err to user process and user space will decide the next action. So next
> > time when mmap handler is getting invoked it will map from the beginning.
> > Correct me if I am wrong.
>
> The opposite function, I suppose, is unmap_region().
>
> > > s/no./number/
> >
> > I didn't get it ??
>
> This is a 'sed' expression.  's' is the 'substitute' command; the /
> is a separator, 'no.' is what you wrote, and 'number' is what Randy
> is recommending instead.

Ok. Will change it in v2.
>
> > > > +     for (i = 0; i < page_count; i++) {
> > > > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > > > +             if (ret < 0)
> > > > +                     return ret;
> > >
> > > For a non-trivial value of page_count:
> > > Is it a problem if vm_insert_page() succeeds for several pages
> > > and then fails?
> >
> > No, it will be considered as total failure and mmap handler will return
> > the err to user space.
>
> I think what Randy means is "What happens to the inserted pages?" and
> the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
> label, which is an accurate name.

Sorry for incorrect understanding of the question.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-16  6:40       ` Matthew Wilcox
  (?)
  (?)
@ 2018-11-16  8:15       ` Souptick Joarder
  -1 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-16  8:15 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, Randy Dunlap

On Fri, Nov 16, 2018 at 12:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
> > On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > > On 11/15/18 7:45 AM, Souptick Joarder wrote:
> > > What is the opposite of vm_insert_range() or even of vm_insert_page()?
> > > or is there no need for that?
> >
> > There is no opposite function of vm_insert_range() / vm_insert_page().
> > My understanding is, in case of any error, mmap handlers will return the
> > err to user process and user space will decide the next action. So next
> > time when mmap handler is getting invoked it will map from the beginning.
> > Correct me if I am wrong.
>
> The opposite function, I suppose, is unmap_region().
>
> > > s/no./number/
> >
> > I didn't get it ??
>
> This is a 'sed' expression.  's' is the 'substitute' command; the /
> is a separator, 'no.' is what you wrote, and 'number' is what Randy
> is recommending instead.

Ok. Will change it in v2.
>
> > > > +     for (i = 0; i < page_count; i++) {
> > > > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > > > +             if (ret < 0)
> > > > +                     return ret;
> > >
> > > For a non-trivial value of page_count:
> > > Is it a problem if vm_insert_page() succeeds for several pages
> > > and then fails?
> >
> > No, it will be considered as total failure and mmap handler will return
> > the err to user space.
>
> I think what Randy means is "What happens to the inserted pages?" and
> the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
> label, which is an accurate name.

Sorry for incorrect understanding of the question.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-16  8:15         ` Souptick Joarder
  (?)
@ 2018-11-16 16:59           ` Randy Dunlap
  -1 siblings, 0 replies; 73+ messages in thread
From: Randy Dunlap @ 2018-11-16 16:59 UTC (permalink / raw)
  To: Souptick Joarder, Matthew Wilcox
  Cc: Andrew Morton, Michal Hocko, Kirill A. Shutemov, vbabka,
	Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On 11/16/18 12:15 AM, Souptick Joarder wrote:
> On Fri, Nov 16, 2018 at 12:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>>
>> On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
>>> On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>>>> On 11/15/18 7:45 AM, Souptick Joarder wrote:
>>>> What is the opposite of vm_insert_range() or even of vm_insert_page()?
>>>> or is there no need for that?
>>>
>>> There is no opposite function of vm_insert_range() / vm_insert_page().
>>> My understanding is, in case of any error, mmap handlers will return the
>>> err to user process and user space will decide the next action. So next
>>> time when mmap handler is getting invoked it will map from the beginning.
>>> Correct me if I am wrong.
>>
>> The opposite function, I suppose, is unmap_region().
>>
>>>> s/no./number/
>>>
>>> I didn't get it ??
>>
>> This is a 'sed' expression.  's' is the 'substitute' command; the /
>> is a separator, 'no.' is what you wrote, and 'number' is what Randy
>> is recommending instead.
> 
> Ok. Will change it in v2.

Thanks.

>>>>> +     for (i = 0; i < page_count; i++) {
>>>>> +             ret = vm_insert_page(vma, uaddr, pages[i]);
>>>>> +             if (ret < 0)
>>>>> +                     return ret;
>>>>
>>>> For a non-trivial value of page_count:
>>>> Is it a problem if vm_insert_page() succeeds for several pages
>>>> and then fails?
>>>
>>> No, it will be considered as total failure and mmap handler will return
>>> the err to user space.
>>
>> I think what Randy means is "What happens to the inserted pages?" and
>> the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
>> label, which is an accurate name.

which says:

	/* Undo any partial mapping done by a device driver. */
	unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);

and that is what I was missing.  Thanks.

> Sorry for incorrect understanding of the question.

No problem.


-- 
~Randy

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16 16:59           ` Randy Dunlap
  0 siblings, 0 replies; 73+ messages in thread
From: Randy Dunlap @ 2018-11-16 16:59 UTC (permalink / raw)
  To: Souptick Joarder, Matthew Wilcox
  Cc: Andrew Morton, Michal Hocko, Kirill A. Shutemov, vbabka,
	Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross

On 11/16/18 12:15 AM, Souptick Joarder wrote:
> On Fri, Nov 16, 2018 at 12:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>>
>> On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
>>> On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>>>> On 11/15/18 7:45 AM, Souptick Joarder wrote:
>>>> What is the opposite of vm_insert_range() or even of vm_insert_page()?
>>>> or is there no need for that?
>>>
>>> There is no opposite function of vm_insert_range() / vm_insert_page().
>>> My understanding is, in case of any error, mmap handlers will return the
>>> err to user process and user space will decide the next action. So next
>>> time when mmap handler is getting invoked it will map from the beginning.
>>> Correct me if I am wrong.
>>
>> The opposite function, I suppose, is unmap_region().
>>
>>>> s/no./number/
>>>
>>> I didn't get it ??
>>
>> This is a 'sed' expression.  's' is the 'substitute' command; the /
>> is a separator, 'no.' is what you wrote, and 'number' is what Randy
>> is recommending instead.
> 
> Ok. Will change it in v2.

Thanks.

>>>>> +     for (i = 0; i < page_count; i++) {
>>>>> +             ret = vm_insert_page(vma, uaddr, pages[i]);
>>>>> +             if (ret < 0)
>>>>> +                     return ret;
>>>>
>>>> For a non-trivial value of page_count:
>>>> Is it a problem if vm_insert_page() succeeds for several pages
>>>> and then fails?
>>>
>>> No, it will be considered as total failure and mmap handler will return
>>> the err to user space.
>>
>> I think what Randy means is "What happens to the inserted pages?" and
>> the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
>> label, which is an accurate name.

which says:

	/* Undo any partial mapping done by a device driver. */
	unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);

and that is what I was missing.  Thanks.

> Sorry for incorrect understanding of the question.

No problem.


-- 
~Randy

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16 16:59           ` Randy Dunlap
  0 siblings, 0 replies; 73+ messages in thread
From: Randy Dunlap @ 2018-11-16 16:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/16/18 12:15 AM, Souptick Joarder wrote:
> On Fri, Nov 16, 2018 at 12:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>>
>> On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
>>> On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>>>> On 11/15/18 7:45 AM, Souptick Joarder wrote:
>>>> What is the opposite of vm_insert_range() or even of vm_insert_page()?
>>>> or is there no need for that?
>>>
>>> There is no opposite function of vm_insert_range() / vm_insert_page().
>>> My understanding is, in case of any error, mmap handlers will return the
>>> err to user process and user space will decide the next action. So next
>>> time when mmap handler is getting invoked it will map from the beginning.
>>> Correct me if I am wrong.
>>
>> The opposite function, I suppose, is unmap_region().
>>
>>>> s/no./number/
>>>
>>> I didn't get it ??
>>
>> This is a 'sed' expression.  's' is the 'substitute' command; the /
>> is a separator, 'no.' is what you wrote, and 'number' is what Randy
>> is recommending instead.
> 
> Ok. Will change it in v2.

Thanks.

>>>>> +     for (i = 0; i < page_count; i++) {
>>>>> +             ret = vm_insert_page(vma, uaddr, pages[i]);
>>>>> +             if (ret < 0)
>>>>> +                     return ret;
>>>>
>>>> For a non-trivial value of page_count:
>>>> Is it a problem if vm_insert_page() succeeds for several pages
>>>> and then fails?
>>>
>>> No, it will be considered as total failure and mmap handler will return
>>> the err to user space.
>>
>> I think what Randy means is "What happens to the inserted pages?" and
>> the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
>> label, which is an accurate name.

which says:

	/* Undo any partial mapping done by a device driver. */
	unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);

and that is what I was missing.  Thanks.

> Sorry for incorrect understanding of the question.

No problem.


-- 
~Randy

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-16  8:15         ` Souptick Joarder
  (?)
  (?)
@ 2018-11-16 16:59         ` Randy Dunlap
  -1 siblings, 0 replies; 73+ messages in thread
From: Randy Dunlap @ 2018-11-16 16:59 UTC (permalink / raw)
  To: Souptick Joarder, Matthew Wilcox
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, hjc, xen-devel

On 11/16/18 12:15 AM, Souptick Joarder wrote:
> On Fri, Nov 16, 2018 at 12:11 PM Matthew Wilcox <willy@infradead.org> wrote:
>>
>> On Fri, Nov 16, 2018 at 11:00:30AM +0530, Souptick Joarder wrote:
>>> On Thu, Nov 15, 2018 at 11:44 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>>>> On 11/15/18 7:45 AM, Souptick Joarder wrote:
>>>> What is the opposite of vm_insert_range() or even of vm_insert_page()?
>>>> or is there no need for that?
>>>
>>> There is no opposite function of vm_insert_range() / vm_insert_page().
>>> My understanding is, in case of any error, mmap handlers will return the
>>> err to user process and user space will decide the next action. So next
>>> time when mmap handler is getting invoked it will map from the beginning.
>>> Correct me if I am wrong.
>>
>> The opposite function, I suppose, is unmap_region().
>>
>>>> s/no./number/
>>>
>>> I didn't get it ??
>>
>> This is a 'sed' expression.  's' is the 'substitute' command; the /
>> is a separator, 'no.' is what you wrote, and 'number' is what Randy
>> is recommending instead.
> 
> Ok. Will change it in v2.

Thanks.

>>>>> +     for (i = 0; i < page_count; i++) {
>>>>> +             ret = vm_insert_page(vma, uaddr, pages[i]);
>>>>> +             if (ret < 0)
>>>>> +                     return ret;
>>>>
>>>> For a non-trivial value of page_count:
>>>> Is it a problem if vm_insert_page() succeeds for several pages
>>>> and then fails?
>>>
>>> No, it will be considered as total failure and mmap handler will return
>>> the err to user space.
>>
>> I think what Randy means is "What happens to the inserted pages?" and
>> the answer is that mmap_region() jumps to the 'unmap_and_free_vma'
>> label, which is an accurate name.

which says:

	/* Undo any partial mapping done by a device driver. */
	unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);

and that is what I was missing.  Thanks.

> Sorry for incorrect understanding of the question.

No problem.


-- 
~Randy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-15 15:45 ` Souptick Joarder
  (?)
@ 2018-11-16 17:48   ` Slavomir Kaslev
  -1 siblings, 0 replies; 73+ messages in thread
From: Slavomir Kaslev @ 2018-11-16 17:48 UTC (permalink / raw)
  To: jrdr.linux
  Cc: mhocko, heiko, peterz, dri-devel, linux-kernel, linux-mm,
	linux1394-devel, m.szyprowski, sfr, oleksandr_andrushchenko,
	joro, linux, willy, airlied, linux-arm-kernel, linux-rockchip,
	tredi

On Thu, Nov 15, 2018 at 5:42 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
>
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
>
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
>
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ed8f62..15ae24f 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
>  extern void tlb_finish_mmu(struct mmu_gather *tlb,
>                                 unsigned long start, unsigned long end);
>
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count);
> +
>  static inline void init_tlb_flush_pending(struct mm_struct *mm)
>  {
>         atomic_set(&mm->tlb_flush_pending, 0);
> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma
> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.
> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count)
> +{
> +       unsigned long uaddr = addr;
> +       int ret = 0, i;
> +
> +       for (i = 0; i < page_count; i++) {
> +               ret = vm_insert_page(vma, uaddr, pages[i]);
> +               if (ret < 0)
> +                       return ret;
> +               uaddr += PAGE_SIZE;
> +       }
> +
> +       return ret;
> +}

+ EXPORT_SYMBOL(vm_insert_range);

> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 749276b..d6ef5c7 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  EXPORT_SYMBOL(vm_insert_page);
>
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count)
> +{
> +       return -EINVAL;
> +}
> +EXPORT_SYMBOL(vm_insert_range);
> +
>  /*
>   *  sys_brk() for the most part doesn't need the global kernel
>   *  lock, except when an application is doing something nasty
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16 17:48   ` Slavomir Kaslev
  0 siblings, 0 replies; 73+ messages in thread
From: Slavomir Kaslev @ 2018-11-16 17:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Nov 15, 2018 at 5:42 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
>
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
>
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
>
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ed8f62..15ae24f 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
>  extern void tlb_finish_mmu(struct mmu_gather *tlb,
>                                 unsigned long start, unsigned long end);
>
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count);
> +
>  static inline void init_tlb_flush_pending(struct mm_struct *mm)
>  {
>         atomic_set(&mm->tlb_flush_pending, 0);
> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma
> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.
> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count)
> +{
> +       unsigned long uaddr = addr;
> +       int ret = 0, i;
> +
> +       for (i = 0; i < page_count; i++) {
> +               ret = vm_insert_page(vma, uaddr, pages[i]);
> +               if (ret < 0)
> +                       return ret;
> +               uaddr += PAGE_SIZE;
> +       }
> +
> +       return ret;
> +}

+ EXPORT_SYMBOL(vm_insert_range);

> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 749276b..d6ef5c7 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  EXPORT_SYMBOL(vm_insert_page);
>
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count)
> +{
> +       return -EINVAL;
> +}
> +EXPORT_SYMBOL(vm_insert_range);
> +
>  /*
>   *  sys_brk() for the most part doesn't need the global kernel
>   *  lock, except when an application is doing something nasty
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-15 15:45 ` Souptick Joarder
                   ` (4 preceding siblings ...)
  (?)
@ 2018-11-16 17:48 ` Slavomir Kaslev
  -1 siblings, 0 replies; 73+ messages in thread
From: Slavomir Kaslev @ 2018-11-16 17:48 UTC (permalink / raw)
  To: jrdr.linux
  Cc: mhocko, heiko, peterz, dri-devel, linux-kernel, linux-mm,
	linux1394-devel, m.szyprowski, sfr, oleksandr_andrushchenko,
	joro, linux, willy, airlied, linux-arm-kernel, linux-rockchip,
	tredi

On Thu, Nov 15, 2018 at 5:42 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
>
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
>
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
>
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ed8f62..15ae24f 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
>  extern void tlb_finish_mmu(struct mmu_gather *tlb,
>                                 unsigned long start, unsigned long end);
>
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count);
> +
>  static inline void init_tlb_flush_pending(struct mm_struct *mm)
>  {
>         atomic_set(&mm->tlb_flush_pending, 0);
> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma
> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.
> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count)
> +{
> +       unsigned long uaddr = addr;
> +       int ret = 0, i;
> +
> +       for (i = 0; i < page_count; i++) {
> +               ret = vm_insert_page(vma, uaddr, pages[i]);
> +               if (ret < 0)
> +                       return ret;
> +               uaddr += PAGE_SIZE;
> +       }
> +
> +       return ret;
> +}

+ EXPORT_SYMBOL(vm_insert_range);

> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 749276b..d6ef5c7 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  EXPORT_SYMBOL(vm_insert_page);
>
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count)
> +{
> +       return -EINVAL;
> +}
> +EXPORT_SYMBOL(vm_insert_range);
> +
>  /*
>   *  sys_brk() for the most part doesn't need the global kernel
>   *  lock, except when an application is doing something nasty
> --
> 1.9.1
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16 17:48   ` Slavomir Kaslev
  0 siblings, 0 replies; 73+ messages in thread
From: Slavomir Kaslev @ 2018-11-16 17:55 UTC (permalink / raw)
  To: jrdr.linux
  Cc: akpm, willy, mhocko, kirill.shutemov, vbabka, riel, sfr, rppt,
	peterz, linux, robin.murphy, iamjoonsoo.kim, treding, keescook,
	m.szyprowski, stefanr, hjc, heiko, airlied,
	oleksandr_andrushchenko, joro, pawel, kyungmin.park, mchehab,
	boris.ostrovsky, jgross, linux-kernel, linux-mm,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Thu, Nov 15, 2018 at 5:42 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
>
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
>
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
>
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ed8f62..15ae24f 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
>  extern void tlb_finish_mmu(struct mmu_gather *tlb,
>                                 unsigned long start, unsigned long end);
>
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count);
> +
>  static inline void init_tlb_flush_pending(struct mm_struct *mm)
>  {
>         atomic_set(&mm->tlb_flush_pending, 0);
> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma
> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.
> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count)
> +{
> +       unsigned long uaddr = addr;
> +       int ret = 0, i;
> +
> +       for (i = 0; i < page_count; i++) {
> +               ret = vm_insert_page(vma, uaddr, pages[i]);
> +               if (ret < 0)
> +                       return ret;
> +               uaddr += PAGE_SIZE;
> +       }
> +
> +       return ret;
> +}

+ EXPORT_SYMBOL(vm_insert_range);

> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 749276b..d6ef5c7 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  EXPORT_SYMBOL(vm_insert_page);
>
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +                       struct page **pages, unsigned long page_count)
> +{
> +       return -EINVAL;
> +}
> +EXPORT_SYMBOL(vm_insert_range);
> +
>  /*
>   *  sys_brk() for the most part doesn't need the global kernel
>   *  lock, except when an application is doing something nasty
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-15 15:45 ` Souptick Joarder
  (?)
@ 2018-11-16 18:28   ` Mike Rapoport
  -1 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-16 18:28 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: akpm, willy, mhocko, kirill.shutemov, vbabka, riel, sfr, rppt,
	peterz, linux, robin.murphy, iamjoonsoo.kim, treding, keescook,
	m.szyprowski, stefanr, hjc, heiko, airlied,
	oleksandr_andrushchenko, joro, pawel, kyungmin.park, mchehab,
	boris.ostrovsky, jgross, linux-kernel, linux-mm,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Thu, Nov 15, 2018 at 09:15:30PM +0530, Souptick Joarder wrote:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ed8f62..15ae24f 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
>  extern void tlb_finish_mmu(struct mmu_gather *tlb,
>  				unsigned long start, unsigned long end);
> 
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count);
> +
>  static inline void init_tlb_flush_pending(struct mm_struct *mm)
>  {
>  	atomic_set(&mm->tlb_flush_pending, 0);
> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
> 
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma
> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.

Please add the return value and context descriptions.

> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	unsigned long uaddr = addr;
> +	int ret = 0, i;
> +
> +	for (i = 0; i < page_count; i++) {
> +		ret = vm_insert_page(vma, uaddr, pages[i]);
> +		if (ret < 0)
> +			return ret;
> +		uaddr += PAGE_SIZE;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 749276b..d6ef5c7 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  EXPORT_SYMBOL(vm_insert_page);
> 
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(vm_insert_range);
> +
>  /*
>   *  sys_brk() for the most part doesn't need the global kernel
>   *  lock, except when an application is doing something nasty
> -- 
> 1.9.1
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16 18:28   ` Mike Rapoport
  0 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-16 18:28 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: akpm, willy, mhocko, kirill.shutemov, vbabka, riel, sfr, rppt,
	peterz, linux, robin.murphy, iamjoonsoo.kim, treding, keescook,
	m.szyprowski, stefanr, hjc, heiko, airlied,
	oleksandr_andrushchenko, joro, pawel, kyungmin.park, mchehab,
	boris.ostrovsky, jgross, linux-kernel, linux-mm,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-de

On Thu, Nov 15, 2018 at 09:15:30PM +0530, Souptick Joarder wrote:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ed8f62..15ae24f 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
>  extern void tlb_finish_mmu(struct mmu_gather *tlb,
>  				unsigned long start, unsigned long end);
> 
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count);
> +
>  static inline void init_tlb_flush_pending(struct mm_struct *mm)
>  {
>  	atomic_set(&mm->tlb_flush_pending, 0);
> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
> 
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma
> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.

Please add the return value and context descriptions.

> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	unsigned long uaddr = addr;
> +	int ret = 0, i;
> +
> +	for (i = 0; i < page_count; i++) {
> +		ret = vm_insert_page(vma, uaddr, pages[i]);
> +		if (ret < 0)
> +			return ret;
> +		uaddr += PAGE_SIZE;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 749276b..d6ef5c7 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  EXPORT_SYMBOL(vm_insert_page);
> 
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(vm_insert_range);
> +
>  /*
>   *  sys_brk() for the most part doesn't need the global kernel
>   *  lock, except when an application is doing something nasty
> -- 
> 1.9.1
> 

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-16 18:28   ` Mike Rapoport
  0 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-16 18:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Nov 15, 2018 at 09:15:30PM +0530, Souptick Joarder wrote:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ed8f62..15ae24f 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
>  extern void tlb_finish_mmu(struct mmu_gather *tlb,
>  				unsigned long start, unsigned long end);
> 
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count);
> +
>  static inline void init_tlb_flush_pending(struct mm_struct *mm)
>  {
>  	atomic_set(&mm->tlb_flush_pending, 0);
> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
> 
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma
> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.

Please add the return value and context descriptions.

> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	unsigned long uaddr = addr;
> +	int ret = 0, i;
> +
> +	for (i = 0; i < page_count; i++) {
> +		ret = vm_insert_page(vma, uaddr, pages[i]);
> +		if (ret < 0)
> +			return ret;
> +		uaddr += PAGE_SIZE;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 749276b..d6ef5c7 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  EXPORT_SYMBOL(vm_insert_page);
> 
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(vm_insert_range);
> +
>  /*
>   *  sys_brk() for the most part doesn't need the global kernel
>   *  lock, except when an application is doing something nasty
> -- 
> 1.9.1
> 

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-15 15:45 ` Souptick Joarder
                   ` (5 preceding siblings ...)
  (?)
@ 2018-11-16 18:28 ` Mike Rapoport
  -1 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-16 18:28 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: mhocko, heiko, peterz, dri-devel, linux-kernel, linux-mm,
	linux1394-devel, m.szyprowski, sfr, oleksandr_andrushchenko,
	joro, linux, willy, airlied, linux-arm-kernel, linux-rockchip,
	treding, linux-media, keescook, pawel, riel, iommu, rppt,
	boris.ostrovsky, mchehab, iamjoonsoo.kim, vbabka, jgross, hjc,
	xen-devel, kyungmin.park, stefanr, akpm, robin.murphy,
	kirill.shutemov

On Thu, Nov 15, 2018 at 09:15:30PM +0530, Souptick Joarder wrote:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
>  include/linux/mm_types.h |  3 +++
>  mm/memory.c              | 28 ++++++++++++++++++++++++++++
>  mm/nommu.c               |  7 +++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ed8f62..15ae24f 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
>  extern void tlb_finish_mmu(struct mmu_gather *tlb,
>  				unsigned long start, unsigned long end);
> 
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count);
> +
>  static inline void init_tlb_flush_pending(struct mm_struct *mm)
>  {
>  	atomic_set(&mm->tlb_flush_pending, 0);
> diff --git a/mm/memory.c b/mm/memory.c
> index 15c417e..da904ed 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
> 
>  /**
> + * vm_insert_range - insert range of kernel pages into user vma
> + * @vma: user vma to map to
> + * @addr: target user address of this page
> + * @pages: pointer to array of source kernel pages
> + * @page_count: no. of pages need to insert into user vma
> + *
> + * This allows drivers to insert range of kernel pages they've allocated
> + * into a user vma. This is a generic function which drivers can use
> + * rather than using their own way of mapping range of kernel pages into
> + * user vma.

Please add the return value and context descriptions.

> + */
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	unsigned long uaddr = addr;
> +	int ret = 0, i;
> +
> +	for (i = 0; i < page_count; i++) {
> +		ret = vm_insert_page(vma, uaddr, pages[i]);
> +		if (ret < 0)
> +			return ret;
> +		uaddr += PAGE_SIZE;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * vm_insert_page - insert single page into user vma
>   * @vma: user vma to map to
>   * @addr: target user address of this page
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 749276b..d6ef5c7 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
>  }
>  EXPORT_SYMBOL(vm_insert_page);
> 
> +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> +			struct page **pages, unsigned long page_count)
> +{
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(vm_insert_range);
> +
>  /*
>   *  sys_brk() for the most part doesn't need the global kernel
>   *  lock, except when an application is doing something nasty
> -- 
> 1.9.1
> 

-- 
Sincerely yours,
Mike.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-16 18:28   ` Mike Rapoport
  (?)
@ 2018-11-17  6:56     ` Souptick Joarder
  -1 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-17  6:56 UTC (permalink / raw)
  To: rppt
  Cc: Andrew Morton, Matthew Wilcox, Michal Hocko, Kirill A. Shutemov,
	vbabka, Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Thu, Nov 15, 2018 at 09:15:30PM +0530, Souptick Joarder wrote:
> > Previouly drivers have their own way of mapping range of
> > kernel pages/memory into user vma and this was done by
> > invoking vm_insert_page() within a loop.
> >
> > As this pattern is common across different drivers, it can
> > be generalized by creating a new function and use it across
> > the drivers.
> >
> > vm_insert_range is the new API which will be used to map a
> > range of kernel memory/pages to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> > Reviewed-by: Matthew Wilcox <willy@infradead.org>
> > ---
> >  include/linux/mm_types.h |  3 +++
> >  mm/memory.c              | 28 ++++++++++++++++++++++++++++
> >  mm/nommu.c               |  7 +++++++
> >  3 files changed, 38 insertions(+)
> >
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 5ed8f62..15ae24f 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
> >  extern void tlb_finish_mmu(struct mmu_gather *tlb,
> >                               unsigned long start, unsigned long end);
> >
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count);
> > +
> >  static inline void init_tlb_flush_pending(struct mm_struct *mm)
> >  {
> >       atomic_set(&mm->tlb_flush_pending, 0);
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 15c417e..da904ed 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >
> >  /**
> > + * vm_insert_range - insert range of kernel pages into user vma
> > + * @vma: user vma to map to
> > + * @addr: target user address of this page
> > + * @pages: pointer to array of source kernel pages
> > + * @page_count: no. of pages need to insert into user vma
> > + *
> > + * This allows drivers to insert range of kernel pages they've allocated
> > + * into a user vma. This is a generic function which drivers can use
> > + * rather than using their own way of mapping range of kernel pages into
> > + * user vma.
>
> Please add the return value and context descriptions.
>
> > + */
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     unsigned long uaddr = addr;
> > +     int ret = 0, i;
> > +
> > +     for (i = 0; i < page_count; i++) {
> > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > +             if (ret < 0)
> > +                     return ret;
> > +             uaddr += PAGE_SIZE;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> >   * vm_insert_page - insert single page into user vma
> >   * @vma: user vma to map to
> >   * @addr: target user address of this page
> > diff --git a/mm/nommu.c b/mm/nommu.c
> > index 749276b..d6ef5c7 100644
> > --- a/mm/nommu.c
> > +++ b/mm/nommu.c
> > @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >  EXPORT_SYMBOL(vm_insert_page);
> >
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     return -EINVAL;
> > +}
> > +EXPORT_SYMBOL(vm_insert_range);
> > +
> >  /*
> >   *  sys_brk() for the most part doesn't need the global kernel
> >   *  lock, except when an application is doing something nasty
> > --
> > 1.9.1
> >
>
> --
> Sincerely yours,
> Mike.
>

Sure I will wait for some time to get additional review comments and
add all of those requested changes in v2.

Any further feedback on driver changes as part of this patch series ?

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-17  6:56     ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-17  6:56 UTC (permalink / raw)
  To: rppt-tEXmvtCZX7AybS5Ee8rs3A
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux-MM,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Stephen Rothwell, oleksandr_andrushchenko-uRwfk40T5oI,
	Russell King - ARM Linux, Matthew Wilcox, airlied-cv59FeDIM0c,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	treding-DDmLM1+adcrQT0dZR+AlfA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Kees Cook,
	pawel-FA/gS7QP4orQT0dZR+AlfA, Rik van Riel,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Boris Ostrovsky,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, iamjoonsoo.kim-Hm3cg6mZ9cc,
	vbabka-AlSwsSmVLrQ, Juergen Gross, hjc-TNX95d0MmH7DzftRWevZcw,
	xen-devel-GuqFBffKawuEi8DpZVb4nw, Kyungmin

On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> wrote:
>
> On Thu, Nov 15, 2018 at 09:15:30PM +0530, Souptick Joarder wrote:
> > Previouly drivers have their own way of mapping range of
> > kernel pages/memory into user vma and this was done by
> > invoking vm_insert_page() within a loop.
> >
> > As this pattern is common across different drivers, it can
> > be generalized by creating a new function and use it across
> > the drivers.
> >
> > vm_insert_range is the new API which will be used to map a
> > range of kernel memory/pages to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Reviewed-by: Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> > ---
> >  include/linux/mm_types.h |  3 +++
> >  mm/memory.c              | 28 ++++++++++++++++++++++++++++
> >  mm/nommu.c               |  7 +++++++
> >  3 files changed, 38 insertions(+)
> >
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 5ed8f62..15ae24f 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
> >  extern void tlb_finish_mmu(struct mmu_gather *tlb,
> >                               unsigned long start, unsigned long end);
> >
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count);
> > +
> >  static inline void init_tlb_flush_pending(struct mm_struct *mm)
> >  {
> >       atomic_set(&mm->tlb_flush_pending, 0);
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 15c417e..da904ed 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >
> >  /**
> > + * vm_insert_range - insert range of kernel pages into user vma
> > + * @vma: user vma to map to
> > + * @addr: target user address of this page
> > + * @pages: pointer to array of source kernel pages
> > + * @page_count: no. of pages need to insert into user vma
> > + *
> > + * This allows drivers to insert range of kernel pages they've allocated
> > + * into a user vma. This is a generic function which drivers can use
> > + * rather than using their own way of mapping range of kernel pages into
> > + * user vma.
>
> Please add the return value and context descriptions.
>
> > + */
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     unsigned long uaddr = addr;
> > +     int ret = 0, i;
> > +
> > +     for (i = 0; i < page_count; i++) {
> > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > +             if (ret < 0)
> > +                     return ret;
> > +             uaddr += PAGE_SIZE;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> >   * vm_insert_page - insert single page into user vma
> >   * @vma: user vma to map to
> >   * @addr: target user address of this page
> > diff --git a/mm/nommu.c b/mm/nommu.c
> > index 749276b..d6ef5c7 100644
> > --- a/mm/nommu.c
> > +++ b/mm/nommu.c
> > @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >  EXPORT_SYMBOL(vm_insert_page);
> >
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     return -EINVAL;
> > +}
> > +EXPORT_SYMBOL(vm_insert_range);
> > +
> >  /*
> >   *  sys_brk() for the most part doesn't need the global kernel
> >   *  lock, except when an application is doing something nasty
> > --
> > 1.9.1
> >
>
> --
> Sincerely yours,
> Mike.
>

Sure I will wait for some time to get additional review comments and
add all of those requested changes in v2.

Any further feedback on driver changes as part of this patch series ?

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-17  6:56     ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-17  6:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Thu, Nov 15, 2018 at 09:15:30PM +0530, Souptick Joarder wrote:
> > Previouly drivers have their own way of mapping range of
> > kernel pages/memory into user vma and this was done by
> > invoking vm_insert_page() within a loop.
> >
> > As this pattern is common across different drivers, it can
> > be generalized by creating a new function and use it across
> > the drivers.
> >
> > vm_insert_range is the new API which will be used to map a
> > range of kernel memory/pages to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> > Reviewed-by: Matthew Wilcox <willy@infradead.org>
> > ---
> >  include/linux/mm_types.h |  3 +++
> >  mm/memory.c              | 28 ++++++++++++++++++++++++++++
> >  mm/nommu.c               |  7 +++++++
> >  3 files changed, 38 insertions(+)
> >
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 5ed8f62..15ae24f 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
> >  extern void tlb_finish_mmu(struct mmu_gather *tlb,
> >                               unsigned long start, unsigned long end);
> >
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count);
> > +
> >  static inline void init_tlb_flush_pending(struct mm_struct *mm)
> >  {
> >       atomic_set(&mm->tlb_flush_pending, 0);
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 15c417e..da904ed 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >
> >  /**
> > + * vm_insert_range - insert range of kernel pages into user vma
> > + * @vma: user vma to map to
> > + * @addr: target user address of this page
> > + * @pages: pointer to array of source kernel pages
> > + * @page_count: no. of pages need to insert into user vma
> > + *
> > + * This allows drivers to insert range of kernel pages they've allocated
> > + * into a user vma. This is a generic function which drivers can use
> > + * rather than using their own way of mapping range of kernel pages into
> > + * user vma.
>
> Please add the return value and context descriptions.
>
> > + */
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     unsigned long uaddr = addr;
> > +     int ret = 0, i;
> > +
> > +     for (i = 0; i < page_count; i++) {
> > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > +             if (ret < 0)
> > +                     return ret;
> > +             uaddr += PAGE_SIZE;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> >   * vm_insert_page - insert single page into user vma
> >   * @vma: user vma to map to
> >   * @addr: target user address of this page
> > diff --git a/mm/nommu.c b/mm/nommu.c
> > index 749276b..d6ef5c7 100644
> > --- a/mm/nommu.c
> > +++ b/mm/nommu.c
> > @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >  EXPORT_SYMBOL(vm_insert_page);
> >
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     return -EINVAL;
> > +}
> > +EXPORT_SYMBOL(vm_insert_range);
> > +
> >  /*
> >   *  sys_brk() for the most part doesn't need the global kernel
> >   *  lock, except when an application is doing something nasty
> > --
> > 1.9.1
> >
>
> --
> Sincerely yours,
> Mike.
>

Sure I will wait for some time to get additional review comments and
add all of those requested changes in v2.

Any further feedback on driver changes as part of this patch series ?

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-16 18:28   ` Mike Rapoport
  (?)
  (?)
@ 2018-11-17  6:56   ` Souptick Joarder
  -1 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-17  6:56 UTC (permalink / raw)
  To: rppt
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, Matthew Wilcox, airlied,
	linux-arm-kernel, linux-rockchip, treding, linux-media,
	Kees Cook, pawel, Rik van Riel, iommu, rppt, Boris Ostrovsky,
	mchehab, iamjoonsoo.kim, vbabka, Juergen Gross

On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Thu, Nov 15, 2018 at 09:15:30PM +0530, Souptick Joarder wrote:
> > Previouly drivers have their own way of mapping range of
> > kernel pages/memory into user vma and this was done by
> > invoking vm_insert_page() within a loop.
> >
> > As this pattern is common across different drivers, it can
> > be generalized by creating a new function and use it across
> > the drivers.
> >
> > vm_insert_range is the new API which will be used to map a
> > range of kernel memory/pages to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> > Reviewed-by: Matthew Wilcox <willy@infradead.org>
> > ---
> >  include/linux/mm_types.h |  3 +++
> >  mm/memory.c              | 28 ++++++++++++++++++++++++++++
> >  mm/nommu.c               |  7 +++++++
> >  3 files changed, 38 insertions(+)
> >
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 5ed8f62..15ae24f 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
> >  extern void tlb_finish_mmu(struct mmu_gather *tlb,
> >                               unsigned long start, unsigned long end);
> >
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count);
> > +
> >  static inline void init_tlb_flush_pending(struct mm_struct *mm)
> >  {
> >       atomic_set(&mm->tlb_flush_pending, 0);
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 15c417e..da904ed 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >
> >  /**
> > + * vm_insert_range - insert range of kernel pages into user vma
> > + * @vma: user vma to map to
> > + * @addr: target user address of this page
> > + * @pages: pointer to array of source kernel pages
> > + * @page_count: no. of pages need to insert into user vma
> > + *
> > + * This allows drivers to insert range of kernel pages they've allocated
> > + * into a user vma. This is a generic function which drivers can use
> > + * rather than using their own way of mapping range of kernel pages into
> > + * user vma.
>
> Please add the return value and context descriptions.
>
> > + */
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     unsigned long uaddr = addr;
> > +     int ret = 0, i;
> > +
> > +     for (i = 0; i < page_count; i++) {
> > +             ret = vm_insert_page(vma, uaddr, pages[i]);
> > +             if (ret < 0)
> > +                     return ret;
> > +             uaddr += PAGE_SIZE;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> >   * vm_insert_page - insert single page into user vma
> >   * @vma: user vma to map to
> >   * @addr: target user address of this page
> > diff --git a/mm/nommu.c b/mm/nommu.c
> > index 749276b..d6ef5c7 100644
> > --- a/mm/nommu.c
> > +++ b/mm/nommu.c
> > @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
> >  }
> >  EXPORT_SYMBOL(vm_insert_page);
> >
> > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > +                     struct page **pages, unsigned long page_count)
> > +{
> > +     return -EINVAL;
> > +}
> > +EXPORT_SYMBOL(vm_insert_range);
> > +
> >  /*
> >   *  sys_brk() for the most part doesn't need the global kernel
> >   *  lock, except when an application is doing something nasty
> > --
> > 1.9.1
> >
>
> --
> Sincerely yours,
> Mike.
>

Sure I will wait for some time to get additional review comments and
add all of those requested changes in v2.

Any further feedback on driver changes as part of this patch series ?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-17  6:56     ` Souptick Joarder
  (?)
@ 2018-11-17 14:37       ` Matthew Wilcox
  -1 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-17 14:37 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: rppt, Andrew Morton, Michal Hocko, Kirill A. Shutemov, vbabka,
	Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > + * vm_insert_range - insert range of kernel pages into user vma
> > > + * @vma: user vma to map to
> > > + * @addr: target user address of this page
> > > + * @pages: pointer to array of source kernel pages
> > > + * @page_count: no. of pages need to insert into user vma
> > > + *
> > > + * This allows drivers to insert range of kernel pages they've allocated
> > > + * into a user vma. This is a generic function which drivers can use
> > > + * rather than using their own way of mapping range of kernel pages into
> > > + * user vma.
> >
> > Please add the return value and context descriptions.
> >
> 
> Sure I will wait for some time to get additional review comments and
> add all of those requested changes in v2.

You could send your proposed wording now which might remove the need
for a v3 if we end up arguing about the wording.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-17 14:37       ` Matthew Wilcox
  0 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-17 14:37 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, rppt, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, hjc

On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > + * vm_insert_range - insert range of kernel pages into user vma
> > > + * @vma: user vma to map to
> > > + * @addr: target user address of this page
> > > + * @pages: pointer to array of source kernel pages
> > > + * @page_count: no. of pages need to insert into user vma
> > > + *
> > > + * This allows drivers to insert range of kernel pages they've allocated
> > > + * into a user vma. This is a generic function which drivers can use
> > > + * rather than using their own way of mapping range of kernel pages into
> > > + * user vma.
> >
> > Please add the return value and context descriptions.
> >
> 
> Sure I will wait for some time to get additional review comments and
> add all of those requested changes in v2.

You could send your proposed wording now which might remove the need
for a v3 if we end up arguing about the wording.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-17 14:37       ` Matthew Wilcox
  0 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-17 14:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > + * vm_insert_range - insert range of kernel pages into user vma
> > > + * @vma: user vma to map to
> > > + * @addr: target user address of this page
> > > + * @pages: pointer to array of source kernel pages
> > > + * @page_count: no. of pages need to insert into user vma
> > > + *
> > > + * This allows drivers to insert range of kernel pages they've allocated
> > > + * into a user vma. This is a generic function which drivers can use
> > > + * rather than using their own way of mapping range of kernel pages into
> > > + * user vma.
> >
> > Please add the return value and context descriptions.
> >
> 
> Sure I will wait for some time to get additional review comments and
> add all of those requested changes in v2.

You could send your proposed wording now which might remove the need
for a v3 if we end up arguing about the wording.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-17  6:56     ` Souptick Joarder
  (?)
  (?)
@ 2018-11-17 14:37     ` Matthew Wilcox
  -1 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-17 14:37 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, rppt, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, hjc

On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > + * vm_insert_range - insert range of kernel pages into user vma
> > > + * @vma: user vma to map to
> > > + * @addr: target user address of this page
> > > + * @pages: pointer to array of source kernel pages
> > > + * @page_count: no. of pages need to insert into user vma
> > > + *
> > > + * This allows drivers to insert range of kernel pages they've allocated
> > > + * into a user vma. This is a generic function which drivers can use
> > > + * rather than using their own way of mapping range of kernel pages into
> > > + * user vma.
> >
> > Please add the return value and context descriptions.
> >
> 
> Sure I will wait for some time to get additional review comments and
> add all of those requested changes in v2.

You could send your proposed wording now which might remove the need
for a v3 if we end up arguing about the wording.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-17 14:37       ` Matthew Wilcox
  (?)
@ 2018-11-19 15:13         ` Souptick Joarder
  -1 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-19 15:13 UTC (permalink / raw)
  To: Matthew Wilcox, rppt
  Cc: Andrew Morton, Michal Hocko, Kirill A. Shutemov, vbabka,
	Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

Hi Mike,

On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > + * @vma: user vma to map to
> > > > + * @addr: target user address of this page
> > > > + * @pages: pointer to array of source kernel pages
> > > > + * @page_count: no. of pages need to insert into user vma
> > > > + *
> > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > + * into a user vma. This is a generic function which drivers can use
> > > > + * rather than using their own way of mapping range of kernel pages into
> > > > + * user vma.
> > >
> > > Please add the return value and context descriptions.
> > >
> >
> > Sure I will wait for some time to get additional review comments and
> > add all of those requested changes in v2.
>
> You could send your proposed wording now which might remove the need
> for a v3 if we end up arguing about the wording.

Does this description looks good ?

/**
 * vm_insert_range - insert range of kernel pages into user vma
 * @vma: user vma to map to
 * @addr: target user address of this page
 * @pages: pointer to array of source kernel pages
 * @page_count: number of pages need to insert into user vma
 *
 * This allows drivers to insert range of kernel pages they've allocated
 * into a user vma. This is a generic function which drivers can use
 * rather than using their own way of mapping range of kernel pages into
 * user vma.
 *
 * Context - Process context. Called by mmap handlers.
 * Return - int error value
 * 0                    - OK
 * -EINVAL              - Invalid argument
 * -ENOMEM              - No memory
 * -EFAULT              - Bad address
 * -EBUSY               - Device or resource busy
 */

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-19 15:13         ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-19 15:13 UTC (permalink / raw)
  To: Matthew Wilcox, rppt
  Cc: Andrew Morton, Michal Hocko, Kirill A. Shutemov, vbabka,
	Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross

Hi Mike,

On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > + * @vma: user vma to map to
> > > > + * @addr: target user address of this page
> > > > + * @pages: pointer to array of source kernel pages
> > > > + * @page_count: no. of pages need to insert into user vma
> > > > + *
> > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > + * into a user vma. This is a generic function which drivers can use
> > > > + * rather than using their own way of mapping range of kernel pages into
> > > > + * user vma.
> > >
> > > Please add the return value and context descriptions.
> > >
> >
> > Sure I will wait for some time to get additional review comments and
> > add all of those requested changes in v2.
>
> You could send your proposed wording now which might remove the need
> for a v3 if we end up arguing about the wording.

Does this description looks good ?

/**
 * vm_insert_range - insert range of kernel pages into user vma
 * @vma: user vma to map to
 * @addr: target user address of this page
 * @pages: pointer to array of source kernel pages
 * @page_count: number of pages need to insert into user vma
 *
 * This allows drivers to insert range of kernel pages they've allocated
 * into a user vma. This is a generic function which drivers can use
 * rather than using their own way of mapping range of kernel pages into
 * user vma.
 *
 * Context - Process context. Called by mmap handlers.
 * Return - int error value
 * 0                    - OK
 * -EINVAL              - Invalid argument
 * -ENOMEM              - No memory
 * -EFAULT              - Bad address
 * -EBUSY               - Device or resource busy
 */

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-19 15:13         ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-19 15:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mike,

On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > + * @vma: user vma to map to
> > > > + * @addr: target user address of this page
> > > > + * @pages: pointer to array of source kernel pages
> > > > + * @page_count: no. of pages need to insert into user vma
> > > > + *
> > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > + * into a user vma. This is a generic function which drivers can use
> > > > + * rather than using their own way of mapping range of kernel pages into
> > > > + * user vma.
> > >
> > > Please add the return value and context descriptions.
> > >
> >
> > Sure I will wait for some time to get additional review comments and
> > add all of those requested changes in v2.
>
> You could send your proposed wording now which might remove the need
> for a v3 if we end up arguing about the wording.

Does this description looks good ?

/**
 * vm_insert_range - insert range of kernel pages into user vma
 * @vma: user vma to map to
 * @addr: target user address of this page
 * @pages: pointer to array of source kernel pages
 * @page_count: number of pages need to insert into user vma
 *
 * This allows drivers to insert range of kernel pages they've allocated
 * into a user vma. This is a generic function which drivers can use
 * rather than using their own way of mapping range of kernel pages into
 * user vma.
 *
 * Context - Process context. Called by mmap handlers.
 * Return - int error value
 * 0                    - OK
 * -EINVAL              - Invalid argument
 * -ENOMEM              - No memory
 * -EFAULT              - Bad address
 * -EBUSY               - Device or resource busy
 */

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-17 14:37       ` Matthew Wilcox
  (?)
  (?)
@ 2018-11-19 15:13       ` Souptick Joarder
  -1 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-19 15:13 UTC (permalink / raw)
  To: Matthew Wilcox, rppt
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, hjc, xen-devel

Hi Mike,

On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > + * @vma: user vma to map to
> > > > + * @addr: target user address of this page
> > > > + * @pages: pointer to array of source kernel pages
> > > > + * @page_count: no. of pages need to insert into user vma
> > > > + *
> > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > + * into a user vma. This is a generic function which drivers can use
> > > > + * rather than using their own way of mapping range of kernel pages into
> > > > + * user vma.
> > >
> > > Please add the return value and context descriptions.
> > >
> >
> > Sure I will wait for some time to get additional review comments and
> > add all of those requested changes in v2.
>
> You could send your proposed wording now which might remove the need
> for a v3 if we end up arguing about the wording.

Does this description looks good ?

/**
 * vm_insert_range - insert range of kernel pages into user vma
 * @vma: user vma to map to
 * @addr: target user address of this page
 * @pages: pointer to array of source kernel pages
 * @page_count: number of pages need to insert into user vma
 *
 * This allows drivers to insert range of kernel pages they've allocated
 * into a user vma. This is a generic function which drivers can use
 * rather than using their own way of mapping range of kernel pages into
 * user vma.
 *
 * Context - Process context. Called by mmap handlers.
 * Return - int error value
 * 0                    - OK
 * -EINVAL              - Invalid argument
 * -ENOMEM              - No memory
 * -EFAULT              - Bad address
 * -EBUSY               - Device or resource busy
 */

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-19 16:26           ` Mike Rapoport
  0 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-19 16:26 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Matthew Wilcox, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
	vbabka, Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> Hi Mike,
> 
> On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > + * @vma: user vma to map to
> > > > > + * @addr: target user address of this page
> > > > > + * @pages: pointer to array of source kernel pages
> > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > + *
> > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > + * user vma.
> > > >
> > > > Please add the return value and context descriptions.
> > > >
> > >
> > > Sure I will wait for some time to get additional review comments and
> > > add all of those requested changes in v2.
> >
> > You could send your proposed wording now which might remove the need
> > for a v3 if we end up arguing about the wording.
> 
> Does this description looks good ?
> 
> /**
>  * vm_insert_range - insert range of kernel pages into user vma
>  * @vma: user vma to map to
>  * @addr: target user address of this page
>  * @pages: pointer to array of source kernel pages
>  * @page_count: number of pages need to insert into user vma
>  *
>  * This allows drivers to insert range of kernel pages they've allocated
>  * into a user vma. This is a generic function which drivers can use
>  * rather than using their own way of mapping range of kernel pages into
>  * user vma.
>  *
>  * Context - Process context. Called by mmap handlers.

Context:

>  * Return - int error value

Return:

>  * 0                    - OK
>  * -EINVAL              - Invalid argument
>  * -ENOMEM              - No memory
>  * -EFAULT              - Bad address
>  * -EBUSY               - Device or resource busy

I don't think that elaborate description of error values is needed, just "0
on success and error code otherwise" would be sufficient.

>  */
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-19 16:26           ` Mike Rapoport
  0 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-19 16:26 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux-MM,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Stephen Rothwell, oleksandr_andrushchenko-uRwfk40T5oI,
	Russell King - ARM Linux, Matthew Wilcox, airlied-cv59FeDIM0c,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	treding-DDmLM1+adcrQT0dZR+AlfA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Kees Cook,
	pawel-FA/gS7QP4orQT0dZR+AlfA, Rik van Riel,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Boris Ostrovsky,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, iamjoonsoo.kim-Hm3cg6mZ9cc,
	vbabka-AlSwsSmVLrQ, Juergen Gross, hjc-TNX95d0MmH7DzftRWevZcw,
	xen-devel-GuqFBffKawuEi8DpZVb4nw, Kyungmin

On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> Hi Mike,
> 
> On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> >
> > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> wrote:
> > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > + * @vma: user vma to map to
> > > > > + * @addr: target user address of this page
> > > > > + * @pages: pointer to array of source kernel pages
> > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > + *
> > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > + * user vma.
> > > >
> > > > Please add the return value and context descriptions.
> > > >
> > >
> > > Sure I will wait for some time to get additional review comments and
> > > add all of those requested changes in v2.
> >
> > You could send your proposed wording now which might remove the need
> > for a v3 if we end up arguing about the wording.
> 
> Does this description looks good ?
> 
> /**
>  * vm_insert_range - insert range of kernel pages into user vma
>  * @vma: user vma to map to
>  * @addr: target user address of this page
>  * @pages: pointer to array of source kernel pages
>  * @page_count: number of pages need to insert into user vma
>  *
>  * This allows drivers to insert range of kernel pages they've allocated
>  * into a user vma. This is a generic function which drivers can use
>  * rather than using their own way of mapping range of kernel pages into
>  * user vma.
>  *
>  * Context - Process context. Called by mmap handlers.

Context:

>  * Return - int error value

Return:

>  * 0                    - OK
>  * -EINVAL              - Invalid argument
>  * -ENOMEM              - No memory
>  * -EFAULT              - Bad address
>  * -EBUSY               - Device or resource busy

I don't think that elaborate description of error values is needed, just "0
on success and error code otherwise" would be sufficient.

>  */
> 

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-19 16:26           ` Mike Rapoport
  0 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-19 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> Hi Mike,
> 
> On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > + * @vma: user vma to map to
> > > > > + * @addr: target user address of this page
> > > > > + * @pages: pointer to array of source kernel pages
> > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > + *
> > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > + * user vma.
> > > >
> > > > Please add the return value and context descriptions.
> > > >
> > >
> > > Sure I will wait for some time to get additional review comments and
> > > add all of those requested changes in v2.
> >
> > You could send your proposed wording now which might remove the need
> > for a v3 if we end up arguing about the wording.
> 
> Does this description looks good ?
> 
> /**
>  * vm_insert_range - insert range of kernel pages into user vma
>  * @vma: user vma to map to
>  * @addr: target user address of this page
>  * @pages: pointer to array of source kernel pages
>  * @page_count: number of pages need to insert into user vma
>  *
>  * This allows drivers to insert range of kernel pages they've allocated
>  * into a user vma. This is a generic function which drivers can use
>  * rather than using their own way of mapping range of kernel pages into
>  * user vma.
>  *
>  * Context - Process context. Called by mmap handlers.

Context:

>  * Return - int error value

Return:

>  * 0                    - OK
>  * -EINVAL              - Invalid argument
>  * -ENOMEM              - No memory
>  * -EFAULT              - Bad address
>  * -EBUSY               - Device or resource busy

I don't think that elaborate description of error values is needed, just "0
on success and error code otherwise" would be sufficient.

>  */
> 

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-19 15:13         ` Souptick Joarder
                           ` (2 preceding siblings ...)
  (?)
@ 2018-11-19 16:26         ` Mike Rapoport
  -1 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-19 16:26 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, Matthew Wilcox, airlied,
	linux-arm-kernel, linux-rockchip, treding, linux-media,
	Kees Cook, pawel, Rik van Riel, iommu, rppt, Boris Ostrovsky,
	mchehab, iamjoonsoo.kim, vbabka, Juergen Gross

On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> Hi Mike,
> 
> On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > + * @vma: user vma to map to
> > > > > + * @addr: target user address of this page
> > > > > + * @pages: pointer to array of source kernel pages
> > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > + *
> > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > + * user vma.
> > > >
> > > > Please add the return value and context descriptions.
> > > >
> > >
> > > Sure I will wait for some time to get additional review comments and
> > > add all of those requested changes in v2.
> >
> > You could send your proposed wording now which might remove the need
> > for a v3 if we end up arguing about the wording.
> 
> Does this description looks good ?
> 
> /**
>  * vm_insert_range - insert range of kernel pages into user vma
>  * @vma: user vma to map to
>  * @addr: target user address of this page
>  * @pages: pointer to array of source kernel pages
>  * @page_count: number of pages need to insert into user vma
>  *
>  * This allows drivers to insert range of kernel pages they've allocated
>  * into a user vma. This is a generic function which drivers can use
>  * rather than using their own way of mapping range of kernel pages into
>  * user vma.
>  *
>  * Context - Process context. Called by mmap handlers.

Context:

>  * Return - int error value

Return:

>  * 0                    - OK
>  * -EINVAL              - Invalid argument
>  * -ENOMEM              - No memory
>  * -EFAULT              - Bad address
>  * -EBUSY               - Device or resource busy

I don't think that elaborate description of error values is needed, just "0
on success and error code otherwise" would be sufficient.

>  */
> 

-- 
Sincerely yours,
Mike.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-19 16:26           ` Mike Rapoport
  (?)
@ 2018-11-19 17:45             ` Souptick Joarder
  -1 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-19 17:45 UTC (permalink / raw)
  To: rppt
  Cc: Matthew Wilcox, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
	vbabka, Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Mon, Nov 19, 2018 at 9:56 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> > Hi Mike,
> >
> > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > > + * @vma: user vma to map to
> > > > > > + * @addr: target user address of this page
> > > > > > + * @pages: pointer to array of source kernel pages
> > > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > > + *
> > > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > > + * user vma.
> > > > >
> > > > > Please add the return value and context descriptions.
> > > > >
> > > >
> > > > Sure I will wait for some time to get additional review comments and
> > > > add all of those requested changes in v2.
> > >
> > > You could send your proposed wording now which might remove the need
> > > for a v3 if we end up arguing about the wording.
> >
> > Does this description looks good ?
> >
> > /**
> >  * vm_insert_range - insert range of kernel pages into user vma
> >  * @vma: user vma to map to
> >  * @addr: target user address of this page
> >  * @pages: pointer to array of source kernel pages
> >  * @page_count: number of pages need to insert into user vma
> >  *
> >  * This allows drivers to insert range of kernel pages they've allocated
> >  * into a user vma. This is a generic function which drivers can use
> >  * rather than using their own way of mapping range of kernel pages into
> >  * user vma.
> >  *
> >  * Context - Process context. Called by mmap handlers.
>
> Context:
>
> >  * Return - int error value
>
> Return:
>
> >  * 0                    - OK
> >  * -EINVAL              - Invalid argument
> >  * -ENOMEM              - No memory
> >  * -EFAULT              - Bad address
> >  * -EBUSY               - Device or resource busy
>
> I don't think that elaborate description of error values is needed, just "0
> on success and error code otherwise" would be sufficient.

/**
 * vm_insert_range - insert range of kernel pages into user vma
 * @vma: user vma to map to
 * @addr: target user address of this page
 * @pages: pointer to array of source kernel pages
 * @page_count: number of pages need to insert into user vma
 *
 * This allows drivers to insert range of kernel pages they've allocated
 * into a user vma. This is a generic function which drivers can use
 * rather than using their own way of mapping range of kernel pages into
 * user vma.
 *
 * Context: Process context. Called by mmap handlers.
 * Return: 0 on success and error code otherwise
 */

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-19 17:45             ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-19 17:45 UTC (permalink / raw)
  To: rppt
  Cc: Matthew Wilcox, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
	vbabka, Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky

On Mon, Nov 19, 2018 at 9:56 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> > Hi Mike,
> >
> > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > > + * @vma: user vma to map to
> > > > > > + * @addr: target user address of this page
> > > > > > + * @pages: pointer to array of source kernel pages
> > > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > > + *
> > > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > > + * user vma.
> > > > >
> > > > > Please add the return value and context descriptions.
> > > > >
> > > >
> > > > Sure I will wait for some time to get additional review comments and
> > > > add all of those requested changes in v2.
> > >
> > > You could send your proposed wording now which might remove the need
> > > for a v3 if we end up arguing about the wording.
> >
> > Does this description looks good ?
> >
> > /**
> >  * vm_insert_range - insert range of kernel pages into user vma
> >  * @vma: user vma to map to
> >  * @addr: target user address of this page
> >  * @pages: pointer to array of source kernel pages
> >  * @page_count: number of pages need to insert into user vma
> >  *
> >  * This allows drivers to insert range of kernel pages they've allocated
> >  * into a user vma. This is a generic function which drivers can use
> >  * rather than using their own way of mapping range of kernel pages into
> >  * user vma.
> >  *
> >  * Context - Process context. Called by mmap handlers.
>
> Context:
>
> >  * Return - int error value
>
> Return:
>
> >  * 0                    - OK
> >  * -EINVAL              - Invalid argument
> >  * -ENOMEM              - No memory
> >  * -EFAULT              - Bad address
> >  * -EBUSY               - Device or resource busy
>
> I don't think that elaborate description of error values is needed, just "0
> on success and error code otherwise" would be sufficient.

/**
 * vm_insert_range - insert range of kernel pages into user vma
 * @vma: user vma to map to
 * @addr: target user address of this page
 * @pages: pointer to array of source kernel pages
 * @page_count: number of pages need to insert into user vma
 *
 * This allows drivers to insert range of kernel pages they've allocated
 * into a user vma. This is a generic function which drivers can use
 * rather than using their own way of mapping range of kernel pages into
 * user vma.
 *
 * Context: Process context. Called by mmap handlers.
 * Return: 0 on success and error code otherwise
 */

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-19 17:45             ` Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-19 17:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 19, 2018 at 9:56 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> > Hi Mike,
> >
> > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > > + * @vma: user vma to map to
> > > > > > + * @addr: target user address of this page
> > > > > > + * @pages: pointer to array of source kernel pages
> > > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > > + *
> > > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > > + * user vma.
> > > > >
> > > > > Please add the return value and context descriptions.
> > > > >
> > > >
> > > > Sure I will wait for some time to get additional review comments and
> > > > add all of those requested changes in v2.
> > >
> > > You could send your proposed wording now which might remove the need
> > > for a v3 if we end up arguing about the wording.
> >
> > Does this description looks good ?
> >
> > /**
> >  * vm_insert_range - insert range of kernel pages into user vma
> >  * @vma: user vma to map to
> >  * @addr: target user address of this page
> >  * @pages: pointer to array of source kernel pages
> >  * @page_count: number of pages need to insert into user vma
> >  *
> >  * This allows drivers to insert range of kernel pages they've allocated
> >  * into a user vma. This is a generic function which drivers can use
> >  * rather than using their own way of mapping range of kernel pages into
> >  * user vma.
> >  *
> >  * Context - Process context. Called by mmap handlers.
>
> Context:
>
> >  * Return - int error value
>
> Return:
>
> >  * 0                    - OK
> >  * -EINVAL              - Invalid argument
> >  * -ENOMEM              - No memory
> >  * -EFAULT              - Bad address
> >  * -EBUSY               - Device or resource busy
>
> I don't think that elaborate description of error values is needed, just "0
> on success and error code otherwise" would be sufficient.

/**
 * vm_insert_range - insert range of kernel pages into user vma
 * @vma: user vma to map to
 * @addr: target user address of this page
 * @pages: pointer to array of source kernel pages
 * @page_count: number of pages need to insert into user vma
 *
 * This allows drivers to insert range of kernel pages they've allocated
 * into a user vma. This is a generic function which drivers can use
 * rather than using their own way of mapping range of kernel pages into
 * user vma.
 *
 * Context: Process context. Called by mmap handlers.
 * Return: 0 on success and error code otherwise
 */

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-19 16:26           ` Mike Rapoport
  (?)
  (?)
@ 2018-11-19 17:45           ` Souptick Joarder
  -1 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-19 17:45 UTC (permalink / raw)
  To: rppt
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, Matthew Wilcox, airlied,
	linux-arm-kernel, linux-rockchip, treding, linux-media,
	Kees Cook, pawel, Rik van Riel, iommu, rppt, Boris Ostrovsky,
	mchehab, iamjoonsoo.kim, vbabka, Juergen Gross

On Mon, Nov 19, 2018 at 9:56 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
>
> On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> > Hi Mike,
> >
> > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > > + * @vma: user vma to map to
> > > > > > + * @addr: target user address of this page
> > > > > > + * @pages: pointer to array of source kernel pages
> > > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > > + *
> > > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > > + * user vma.
> > > > >
> > > > > Please add the return value and context descriptions.
> > > > >
> > > >
> > > > Sure I will wait for some time to get additional review comments and
> > > > add all of those requested changes in v2.
> > >
> > > You could send your proposed wording now which might remove the need
> > > for a v3 if we end up arguing about the wording.
> >
> > Does this description looks good ?
> >
> > /**
> >  * vm_insert_range - insert range of kernel pages into user vma
> >  * @vma: user vma to map to
> >  * @addr: target user address of this page
> >  * @pages: pointer to array of source kernel pages
> >  * @page_count: number of pages need to insert into user vma
> >  *
> >  * This allows drivers to insert range of kernel pages they've allocated
> >  * into a user vma. This is a generic function which drivers can use
> >  * rather than using their own way of mapping range of kernel pages into
> >  * user vma.
> >  *
> >  * Context - Process context. Called by mmap handlers.
>
> Context:
>
> >  * Return - int error value
>
> Return:
>
> >  * 0                    - OK
> >  * -EINVAL              - Invalid argument
> >  * -ENOMEM              - No memory
> >  * -EFAULT              - Bad address
> >  * -EBUSY               - Device or resource busy
>
> I don't think that elaborate description of error values is needed, just "0
> on success and error code otherwise" would be sufficient.

/**
 * vm_insert_range - insert range of kernel pages into user vma
 * @vma: user vma to map to
 * @addr: target user address of this page
 * @pages: pointer to array of source kernel pages
 * @page_count: number of pages need to insert into user vma
 *
 * This allows drivers to insert range of kernel pages they've allocated
 * into a user vma. This is a generic function which drivers can use
 * rather than using their own way of mapping range of kernel pages into
 * user vma.
 *
 * Context: Process context. Called by mmap handlers.
 * Return: 0 on success and error code otherwise
 */

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-21 11:19           ` William Kucharski
  0 siblings, 0 replies; 73+ messages in thread
From: William Kucharski @ 2018-11-21 11:19 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Matthew Wilcox, rppt, Andrew Morton, Michal Hocko,
	Kirill A. Shutemov, vbabka, Rik van Riel, Stephen Rothwell, rppt,
	Peter Zijlstra, Russell King - ARM Linux, robin.murphy,
	iamjoonsoo.kim, treding, Kees Cook, Marek Szyprowski, stefanr,
	hjc, Heiko Stuebner, airlied, oleksandr_andrushchenko, joro,
	pawel, Kyungmin Park, mchehab, Boris Ostrovsky, Juergen Gross,
	linux-kernel, Linux-MM, linux-arm-kernel, linux1394-devel,
	dri-devel, linux-rockchip, xen-devel, iommu, linux-media

Could you add a line to the description explicitly stating that a failure
to insert any page in the range will fail the entire routine, something
like:

> * This allows drivers to insert range of kernel pages they've allocated
> * into a user vma. This is a generic function which drivers can use
> * rather than using their own way of mapping range of kernel pages into
> * user vma.
> *
> * A failure to insert any page in the range will fail the call as a whole.

It's obvious when reading the code, but it would be self-documenting to
state it outright.

Thanks!
    -- Bill



^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-21 11:19           ` William Kucharski
  0 siblings, 0 replies; 73+ messages in thread
From: William Kucharski @ 2018-11-21 11:19 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux-MM,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Marek Szyprowski, Stephen Rothwell,
	oleksandr_andrushchenko-uRwfk40T5oI, joro-zLv9SwRftAIdnm+yROfE0A,
	Russell King - ARM Linux, Matthew Wilcox,
	rppt-tEXmvtCZX7AybS5Ee8rs3A, airlied-cv59FeDIM0c,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	treding-DDmLM1+adcrQT0dZR+AlfA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Kees Cook,
	pawel-FA/gS7QP4orQT0dZR+AlfA, Rik van Riel,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Boris Ostrovsky,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, iamjoonsoo.kim-Hm3cg6mZ9cc,
	vbabka-AlSwsSmVLrQ, Juergen

Could you add a line to the description explicitly stating that a failure
to insert any page in the range will fail the entire routine, something
like:

> * This allows drivers to insert range of kernel pages they've allocated
> * into a user vma. This is a generic function which drivers can use
> * rather than using their own way of mapping range of kernel pages into
> * user vma.
> *
> * A failure to insert any page in the range will fail the call as a whole.

It's obvious when reading the code, but it would be self-documenting to
state it outright.

Thanks!
    -- Bill

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-21 11:19           ` William Kucharski
  0 siblings, 0 replies; 73+ messages in thread
From: William Kucharski @ 2018-11-21 11:19 UTC (permalink / raw)
  To: linux-arm-kernel

Could you add a line to the description explicitly stating that a failure
to insert any page in the range will fail the entire routine, something
like:

> * This allows drivers to insert range of kernel pages they've allocated
> * into a user vma. This is a generic function which drivers can use
> * rather than using their own way of mapping range of kernel pages into
> * user vma.
> *
> * A failure to insert any page in the range will fail the call as a whole.

It's obvious when reading the code, but it would be self-documenting to
state it outright.

Thanks!
    -- Bill

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-19 15:13         ` Souptick Joarder
                           ` (3 preceding siblings ...)
  (?)
@ 2018-11-21 11:19         ` William Kucharski
  -1 siblings, 0 replies; 73+ messages in thread
From: William Kucharski @ 2018-11-21 11:19 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, Matthew Wilcox, rppt, airlied,
	linux-arm-kernel, linux-rockchip, treding, linux-media,
	Kees Cook, pawel, Rik van Riel, iommu, rppt, Boris Ostrovsky,
	mchehab, iamjoonsoo.kim, vbabka, Juergen

Could you add a line to the description explicitly stating that a failure
to insert any page in the range will fail the entire routine, something
like:

> * This allows drivers to insert range of kernel pages they've allocated
> * into a user vma. This is a generic function which drivers can use
> * rather than using their own way of mapping range of kernel pages into
> * user vma.
> *
> * A failure to insert any page in the range will fail the call as a whole.

It's obvious when reading the code, but it would be self-documenting to
state it outright.

Thanks!
    -- Bill



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-21 11:19           ` William Kucharski
  (?)
@ 2018-11-21 12:35             ` Matthew Wilcox
  -1 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-21 12:35 UTC (permalink / raw)
  To: William Kucharski
  Cc: Souptick Joarder, rppt, Andrew Morton, Michal Hocko,
	Kirill A. Shutemov, vbabka, Rik van Riel, Stephen Rothwell, rppt,
	Peter Zijlstra, Russell King - ARM Linux, robin.murphy,
	iamjoonsoo.kim, treding, Kees Cook, Marek Szyprowski, stefanr,
	hjc, Heiko Stuebner, airlied, oleksandr_andrushchenko, joro,
	pawel, Kyungmin Park, mchehab, Boris Ostrovsky, Juergen Gross,
	linux-kernel, Linux-MM, linux-arm-kernel, linux1394-devel,
	dri-devel, linux-rockchip, xen-devel, iommu, linux-media

On Wed, Nov 21, 2018 at 04:19:11AM -0700, William Kucharski wrote:
> Could you add a line to the description explicitly stating that a failure
> to insert any page in the range will fail the entire routine, something
> like:
> 
> > * This allows drivers to insert range of kernel pages they've allocated
> > * into a user vma. This is a generic function which drivers can use
> > * rather than using their own way of mapping range of kernel pages into
> > * user vma.
> > *
> > * A failure to insert any page in the range will fail the call as a whole.
> 
> It's obvious when reading the code, but it would be self-documenting to
> state it outright.

It's probably better to be more explicit and answer Randy's question:

 * If we fail to insert any page into the vma, the function will return
 * immediately leaving any previously-inserted pages present.  Callers
 * from the mmap handler may immediately return the error as their
 * caller will destroy the vma, removing any successfully-inserted pages.
 * Other callers should make their own arrangements for calling unmap_region().

Although unmap_region() is static so there clearly isn't any code in the
kernel today other than in mmap handlers (or fault handlers) that needs to
insert pages into a VMA.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-21 12:35             ` Matthew Wilcox
  0 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-21 12:35 UTC (permalink / raw)
  To: William Kucharski
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, rppt, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, hjc

On Wed, Nov 21, 2018 at 04:19:11AM -0700, William Kucharski wrote:
> Could you add a line to the description explicitly stating that a failure
> to insert any page in the range will fail the entire routine, something
> like:
> 
> > * This allows drivers to insert range of kernel pages they've allocated
> > * into a user vma. This is a generic function which drivers can use
> > * rather than using their own way of mapping range of kernel pages into
> > * user vma.
> > *
> > * A failure to insert any page in the range will fail the call as a whole.
> 
> It's obvious when reading the code, but it would be self-documenting to
> state it outright.

It's probably better to be more explicit and answer Randy's question:

 * If we fail to insert any page into the vma, the function will return
 * immediately leaving any previously-inserted pages present.  Callers
 * from the mmap handler may immediately return the error as their
 * caller will destroy the vma, removing any successfully-inserted pages.
 * Other callers should make their own arrangements for calling unmap_region().

Although unmap_region() is static so there clearly isn't any code in the
kernel today other than in mmap handlers (or fault handlers) that needs to
insert pages into a VMA.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-21 12:35             ` Matthew Wilcox
  0 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-21 12:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 21, 2018 at 04:19:11AM -0700, William Kucharski wrote:
> Could you add a line to the description explicitly stating that a failure
> to insert any page in the range will fail the entire routine, something
> like:
> 
> > * This allows drivers to insert range of kernel pages they've allocated
> > * into a user vma. This is a generic function which drivers can use
> > * rather than using their own way of mapping range of kernel pages into
> > * user vma.
> > *
> > * A failure to insert any page in the range will fail the call as a whole.
> 
> It's obvious when reading the code, but it would be self-documenting to
> state it outright.

It's probably better to be more explicit and answer Randy's question:

 * If we fail to insert any page into the vma, the function will return
 * immediately leaving any previously-inserted pages present.  Callers
 * from the mmap handler may immediately return the error as their
 * caller will destroy the vma, removing any successfully-inserted pages.
 * Other callers should make their own arrangements for calling unmap_region().

Although unmap_region() is static so there clearly isn't any code in the
kernel today other than in mmap handlers (or fault handlers) that needs to
insert pages into a VMA.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-21 11:19           ` William Kucharski
                             ` (2 preceding siblings ...)
  (?)
@ 2018-11-21 12:35           ` Matthew Wilcox
  -1 siblings, 0 replies; 73+ messages in thread
From: Matthew Wilcox @ 2018-11-21 12:35 UTC (permalink / raw)
  To: William Kucharski
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, rppt, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, hjc

On Wed, Nov 21, 2018 at 04:19:11AM -0700, William Kucharski wrote:
> Could you add a line to the description explicitly stating that a failure
> to insert any page in the range will fail the entire routine, something
> like:
> 
> > * This allows drivers to insert range of kernel pages they've allocated
> > * into a user vma. This is a generic function which drivers can use
> > * rather than using their own way of mapping range of kernel pages into
> > * user vma.
> > *
> > * A failure to insert any page in the range will fail the call as a whole.
> 
> It's obvious when reading the code, but it would be self-documenting to
> state it outright.

It's probably better to be more explicit and answer Randy's question:

 * If we fail to insert any page into the vma, the function will return
 * immediately leaving any previously-inserted pages present.  Callers
 * from the mmap handler may immediately return the error as their
 * caller will destroy the vma, removing any successfully-inserted pages.
 * Other callers should make their own arrangements for calling unmap_region().

Although unmap_region() is static so there clearly isn't any code in the
kernel today other than in mmap handlers (or fault handlers) that needs to
insert pages into a VMA.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-21 14:29               ` William Kucharski
  0 siblings, 0 replies; 73+ messages in thread
From: William Kucharski @ 2018-11-21 14:29 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Souptick Joarder, rppt, Andrew Morton, Michal Hocko,
	Kirill A. Shutemov, vbabka, Rik van Riel, Stephen Rothwell, rppt,
	Peter Zijlstra, Russell King - ARM Linux, robin.murphy,
	iamjoonsoo.kim, treding, Kees Cook, Marek Szyprowski, stefanr,
	hjc, Heiko Stuebner, airlied, oleksandr_andrushchenko, joro,
	pawel, Kyungmin Park, mchehab, Boris Ostrovsky, Juergen Gross,
	linux-kernel, Linux-MM, linux-arm-kernel, linux1394-devel,
	dri-devel, linux-rockchip, xen-devel, iommu, linux-media



> On Nov 21, 2018, at 5:35 AM, Matthew Wilcox <willy@infradead.org> wrote:
> 
> It's probably better to be more explicit and answer Randy's question:
> 
> * If we fail to insert any page into the vma, the function will return
> * immediately leaving any previously-inserted pages present.  Callers
> * from the mmap handler may immediately return the error as their
> * caller will destroy the vma, removing any successfully-inserted pages.
> * Other callers should make their own arrangements for calling unmap_region().

That works for me as well.



^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-21 14:29               ` William Kucharski
  0 siblings, 0 replies; 73+ messages in thread
From: William Kucharski @ 2018-11-21 14:29 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux-MM,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Marek Szyprowski, Stephen Rothwell,
	oleksandr_andrushchenko-uRwfk40T5oI, joro-zLv9SwRftAIdnm+yROfE0A,
	Russell King - ARM Linux,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	rppt-tEXmvtCZX7AybS5Ee8rs3A, airlied-cv59FeDIM0c,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	treding-DDmLM1+adcrQT0dZR+AlfA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Kees Cook,
	pawel-FA/gS7QP4orQT0dZR+AlfA, Rik van Riel,
	rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Boris Ostrovsky,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, iamjoonsoo.kim-Hm3cg6mZ9cc,
	vbabka-AlSwsSmVLrQ, Juergen Gross, hjc



> On Nov 21, 2018, at 5:35 AM, Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> 
> It's probably better to be more explicit and answer Randy's question:
> 
> * If we fail to insert any page into the vma, the function will return
> * immediately leaving any previously-inserted pages present.  Callers
> * from the mmap handler may immediately return the error as their
> * caller will destroy the vma, removing any successfully-inserted pages.
> * Other callers should make their own arrangements for calling unmap_region().

That works for me as well.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-21 14:29               ` William Kucharski
  0 siblings, 0 replies; 73+ messages in thread
From: William Kucharski @ 2018-11-21 14:29 UTC (permalink / raw)
  To: linux-arm-kernel



> On Nov 21, 2018, at 5:35 AM, Matthew Wilcox <willy@infradead.org> wrote:
> 
> It's probably better to be more explicit and answer Randy's question:
> 
> * If we fail to insert any page into the vma, the function will return
> * immediately leaving any previously-inserted pages present.  Callers
> * from the mmap handler may immediately return the error as their
> * caller will destroy the vma, removing any successfully-inserted pages.
> * Other callers should make their own arrangements for calling unmap_region().

That works for me as well.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-21 12:35             ` Matthew Wilcox
                               ` (2 preceding siblings ...)
  (?)
@ 2018-11-21 14:29             ` William Kucharski
  -1 siblings, 0 replies; 73+ messages in thread
From: William Kucharski @ 2018-11-21 14:29 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, iommu, rppt, airlied, linux-arm-kernel,
	linux-rockchip, treding, linux-media, Kees Cook, pawel,
	Rik van Riel, rppt, Boris Ostrovsky, mchehab, iamjoonsoo.kim,
	vbabka, Juergen Gross, hjc



> On Nov 21, 2018, at 5:35 AM, Matthew Wilcox <willy@infradead.org> wrote:
> 
> It's probably better to be more explicit and answer Randy's question:
> 
> * If we fail to insert any page into the vma, the function will return
> * immediately leaving any previously-inserted pages present.  Callers
> * from the mmap handler may immediately return the error as their
> * caller will destroy the vma, removing any successfully-inserted pages.
> * Other callers should make their own arrangements for calling unmap_region().

That works for me as well.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-21 12:35             ` Matthew Wilcox
  (?)
  (?)
@ 2018-11-21 14:29             ` William Kucharski
  -1 siblings, 0 replies; 73+ messages in thread
From: William Kucharski @ 2018-11-21 14:29 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Michal Hocko, Peter Zijlstra, dri-devel, Linux-MM,
	linux1394-devel, Marek Szyprowski, Stephen Rothwell,
	oleksandr_andrushchenko, Russell King - ARM Linux, rppt,
	Kyungmin Park, airlied, vbabka, linux-rockchip, treding,
	linux-media, Kees Cook, pawel, Rik van Riel, rppt,
	Boris Ostrovsky, mchehab, Andrew Morton, linux-arm-kernel,
	Juergen Gross, linux-kernel, xen-devel, iommu



> On Nov 21, 2018, at 5:35 AM, Matthew Wilcox <willy@infradead.org> wrote:
> 
> It's probably better to be more explicit and answer Randy's question:
> 
> * If we fail to insert any page into the vma, the function will return
> * immediately leaving any previously-inserted pages present.  Callers
> * from the mmap handler may immediately return the error as their
> * caller will destroy the vma, removing any successfully-inserted pages.
> * Other callers should make their own arrangements for calling unmap_region().

That works for me as well.


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-23  7:19               ` Mike Rapoport
  0 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-23  7:19 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Matthew Wilcox, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
	vbabka, Rik van Riel, Stephen Rothwell, rppt, Peter Zijlstra,
	Russell King - ARM Linux, robin.murphy, iamjoonsoo.kim, treding,
	Kees Cook, Marek Szyprowski, stefanr, hjc, Heiko Stuebner,
	airlied, oleksandr_andrushchenko, joro, pawel, Kyungmin Park,
	mchehab, Boris Ostrovsky, Juergen Gross, linux-kernel, Linux-MM,
	linux-arm-kernel, linux1394-devel, dri-devel, linux-rockchip,
	xen-devel, iommu, linux-media

On Mon, Nov 19, 2018 at 11:15:15PM +0530, Souptick Joarder wrote:
> On Mon, Nov 19, 2018 at 9:56 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> >
> > On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> > > Hi Mike,
> > >
> > > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > >
> > > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > > > + * @vma: user vma to map to
> > > > > > > + * @addr: target user address of this page
> > > > > > > + * @pages: pointer to array of source kernel pages
> > > > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > > > + *
> > > > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > > > + * user vma.
> > > > > >
> > > > > > Please add the return value and context descriptions.
> > > > > >
> > > > >
> > > > > Sure I will wait for some time to get additional review comments and
> > > > > add all of those requested changes in v2.
> > > >
> > > > You could send your proposed wording now which might remove the need
> > > > for a v3 if we end up arguing about the wording.
> > >
> > > Does this description looks good ?
> > >
> > > /**
> > >  * vm_insert_range - insert range of kernel pages into user vma
> > >  * @vma: user vma to map to
> > >  * @addr: target user address of this page
> > >  * @pages: pointer to array of source kernel pages
> > >  * @page_count: number of pages need to insert into user vma
> > >  *
> > >  * This allows drivers to insert range of kernel pages they've allocated
> > >  * into a user vma. This is a generic function which drivers can use
> > >  * rather than using their own way of mapping range of kernel pages into
> > >  * user vma.
> > >  *
> > >  * Context - Process context. Called by mmap handlers.
> >
> > Context:
> >
> > >  * Return - int error value
> >
> > Return:
> >
> > >  * 0                    - OK
> > >  * -EINVAL              - Invalid argument
> > >  * -ENOMEM              - No memory
> > >  * -EFAULT              - Bad address
> > >  * -EBUSY               - Device or resource busy
> >
> > I don't think that elaborate description of error values is needed, just "0
> > on success and error code otherwise" would be sufficient.
> 
> /**
>  * vm_insert_range - insert range of kernel pages into user vma
>  * @vma: user vma to map to
>  * @addr: target user address of this page
>  * @pages: pointer to array of source kernel pages
>  * @page_count: number of pages need to insert into user vma
>  *
>  * This allows drivers to insert range of kernel pages they've allocated
>  * into a user vma. This is a generic function which drivers can use
>  * rather than using their own way of mapping range of kernel pages into
>  * user vma.
>  *
>  * Context: Process context. Called by mmap handlers.
>  * Return: 0 on success and error code otherwise
>  */

Looks good to me.

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-23  7:19               ` Mike Rapoport
  0 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-23  7:19 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux-MM,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Stephen Rothwell, oleksandr_andrushchenko-uRwfk40T5oI,
	Russell King - ARM Linux, Matthew Wilcox, airlied-cv59FeDIM0c,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	treding-DDmLM1+adcrQT0dZR+AlfA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Kees Cook,
	pawel-FA/gS7QP4orQT0dZR+AlfA, Rik van Riel,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	rppt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Boris Ostrovsky,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, iamjoonsoo.kim-Hm3cg6mZ9cc,
	vbabka-AlSwsSmVLrQ, Juergen Gross, hjc-TNX95d0MmH7DzftRWevZcw,
	xen-devel-GuqFBffKawuEi8DpZVb4nw, Kyungmin

On Mon, Nov 19, 2018 at 11:15:15PM +0530, Souptick Joarder wrote:
> On Mon, Nov 19, 2018 at 9:56 PM Mike Rapoport <rppt-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> wrote:
> >
> > On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> > > Hi Mike,
> > >
> > > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> > > >
> > > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> wrote:
> > > > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > > > + * @vma: user vma to map to
> > > > > > > + * @addr: target user address of this page
> > > > > > > + * @pages: pointer to array of source kernel pages
> > > > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > > > + *
> > > > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > > > + * user vma.
> > > > > >
> > > > > > Please add the return value and context descriptions.
> > > > > >
> > > > >
> > > > > Sure I will wait for some time to get additional review comments and
> > > > > add all of those requested changes in v2.
> > > >
> > > > You could send your proposed wording now which might remove the need
> > > > for a v3 if we end up arguing about the wording.
> > >
> > > Does this description looks good ?
> > >
> > > /**
> > >  * vm_insert_range - insert range of kernel pages into user vma
> > >  * @vma: user vma to map to
> > >  * @addr: target user address of this page
> > >  * @pages: pointer to array of source kernel pages
> > >  * @page_count: number of pages need to insert into user vma
> > >  *
> > >  * This allows drivers to insert range of kernel pages they've allocated
> > >  * into a user vma. This is a generic function which drivers can use
> > >  * rather than using their own way of mapping range of kernel pages into
> > >  * user vma.
> > >  *
> > >  * Context - Process context. Called by mmap handlers.
> >
> > Context:
> >
> > >  * Return - int error value
> >
> > Return:
> >
> > >  * 0                    - OK
> > >  * -EINVAL              - Invalid argument
> > >  * -ENOMEM              - No memory
> > >  * -EFAULT              - Bad address
> > >  * -EBUSY               - Device or resource busy
> >
> > I don't think that elaborate description of error values is needed, just "0
> > on success and error code otherwise" would be sufficient.
> 
> /**
>  * vm_insert_range - insert range of kernel pages into user vma
>  * @vma: user vma to map to
>  * @addr: target user address of this page
>  * @pages: pointer to array of source kernel pages
>  * @page_count: number of pages need to insert into user vma
>  *
>  * This allows drivers to insert range of kernel pages they've allocated
>  * into a user vma. This is a generic function which drivers can use
>  * rather than using their own way of mapping range of kernel pages into
>  * user vma.
>  *
>  * Context: Process context. Called by mmap handlers.
>  * Return: 0 on success and error code otherwise
>  */

Looks good to me.

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-23  7:19               ` Mike Rapoport
  0 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-23  7:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 19, 2018 at 11:15:15PM +0530, Souptick Joarder wrote:
> On Mon, Nov 19, 2018 at 9:56 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> >
> > On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> > > Hi Mike,
> > >
> > > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > >
> > > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > > > + * @vma: user vma to map to
> > > > > > > + * @addr: target user address of this page
> > > > > > > + * @pages: pointer to array of source kernel pages
> > > > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > > > + *
> > > > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > > > + * user vma.
> > > > > >
> > > > > > Please add the return value and context descriptions.
> > > > > >
> > > > >
> > > > > Sure I will wait for some time to get additional review comments and
> > > > > add all of those requested changes in v2.
> > > >
> > > > You could send your proposed wording now which might remove the need
> > > > for a v3 if we end up arguing about the wording.
> > >
> > > Does this description looks good ?
> > >
> > > /**
> > >  * vm_insert_range - insert range of kernel pages into user vma
> > >  * @vma: user vma to map to
> > >  * @addr: target user address of this page
> > >  * @pages: pointer to array of source kernel pages
> > >  * @page_count: number of pages need to insert into user vma
> > >  *
> > >  * This allows drivers to insert range of kernel pages they've allocated
> > >  * into a user vma. This is a generic function which drivers can use
> > >  * rather than using their own way of mapping range of kernel pages into
> > >  * user vma.
> > >  *
> > >  * Context - Process context. Called by mmap handlers.
> >
> > Context:
> >
> > >  * Return - int error value
> >
> > Return:
> >
> > >  * 0                    - OK
> > >  * -EINVAL              - Invalid argument
> > >  * -ENOMEM              - No memory
> > >  * -EFAULT              - Bad address
> > >  * -EBUSY               - Device or resource busy
> >
> > I don't think that elaborate description of error values is needed, just "0
> > on success and error code otherwise" would be sufficient.
> 
> /**
>  * vm_insert_range - insert range of kernel pages into user vma
>  * @vma: user vma to map to
>  * @addr: target user address of this page
>  * @pages: pointer to array of source kernel pages
>  * @page_count: number of pages need to insert into user vma
>  *
>  * This allows drivers to insert range of kernel pages they've allocated
>  * into a user vma. This is a generic function which drivers can use
>  * rather than using their own way of mapping range of kernel pages into
>  * user vma.
>  *
>  * Context: Process context. Called by mmap handlers.
>  * Return: 0 on success and error code otherwise
>  */

Looks good to me.

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-19 17:45             ` Souptick Joarder
                               ` (2 preceding siblings ...)
  (?)
@ 2018-11-23  7:19             ` Mike Rapoport
  -1 siblings, 0 replies; 73+ messages in thread
From: Mike Rapoport @ 2018-11-23  7:19 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Michal Hocko, Heiko Stuebner, Peter Zijlstra, dri-devel,
	linux-kernel, Linux-MM, linux1394-devel, Marek Szyprowski,
	Stephen Rothwell, oleksandr_andrushchenko, joro,
	Russell King - ARM Linux, Matthew Wilcox, airlied,
	linux-arm-kernel, linux-rockchip, treding, linux-media,
	Kees Cook, pawel, Rik van Riel, iommu, rppt, Boris Ostrovsky,
	mchehab, iamjoonsoo.kim, vbabka, Juergen Gross

On Mon, Nov 19, 2018 at 11:15:15PM +0530, Souptick Joarder wrote:
> On Mon, Nov 19, 2018 at 9:56 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> >
> > On Mon, Nov 19, 2018 at 08:43:09PM +0530, Souptick Joarder wrote:
> > > Hi Mike,
> > >
> > > On Sat, Nov 17, 2018 at 8:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > >
> > > > On Sat, Nov 17, 2018 at 12:26:38PM +0530, Souptick Joarder wrote:
> > > > > On Fri, Nov 16, 2018 at 11:59 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> > > > > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > > > > + * @vma: user vma to map to
> > > > > > > + * @addr: target user address of this page
> > > > > > > + * @pages: pointer to array of source kernel pages
> > > > > > > + * @page_count: no. of pages need to insert into user vma
> > > > > > > + *
> > > > > > > + * This allows drivers to insert range of kernel pages they've allocated
> > > > > > > + * into a user vma. This is a generic function which drivers can use
> > > > > > > + * rather than using their own way of mapping range of kernel pages into
> > > > > > > + * user vma.
> > > > > >
> > > > > > Please add the return value and context descriptions.
> > > > > >
> > > > >
> > > > > Sure I will wait for some time to get additional review comments and
> > > > > add all of those requested changes in v2.
> > > >
> > > > You could send your proposed wording now which might remove the need
> > > > for a v3 if we end up arguing about the wording.
> > >
> > > Does this description looks good ?
> > >
> > > /**
> > >  * vm_insert_range - insert range of kernel pages into user vma
> > >  * @vma: user vma to map to
> > >  * @addr: target user address of this page
> > >  * @pages: pointer to array of source kernel pages
> > >  * @page_count: number of pages need to insert into user vma
> > >  *
> > >  * This allows drivers to insert range of kernel pages they've allocated
> > >  * into a user vma. This is a generic function which drivers can use
> > >  * rather than using their own way of mapping range of kernel pages into
> > >  * user vma.
> > >  *
> > >  * Context - Process context. Called by mmap handlers.
> >
> > Context:
> >
> > >  * Return - int error value
> >
> > Return:
> >
> > >  * 0                    - OK
> > >  * -EINVAL              - Invalid argument
> > >  * -ENOMEM              - No memory
> > >  * -EFAULT              - Bad address
> > >  * -EBUSY               - Device or resource busy
> >
> > I don't think that elaborate description of error values is needed, just "0
> > on success and error code otherwise" would be sufficient.
> 
> /**
>  * vm_insert_range - insert range of kernel pages into user vma
>  * @vma: user vma to map to
>  * @addr: target user address of this page
>  * @pages: pointer to array of source kernel pages
>  * @page_count: number of pages need to insert into user vma
>  *
>  * This allows drivers to insert range of kernel pages they've allocated
>  * into a user vma. This is a generic function which drivers can use
>  * rather than using their own way of mapping range of kernel pages into
>  * user vma.
>  *
>  * Context: Process context. Called by mmap handlers.
>  * Return: 0 on success and error code otherwise
>  */

Looks good to me.

-- 
Sincerely yours,
Mike.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-15 15:45 ` Souptick Joarder
  (?)
@ 2018-11-28 15:21   ` Heiko Stübner
  -1 siblings, 0 replies; 73+ messages in thread
From: Heiko Stübner @ 2018-11-28 15:21 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: akpm, willy, mhocko, kirill.shutemov, vbabka, riel, sfr, rppt,
	peterz, linux, robin.murphy, iamjoonsoo.kim, treding, keescook,
	m.szyprowski, stefanr, hjc, airlied, oleksandr_andrushchenko,
	joro, pawel, kyungmin.park, mchehab, boris.ostrovsky, jgross,
	linux-kernel, linux-mm, linux-arm-kernel, linux1394-devel,
	dri-devel, linux-rockchip, xen-devel, iommu, linux-media

Am Donnerstag, 15. November 2018, 16:45:30 CET schrieb Souptick Joarder:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>

Except the missing EXPORT_SYMBOL for module builds this new
API is supposed to run also within the Rockchip drm driver, so
on rk3188, rk3288, rk3328 and rk3399 with graphics
Tested-by: Heiko Stuebner <heiko@sntech.de>




^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-28 15:21   ` Heiko Stübner
  0 siblings, 0 replies; 73+ messages in thread
From: Heiko Stübner @ 2018-11-28 15:21 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: mhocko, peterz, dri-devel, linux-kernel, linux-mm,
	linux1394-devel, m.szyprowski, sfr, oleksandr_andrushchenko,
	linux, willy, airlied, linux-arm-kernel, linux-rockchip, treding,
	linux-media, keescook, pawel, riel, iommu, rppt, boris.ostrovsky,
	mchehab, iamjoonsoo.kim, vbabka, jgross, xen-devel,
	kyungmin.park, stefanr, akpm, robin.murphy, kirill.shutemov

Am Donnerstag, 15. November 2018, 16:45:30 CET schrieb Souptick Joarder:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>

Except the missing EXPORT_SYMBOL for module builds this new
API is supposed to run also within the Rockchip drm driver, so
on rk3188, rk3288, rk3328 and rk3399 with graphics
Tested-by: Heiko Stuebner <heiko@sntech.de>



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-28 15:21   ` Heiko Stübner
  0 siblings, 0 replies; 73+ messages in thread
From: Heiko Stübner @ 2018-11-28 15:21 UTC (permalink / raw)
  To: linux-arm-kernel

Am Donnerstag, 15. November 2018, 16:45:30 CET schrieb Souptick Joarder:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>

Except the missing EXPORT_SYMBOL for module builds this new
API is supposed to run also within the Rockchip drm driver, so
on rk3188, rk3288, rk3328 and rk3399 with graphics
Tested-by: Heiko Stuebner <heiko@sntech.de>

^ permalink raw reply	[flat|nested] 73+ messages in thread

* Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
  2018-11-15 15:45 ` Souptick Joarder
                   ` (7 preceding siblings ...)
  (?)
@ 2018-11-28 15:21 ` Heiko Stübner
  -1 siblings, 0 replies; 73+ messages in thread
From: Heiko Stübner @ 2018-11-28 15:21 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: mhocko, peterz, dri-devel, linux-kernel, linux-mm,
	linux1394-devel, m.szyprowski, sfr, oleksandr_andrushchenko,
	joro, linux, willy, airlied, linux-arm-kernel, linux-rockchip,
	treding, linux-media, keescook, pawel, riel, iommu, rppt,
	boris.ostrovsky, mchehab, iamjoonsoo.kim, vbabka, jgross, hjc,
	xen-devel, kyungmin.park, stefanr, akpm, robin.murphy,
	kirill.shutemov

Am Donnerstag, 15. November 2018, 16:45:30 CET schrieb Souptick Joarder:
> Previouly drivers have their own way of mapping range of
> kernel pages/memory into user vma and this was done by
> invoking vm_insert_page() within a loop.
> 
> As this pattern is common across different drivers, it can
> be generalized by creating a new function and use it across
> the drivers.
> 
> vm_insert_range is the new API which will be used to map a
> range of kernel memory/pages to user vma.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>

Except the missing EXPORT_SYMBOL for module builds this new
API is supposed to run also within the Rockchip drm driver, so
on rk3188, rk3288, rk3328 and rk3399 with graphics
Tested-by: Heiko Stuebner <heiko@sntech.de>




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 73+ messages in thread

* [PATCH 1/9] mm: Introduce new vm_insert_range API
@ 2018-11-15 15:45 Souptick Joarder
  0 siblings, 0 replies; 73+ messages in thread
From: Souptick Joarder @ 2018-11-15 15:45 UTC (permalink / raw)
  To: akpm, willy, mhocko, kirill.shutemov, vbabka, riel, sfr, rppt,
	peterz, linux, robin.murphy, iamjoonsoo.kim, treding, keescook,
	m.szyprowski, stefanr, hjc, heiko, airlied,
	oleksandr_andrushchenko, joro, pawel, kyungmin.park, mchehab,
	boris.ostrovsky, jgross
  Cc: linux-rockchip, linux-kernel, dri-devel, xen-devel, linux-mm,
	iommu, linux1394-devel, linux-arm-kernel, linux-media

Previouly drivers have their own way of mapping range of
kernel pages/memory into user vma and this was done by
invoking vm_insert_page() within a loop.

As this pattern is common across different drivers, it can
be generalized by creating a new function and use it across
the drivers.

vm_insert_range is the new API which will be used to map a
range of kernel memory/pages to user vma.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Reviewed-by: Matthew Wilcox <willy@infradead.org>
---
 include/linux/mm_types.h |  3 +++
 mm/memory.c              | 28 ++++++++++++++++++++++++++++
 mm/nommu.c               |  7 +++++++
 3 files changed, 38 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5ed8f62..15ae24f 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm,
 extern void tlb_finish_mmu(struct mmu_gather *tlb,
 				unsigned long start, unsigned long end);
 
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count);
+
 static inline void init_tlb_flush_pending(struct mm_struct *mm)
 {
 	atomic_set(&mm->tlb_flush_pending, 0);
diff --git a/mm/memory.c b/mm/memory.c
index 15c417e..da904ed 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1478,6 +1478,34 @@ static int insert_page(struct vm_area_struct *vma, unsigned long addr,
 }
 
 /**
+ * vm_insert_range - insert range of kernel pages into user vma
+ * @vma: user vma to map to
+ * @addr: target user address of this page
+ * @pages: pointer to array of source kernel pages
+ * @page_count: no. of pages need to insert into user vma
+ *
+ * This allows drivers to insert range of kernel pages they've allocated
+ * into a user vma. This is a generic function which drivers can use
+ * rather than using their own way of mapping range of kernel pages into
+ * user vma.
+ */
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count)
+{
+	unsigned long uaddr = addr;
+	int ret = 0, i;
+
+	for (i = 0; i < page_count; i++) {
+		ret = vm_insert_page(vma, uaddr, pages[i]);
+		if (ret < 0)
+			return ret;
+		uaddr += PAGE_SIZE;
+	}
+
+	return ret;
+}
+
+/**
  * vm_insert_page - insert single page into user vma
  * @vma: user vma to map to
  * @addr: target user address of this page
diff --git a/mm/nommu.c b/mm/nommu.c
index 749276b..d6ef5c7 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, unsigned long addr,
 }
 EXPORT_SYMBOL(vm_insert_page);
 
+int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
+			struct page **pages, unsigned long page_count)
+{
+	return -EINVAL;
+}
+EXPORT_SYMBOL(vm_insert_range);
+
 /*
  *  sys_brk() for the most part doesn't need the global kernel
  *  lock, except when an application is doing something nasty
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 73+ messages in thread

end of thread, other threads:[~2018-11-28 15:22 UTC | newest]

Thread overview: 73+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-15 15:45 [PATCH 1/9] mm: Introduce new vm_insert_range API Souptick Joarder
2018-11-15 15:45 ` Souptick Joarder
2018-11-15 15:45 ` Souptick Joarder
2018-11-15 18:13 ` Randy Dunlap
2018-11-15 18:13   ` Randy Dunlap
2018-11-15 18:13   ` Randy Dunlap
2018-11-16  5:30   ` Souptick Joarder
2018-11-16  5:30   ` Souptick Joarder
2018-11-16  5:30     ` Souptick Joarder
2018-11-16  5:30     ` Souptick Joarder
2018-11-16  6:40     ` Matthew Wilcox
2018-11-16  6:40       ` Matthew Wilcox
2018-11-16  6:40       ` Matthew Wilcox
2018-11-16  8:15       ` Souptick Joarder
2018-11-16  8:15       ` Souptick Joarder
2018-11-16  8:15         ` Souptick Joarder
2018-11-16  8:15         ` Souptick Joarder
2018-11-16 16:59         ` Randy Dunlap
2018-11-16 16:59         ` Randy Dunlap
2018-11-16 16:59           ` Randy Dunlap
2018-11-16 16:59           ` Randy Dunlap
2018-11-16  6:40     ` Matthew Wilcox
2018-11-15 18:13 ` Randy Dunlap
2018-11-16 17:48 ` Slavomir Kaslev
2018-11-16 17:55   ` Slavomir Kaslev
2018-11-16 17:48   ` Slavomir Kaslev
2018-11-16 17:48 ` Slavomir Kaslev
2018-11-16 18:28 ` Mike Rapoport
2018-11-16 18:28 ` Mike Rapoport
2018-11-16 18:28   ` Mike Rapoport
2018-11-16 18:28   ` Mike Rapoport
2018-11-17  6:56   ` Souptick Joarder
2018-11-17  6:56   ` Souptick Joarder
2018-11-17  6:56     ` Souptick Joarder
2018-11-17  6:56     ` Souptick Joarder
2018-11-17 14:37     ` Matthew Wilcox
2018-11-17 14:37     ` Matthew Wilcox
2018-11-17 14:37       ` Matthew Wilcox
2018-11-17 14:37       ` Matthew Wilcox
2018-11-19 15:13       ` Souptick Joarder
2018-11-19 15:13       ` Souptick Joarder
2018-11-19 15:13         ` Souptick Joarder
2018-11-19 15:13         ` Souptick Joarder
2018-11-19 16:26         ` Mike Rapoport
2018-11-19 16:26           ` Mike Rapoport
2018-11-19 16:26           ` Mike Rapoport
2018-11-19 17:45           ` Souptick Joarder
2018-11-19 17:45           ` Souptick Joarder
2018-11-19 17:45             ` Souptick Joarder
2018-11-19 17:45             ` Souptick Joarder
2018-11-23  7:19             ` Mike Rapoport
2018-11-23  7:19               ` Mike Rapoport
2018-11-23  7:19               ` Mike Rapoport
2018-11-23  7:19             ` Mike Rapoport
2018-11-19 16:26         ` Mike Rapoport
2018-11-21 11:19         ` William Kucharski
2018-11-21 11:19         ` William Kucharski
2018-11-21 11:19           ` William Kucharski
2018-11-21 11:19           ` William Kucharski
2018-11-21 12:35           ` Matthew Wilcox
2018-11-21 12:35             ` Matthew Wilcox
2018-11-21 12:35             ` Matthew Wilcox
2018-11-21 14:29             ` William Kucharski
2018-11-21 14:29             ` William Kucharski
2018-11-21 14:29             ` William Kucharski
2018-11-21 14:29               ` William Kucharski
2018-11-21 14:29               ` William Kucharski
2018-11-21 12:35           ` Matthew Wilcox
2018-11-28 15:21 ` Heiko Stübner
2018-11-28 15:21 ` Heiko Stübner
2018-11-28 15:21   ` Heiko Stübner
2018-11-28 15:21   ` Heiko Stübner
  -- strict thread matches above, loose matches on Subject: below --
2018-11-15 15:45 Souptick Joarder

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.