| Message ID | 20190111151235.GA2836@jordon-HP-15-Notebook-PC (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use vm_insert_range and vm_insert_range_buggy | expand |
On 1/11/19 10:12 AM, 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: Boris Ostrovsky <boris.ostrovsky@oracle.com> (although it would be good to mention in the commit that you are also replacing count with vma_pages(vma), and why) > --- > drivers/xen/gntdev.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c > index b0b02a5..ca4acee 100644 > --- a/drivers/xen/gntdev.c > +++ b/drivers/xen/gntdev.c > @@ -1082,18 +1082,17 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) > { > struct gntdev_priv *priv = flip->private_data; > int index = vma->vm_pgoff; > - int count = vma_pages(vma); > struct gntdev_grant_map *map; > - int i, err = -EINVAL; > + int err = -EINVAL; > > if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags & VM_SHARED)) > return -EINVAL; > > pr_debug("map %d+%d at %lx (pgoff %lx)\n", > - index, count, vma->vm_start, vma->vm_pgoff); > + index, vma_pages(vma), vma->vm_start, vma->vm_pgoff); > > mutex_lock(&priv->lock); > - map = gntdev_find_map_index(priv, index, count); > + map = gntdev_find_map_index(priv, index, vma_pages(vma)); > if (!map) > goto unlock_out; > if (use_ptemod && map->vma) > @@ -1145,12 +1144,9 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) > goto out_put_map; > > if (!use_ptemod) { > - for (i = 0; i < count; i++) { > - err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE, > - map->pages[i]); > - if (err) > - goto out_put_map; > - } > + err = vm_insert_range(vma, map->pages, map->count); > + if (err) > + goto out_put_map; > } else { > #ifdef CONFIG_X86 > /*
On Tue, Jan 15, 2019 at 4:58 AM Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote: > > On 1/11/19 10:12 AM, 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: Boris Ostrovsky <boris.ostrovsky@oracle.com> > > (although it would be good to mention in the commit that you are also > replacing count with vma_pages(vma), and why) The original code was using count ( *count = vma_pages(vma)* ) which is same as this patch. Do I need capture it change log ? > > > > --- > > drivers/xen/gntdev.c | 16 ++++++---------- > > 1 file changed, 6 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c > > index b0b02a5..ca4acee 100644 > > --- a/drivers/xen/gntdev.c > > +++ b/drivers/xen/gntdev.c > > @@ -1082,18 +1082,17 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) > > { > > struct gntdev_priv *priv = flip->private_data; > > int index = vma->vm_pgoff; > > - int count = vma_pages(vma); > > struct gntdev_grant_map *map; > > - int i, err = -EINVAL; > > + int err = -EINVAL; > > > > if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags & VM_SHARED)) > > return -EINVAL; > > > > pr_debug("map %d+%d at %lx (pgoff %lx)\n", > > - index, count, vma->vm_start, vma->vm_pgoff); > > + index, vma_pages(vma), vma->vm_start, vma->vm_pgoff); > > > > mutex_lock(&priv->lock); > > - map = gntdev_find_map_index(priv, index, count); > > + map = gntdev_find_map_index(priv, index, vma_pages(vma)); > > if (!map) > > goto unlock_out; > > if (use_ptemod && map->vma) > > @@ -1145,12 +1144,9 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) > > goto out_put_map; > > > > if (!use_ptemod) { > > - for (i = 0; i < count; i++) { > > - err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE, > > - map->pages[i]); > > - if (err) > > - goto out_put_map; > > - } > > + err = vm_insert_range(vma, map->pages, map->count); > > + if (err) > > + goto out_put_map; > > } else { > > #ifdef CONFIG_X86 > > /* >
On 1/14/19 11:49 PM, Souptick Joarder wrote: > On Tue, Jan 15, 2019 at 4:58 AM Boris Ostrovsky > <boris.ostrovsky@oracle.com> wrote: >> On 1/11/19 10:12 AM, 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: Boris Ostrovsky <boris.ostrovsky@oracle.com> >> >> (although it would be good to mention in the commit that you are also >> replacing count with vma_pages(vma), and why) > The original code was using count ( *count = vma_pages(vma)* ) > which is same as this patch. Do I need capture it change log ? I'd just say that because theoretically count might not be equal to map->count we should use the latter as input to vm_insert_range(). Thanks. -boris > >> >>> --- >>> drivers/xen/gntdev.c | 16 ++++++---------- >>> 1 file changed, 6 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c >>> index b0b02a5..ca4acee 100644 >>> --- a/drivers/xen/gntdev.c >>> +++ b/drivers/xen/gntdev.c >>> @@ -1082,18 +1082,17 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) >>> { >>> struct gntdev_priv *priv = flip->private_data; >>> int index = vma->vm_pgoff; >>> - int count = vma_pages(vma); >>> struct gntdev_grant_map *map; >>> - int i, err = -EINVAL; >>> + int err = -EINVAL; >>> >>> if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags & VM_SHARED)) >>> return -EINVAL; >>> >>> pr_debug("map %d+%d at %lx (pgoff %lx)\n", >>> - index, count, vma->vm_start, vma->vm_pgoff); >>> + index, vma_pages(vma), vma->vm_start, vma->vm_pgoff); >>> >>> mutex_lock(&priv->lock); >>> - map = gntdev_find_map_index(priv, index, count); >>> + map = gntdev_find_map_index(priv, index, vma_pages(vma)); >>> if (!map) >>> goto unlock_out; >>> if (use_ptemod && map->vma) >>> @@ -1145,12 +1144,9 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) >>> goto out_put_map; >>> >>> if (!use_ptemod) { >>> - for (i = 0; i < count; i++) { >>> - err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE, >>> - map->pages[i]); >>> - if (err) >>> - goto out_put_map; >>> - } >>> + err = vm_insert_range(vma, map->pages, map->count); >>> + if (err) >>> + goto out_put_map; >>> } else { >>> #ifdef CONFIG_X86 >>> /*
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index b0b02a5..ca4acee 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -1082,18 +1082,17 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) { struct gntdev_priv *priv = flip->private_data; int index = vma->vm_pgoff; - int count = vma_pages(vma); struct gntdev_grant_map *map; - int i, err = -EINVAL; + int err = -EINVAL; if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags & VM_SHARED)) return -EINVAL; pr_debug("map %d+%d at %lx (pgoff %lx)\n", - index, count, vma->vm_start, vma->vm_pgoff); + index, vma_pages(vma), vma->vm_start, vma->vm_pgoff); mutex_lock(&priv->lock); - map = gntdev_find_map_index(priv, index, count); + map = gntdev_find_map_index(priv, index, vma_pages(vma)); if (!map) goto unlock_out; if (use_ptemod && map->vma) @@ -1145,12 +1144,9 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) goto out_put_map; if (!use_ptemod) { - for (i = 0; i < count; i++) { - err = vm_insert_page(vma, vma->vm_start + i*PAGE_SIZE, - map->pages[i]); - if (err) - goto out_put_map; - } + err = vm_insert_range(vma, map->pages, map->count); + if (err) + goto out_put_map; } else { #ifdef CONFIG_X86 /*
Convert to use vm_insert_range() to map range of kernel memory to user vma. Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> --- drivers/xen/gntdev.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)