* [PATCH 5/9] drm/xen/xen_drm_front_gem.c: Convert to use vm_insert_range
@ 2018-11-15 15:49 Souptick Joarder
2018-11-19 9:52 ` [Xen-devel] " Oleksandr Andrushchenko
0 siblings, 1 reply; 5+ messages in thread
From: Souptick Joarder @ 2018-11-15 15:49 UTC (permalink / raw)
To: akpm, willy, mhocko, oleksandr_andrushchenko, airlied
Cc: linux-kernel, linux-mm, dri-devel, xen-devel
Convert to use vm_insert_range() to map range of kernel
memory to user vma.
Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Reviewed-by: Matthew Wilcox <willy@infradead.org>
---
drivers/gpu/drm/xen/xen_drm_front_gem.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
index 47ff019..a3eade6 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -225,8 +225,7 @@ struct drm_gem_object *
static int gem_mmap_obj(struct xen_gem_object *xen_obj,
struct vm_area_struct *vma)
{
- unsigned long addr = vma->vm_start;
- int i;
+ int err;
/*
* clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
@@ -247,18 +246,11 @@ static int gem_mmap_obj(struct xen_gem_object *xen_obj,
* FIXME: as we insert all the pages now then no .fault handler must
* be called, so don't provide one
*/
- for (i = 0; i < xen_obj->num_pages; i++) {
- int ret;
-
- ret = vm_insert_page(vma, addr, xen_obj->pages[i]);
- if (ret < 0) {
- DRM_ERROR("Failed to insert pages into vma: %d\n", ret);
- return ret;
- }
-
- addr += PAGE_SIZE;
- }
- return 0;
+ err = vm_insert_range(vma, vma->vm_start, xen_obj->pages,
+ xen_obj->num_pages);
+ if (err < 0)
+ DRM_ERROR("Failed to insert pages into vma: %d\n", err);
+ return err;
}
int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma)
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Xen-devel] [PATCH 5/9] drm/xen/xen_drm_front_gem.c: Convert to use vm_insert_range
2018-11-15 15:49 [PATCH 5/9] drm/xen/xen_drm_front_gem.c: Convert to use vm_insert_range Souptick Joarder
@ 2018-11-19 9:52 ` Oleksandr Andrushchenko
2018-11-19 10:42 ` Souptick Joarder
0 siblings, 1 reply; 5+ messages in thread
From: Oleksandr Andrushchenko @ 2018-11-19 9:52 UTC (permalink / raw)
To: Souptick Joarder, akpm, willy, mhocko, oleksandr_andrushchenko, airlied
Cc: linux-mm, linux-kernel, dri-devel, xen-devel
On 11/15/18 5:49 PM, Souptick Joarder wrote:
> Convert to use vm_insert_range() to map range of kernel
> memory to user vma.
>
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> ---
> drivers/gpu/drm/xen/xen_drm_front_gem.c | 20 ++++++--------------
> 1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> index 47ff019..a3eade6 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> @@ -225,8 +225,7 @@ struct drm_gem_object *
> static int gem_mmap_obj(struct xen_gem_object *xen_obj,
> struct vm_area_struct *vma)
> {
> - unsigned long addr = vma->vm_start;
> - int i;
> + int err;
I would love to keep ret, not err
>
> /*
> * clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
> @@ -247,18 +246,11 @@ static int gem_mmap_obj(struct xen_gem_object *xen_obj,
> * FIXME: as we insert all the pages now then no .fault handler must
> * be called, so don't provide one
> */
> - for (i = 0; i < xen_obj->num_pages; i++) {
> - int ret;
> -
> - ret = vm_insert_page(vma, addr, xen_obj->pages[i]);
> - if (ret < 0) {
> - DRM_ERROR("Failed to insert pages into vma: %d\n", ret);
> - return ret;
> - }
> -
> - addr += PAGE_SIZE;
> - }
> - return 0;
> + err = vm_insert_range(vma, vma->vm_start, xen_obj->pages,
> + xen_obj->num_pages);
> + if (err < 0)
> + DRM_ERROR("Failed to insert pages into vma: %d\n", err);
> + return err;
> }
>
> int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma)
With the above fixed,
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Xen-devel] [PATCH 5/9] drm/xen/xen_drm_front_gem.c: Convert to use vm_insert_range
2018-11-19 9:52 ` [Xen-devel] " Oleksandr Andrushchenko
@ 2018-11-19 10:42 ` Souptick Joarder
2018-11-19 11:02 ` Oleksandr Andrushchenko
0 siblings, 1 reply; 5+ messages in thread
From: Souptick Joarder @ 2018-11-19 10:42 UTC (permalink / raw)
To: andr2000
Cc: Andrew Morton, Matthew Wilcox, Michal Hocko,
oleksandr_andrushchenko, airlied, Linux-MM, linux-kernel,
dri-devel, xen-devel
On Mon, Nov 19, 2018 at 3:22 PM Oleksandr Andrushchenko
<andr2000@gmail.com> wrote:
>
> On 11/15/18 5:49 PM, Souptick Joarder wrote:
> > Convert to use vm_insert_range() to map range of kernel
> > memory to user vma.
> >
> > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> > Reviewed-by: Matthew Wilcox <willy@infradead.org>
> > ---
> > drivers/gpu/drm/xen/xen_drm_front_gem.c | 20 ++++++--------------
> > 1 file changed, 6 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> > index 47ff019..a3eade6 100644
> > --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
> > +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> > @@ -225,8 +225,7 @@ struct drm_gem_object *
> > static int gem_mmap_obj(struct xen_gem_object *xen_obj,
> > struct vm_area_struct *vma)
> > {
> > - unsigned long addr = vma->vm_start;
> > - int i;
> > + int err;
> I would love to keep ret, not err
Sure, will add it in v2.
But I think, err is more appropriate here.
> >
> > /*
> > * clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
> > @@ -247,18 +246,11 @@ static int gem_mmap_obj(struct xen_gem_object *xen_obj,
> > * FIXME: as we insert all the pages now then no .fault handler must
> > * be called, so don't provide one
> > */
> > - for (i = 0; i < xen_obj->num_pages; i++) {
> > - int ret;
> > -
> > - ret = vm_insert_page(vma, addr, xen_obj->pages[i]);
> > - if (ret < 0) {
> > - DRM_ERROR("Failed to insert pages into vma: %d\n", ret);
> > - return ret;
> > - }
> > -
> > - addr += PAGE_SIZE;
> > - }
> > - return 0;
> > + err = vm_insert_range(vma, vma->vm_start, xen_obj->pages,
> > + xen_obj->num_pages);
> > + if (err < 0)
> > + DRM_ERROR("Failed to insert pages into vma: %d\n", err);
> > + return err;
> > }
> >
> > int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>
> With the above fixed,
>
> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Xen-devel] [PATCH 5/9] drm/xen/xen_drm_front_gem.c: Convert to use vm_insert_range
2018-11-19 10:42 ` Souptick Joarder
@ 2018-11-19 11:02 ` Oleksandr Andrushchenko
2018-11-19 12:29 ` Matthew Wilcox
0 siblings, 1 reply; 5+ messages in thread
From: Oleksandr Andrushchenko @ 2018-11-19 11:02 UTC (permalink / raw)
To: Souptick Joarder
Cc: Andrew Morton, Matthew Wilcox, Michal Hocko,
oleksandr_andrushchenko, airlied, Linux-MM, linux-kernel,
dri-devel, xen-devel
On 11/19/18 12:42 PM, Souptick Joarder wrote:
> On Mon, Nov 19, 2018 at 3:22 PM Oleksandr Andrushchenko
> <andr2000@gmail.com> wrote:
>> On 11/15/18 5:49 PM, Souptick Joarder wrote:
>>> Convert to use vm_insert_range() to map range of kernel
>>> memory to user vma.
>>>
>>> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
>>> Reviewed-by: Matthew Wilcox <willy@infradead.org>
>>> ---
>>> drivers/gpu/drm/xen/xen_drm_front_gem.c | 20 ++++++--------------
>>> 1 file changed, 6 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>>> index 47ff019..a3eade6 100644
>>> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
>>> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
>>> @@ -225,8 +225,7 @@ struct drm_gem_object *
>>> static int gem_mmap_obj(struct xen_gem_object *xen_obj,
>>> struct vm_area_struct *vma)
>>> {
>>> - unsigned long addr = vma->vm_start;
>>> - int i;
>>> + int err;
>> I would love to keep ret, not err
> Sure, will add it in v2.
> But I think, err is more appropriate here.
I used "ret" throughout the driver, so this is just to remain consistent:
grep -rnw err drivers/gpu/drm/xen/ | wc -l
0
grep -rnw ret drivers/gpu/drm/xen/ | wc -l
204
>>> /*
>>> * clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
>>> @@ -247,18 +246,11 @@ static int gem_mmap_obj(struct xen_gem_object *xen_obj,
>>> * FIXME: as we insert all the pages now then no .fault handler must
>>> * be called, so don't provide one
>>> */
>>> - for (i = 0; i < xen_obj->num_pages; i++) {
>>> - int ret;
>>> -
>>> - ret = vm_insert_page(vma, addr, xen_obj->pages[i]);
>>> - if (ret < 0) {
>>> - DRM_ERROR("Failed to insert pages into vma: %d\n", ret);
>>> - return ret;
>>> - }
>>> -
>>> - addr += PAGE_SIZE;
>>> - }
>>> - return 0;
>>> + err = vm_insert_range(vma, vma->vm_start, xen_obj->pages,
>>> + xen_obj->num_pages);
>>> + if (err < 0)
>>> + DRM_ERROR("Failed to insert pages into vma: %d\n", err);
>>> + return err;
>>> }
>>>
>>> int xen_drm_front_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> With the above fixed,
>>
>> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Xen-devel] [PATCH 5/9] drm/xen/xen_drm_front_gem.c: Convert to use vm_insert_range
2018-11-19 11:02 ` Oleksandr Andrushchenko
@ 2018-11-19 12:29 ` Matthew Wilcox
0 siblings, 0 replies; 5+ messages in thread
From: Matthew Wilcox @ 2018-11-19 12:29 UTC (permalink / raw)
To: Oleksandr Andrushchenko
Cc: Souptick Joarder, Andrew Morton, Michal Hocko,
oleksandr_andrushchenko, airlied, Linux-MM, linux-kernel,
dri-devel, xen-devel
On Mon, Nov 19, 2018 at 01:02:46PM +0200, Oleksandr Andrushchenko wrote:
> On 11/19/18 12:42 PM, Souptick Joarder wrote:
> > On Mon, Nov 19, 2018 at 3:22 PM Oleksandr Andrushchenko
> > <andr2000@gmail.com> wrote:
> > > > - unsigned long addr = vma->vm_start;
> > > > - int i;
> > > > + int err;
> > > I would love to keep ret, not err
> > Sure, will add it in v2.
> > But I think, err is more appropriate here.
>
> I used "ret" throughout the driver, so this is just to remain consistent:
>
> grep -rnw err drivers/gpu/drm/xen/ | wc -l
> 0
> grep -rnw ret drivers/gpu/drm/xen/ | wc -l
> 204
It's your driver, so that's fine. The reason we chose 'err' over 'ret'
is that there's a history of errno vs VM_FAULT_xxx code confusion in
this area. Naming a variable 'err' makes it clear this is an errno and
not a vm_fault_t.
> > > With the above fixed,
> > >
> > > Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-19 12:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-15 15:49 [PATCH 5/9] drm/xen/xen_drm_front_gem.c: Convert to use vm_insert_range Souptick Joarder
2018-11-19 9:52 ` [Xen-devel] " Oleksandr Andrushchenko
2018-11-19 10:42 ` Souptick Joarder
2018-11-19 11:02 ` Oleksandr Andrushchenko
2018-11-19 12:29 ` Matthew Wilcox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).