diff mbox series

[v3,8/9] xen/gntdev.c: Convert to use vm_map_pages()

Message ID 20190213140728.GA22080@jordon-HP-15-Notebook-PC (mailing list archive)
State New, archived
Headers show
Series mm: Use vm_map_pages() and vm_map_pages_zero() API | expand

Commit Message

Souptick Joarder Feb. 13, 2019, 2:07 p.m. UTC 
Convert to use vm_map_pages() to map range of kernel
memory to user vma.

map->count is passed to vm_map_pages() and internal API
verify map->count against count ( count = vma_pages(vma))
for page array boundary overrun. With this count is not
needed inside gntdev_mmap() and it could be replaced with
vma_pages(vma).

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 drivers/xen/gntdev.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

Comments

kernel test robot Feb. 14, 2019, 3:59 p.m. UTC | #1 
Hi Souptick,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.0-rc4 next-20190214]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Souptick-Joarder/mm-Use-vm_map_pages-and-vm_map_pages_zero-API/20190214-213457
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/xen/gntdev.c: In function 'gntdev_mmap':
>> drivers/xen/gntdev.c:23:21: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=]
    #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
                        ^~~~~~
   include/linux/dynamic_debug.h:127:35: note: in expansion of macro 'pr_fmt'
      __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
                                      ^~~~~~
   include/linux/printk.h:335:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers/xen/gntdev.c:1091:2: note: in expansion of macro 'pr_debug'
     pr_debug("map %d+%d at %lx (pgoff %lx)\n",
     ^~~~~~~~

vim +23 drivers/xen/gntdev.c

ab31523c Gerd Hoffmann 2010-12-14  22  
283c0972 Joe Perches   2013-06-28 @23  #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
283c0972 Joe Perches   2013-06-28  24  

:::::: The code at line 23 was first introduced by commit
:::::: 283c0972d53769ee44750cad4c27e3f5fa26ec1f xen: Convert printks to pr_<level>

:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Feb. 14, 2019, 4:55 p.m. UTC | #2 
Hi Souptick,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.0-rc4 next-20190214]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Souptick-Joarder/mm-Use-vm_map_pages-and-vm_map_pages_zero-API/20190214-213457
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/xen/gntdev.c: In function 'gntdev_mmap':
   drivers/xen/gntdev.c:23:21: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=]
    #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
                        ^
>> include/linux/dynamic_debug.h:127:35: note: in expansion of macro 'pr_fmt'
      __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
                                      ^~~~~~
   include/linux/printk.h:335:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
>> drivers/xen/gntdev.c:1091:2: note: in expansion of macro 'pr_debug'
     pr_debug("map %d+%d at %lx (pgoff %lx)\n",
     ^~~~~~~~
--
   drivers//xen/gntdev.c: In function 'gntdev_mmap':
   drivers//xen/gntdev.c:23:21: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=]
    #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
                        ^
>> include/linux/dynamic_debug.h:127:35: note: in expansion of macro 'pr_fmt'
      __dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
                                      ^~~~~~
   include/linux/printk.h:335:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers//xen/gntdev.c:1091:2: note: in expansion of macro 'pr_debug'
     pr_debug("map %d+%d at %lx (pgoff %lx)\n",
     ^~~~~~~~

vim +/pr_debug +1091 drivers/xen/gntdev.c

ab31523c2 Gerd Hoffmann           2010-12-14  1080  
ab31523c2 Gerd Hoffmann           2010-12-14  1081  static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma)
ab31523c2 Gerd Hoffmann           2010-12-14  1082  {
ab31523c2 Gerd Hoffmann           2010-12-14  1083  	struct gntdev_priv *priv = flip->private_data;
ab31523c2 Gerd Hoffmann           2010-12-14  1084  	int index = vma->vm_pgoff;
1d3145675 Oleksandr Andrushchenko 2018-07-20  1085  	struct gntdev_grant_map *map;
29222b665 Souptick Joarder        2019-02-13  1086  	int err = -EINVAL;
ab31523c2 Gerd Hoffmann           2010-12-14  1087  
ab31523c2 Gerd Hoffmann           2010-12-14  1088  	if ((vma->vm_flags & VM_WRITE) && !(vma->vm_flags & VM_SHARED))
ab31523c2 Gerd Hoffmann           2010-12-14  1089  		return -EINVAL;
ab31523c2 Gerd Hoffmann           2010-12-14  1090  
ab31523c2 Gerd Hoffmann           2010-12-14 @1091  	pr_debug("map %d+%d at %lx (pgoff %lx)\n",
29222b665 Souptick Joarder        2019-02-13  1092  			index, vma_pages(vma), vma->vm_start, vma->vm_pgoff);
ab31523c2 Gerd Hoffmann           2010-12-14  1093  
1401c00e5 David Vrabel            2015-01-09  1094  	mutex_lock(&priv->lock);
29222b665 Souptick Joarder        2019-02-13  1095  	map = gntdev_find_map_index(priv, index, vma_pages(vma));
ab31523c2 Gerd Hoffmann           2010-12-14  1096  	if (!map)
ab31523c2 Gerd Hoffmann           2010-12-14  1097  		goto unlock_out;
aab8f11a6 Daniel De Graaf         2011-02-03  1098  	if (use_ptemod && map->vma)
ab31523c2 Gerd Hoffmann           2010-12-14  1099  		goto unlock_out;
aab8f11a6 Daniel De Graaf         2011-02-03  1100  	if (use_ptemod && priv->mm != vma->vm_mm) {
283c0972d Joe Perches             2013-06-28  1101  		pr_warn("Huh? Other mm?\n");
ab31523c2 Gerd Hoffmann           2010-12-14  1102  		goto unlock_out;
ab31523c2 Gerd Hoffmann           2010-12-14  1103  	}
ab31523c2 Gerd Hoffmann           2010-12-14  1104  
c5f7c5a9a Elena Reshetova         2017-03-06  1105  	refcount_inc(&map->users);
68b025c81 Daniel De Graaf         2011-02-03  1106  
ab31523c2 Gerd Hoffmann           2010-12-14  1107  	vma->vm_ops = &gntdev_vmops;
ab31523c2 Gerd Hoffmann           2010-12-14  1108  
30faaafdf Boris Ostrovsky         2016-11-21  1109  	vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP | VM_MIXEDMAP;
d79647aea Daniel De Graaf         2011-03-07  1110  
d79647aea Daniel De Graaf         2011-03-07  1111  	if (use_ptemod)
e8e937be9 Stefano Stabellini      2012-04-03  1112  		vma->vm_flags |= VM_DONTCOPY;
ab31523c2 Gerd Hoffmann           2010-12-14  1113  
ab31523c2 Gerd Hoffmann           2010-12-14  1114  	vma->vm_private_data = map;
aab8f11a6 Daniel De Graaf         2011-02-03  1115  
aab8f11a6 Daniel De Graaf         2011-02-03  1116  	if (use_ptemod)
ab31523c2 Gerd Hoffmann           2010-12-14  1117  		map->vma = vma;
ab31523c2 Gerd Hoffmann           2010-12-14  1118  
12996fc38 Daniel De Graaf         2011-02-09  1119  	if (map->flags) {
12996fc38 Daniel De Graaf         2011-02-09  1120  		if ((vma->vm_flags & VM_WRITE) &&
12996fc38 Daniel De Graaf         2011-02-09  1121  				(map->flags & GNTMAP_readonly))
a93e20a83 Dan Carpenter           2011-03-19  1122  			goto out_unlock_put;
12996fc38 Daniel De Graaf         2011-02-09  1123  	} else {
aab8f11a6 Daniel De Graaf         2011-02-03  1124  		map->flags = GNTMAP_host_map;
ab31523c2 Gerd Hoffmann           2010-12-14  1125  		if (!(vma->vm_flags & VM_WRITE))
ab31523c2 Gerd Hoffmann           2010-12-14  1126  			map->flags |= GNTMAP_readonly;
12996fc38 Daniel De Graaf         2011-02-09  1127  	}
ab31523c2 Gerd Hoffmann           2010-12-14  1128  
1401c00e5 David Vrabel            2015-01-09  1129  	mutex_unlock(&priv->lock);
f0a70c882 Daniel De Graaf         2011-01-07  1130  
aab8f11a6 Daniel De Graaf         2011-02-03  1131  	if (use_ptemod) {
298d275d4 Juergen Gross           2017-10-25  1132  		map->pages_vm_start = vma->vm_start;
ab31523c2 Gerd Hoffmann           2010-12-14  1133  		err = apply_to_page_range(vma->vm_mm, vma->vm_start,
ab31523c2 Gerd Hoffmann           2010-12-14  1134  					  vma->vm_end - vma->vm_start,
ab31523c2 Gerd Hoffmann           2010-12-14  1135  					  find_grant_ptes, map);
ab31523c2 Gerd Hoffmann           2010-12-14  1136  		if (err) {
283c0972d Joe Perches             2013-06-28  1137  			pr_warn("find_grant_ptes() failure.\n");
90b6f3054 Daniel De Graaf         2011-02-03  1138  			goto out_put_map;
ab31523c2 Gerd Hoffmann           2010-12-14  1139  		}
aab8f11a6 Daniel De Graaf         2011-02-03  1140  	}
ab31523c2 Gerd Hoffmann           2010-12-14  1141  
1d3145675 Oleksandr Andrushchenko 2018-07-20  1142  	err = gntdev_map_grant_pages(map);
90b6f3054 Daniel De Graaf         2011-02-03  1143  	if (err)
90b6f3054 Daniel De Graaf         2011-02-03  1144  		goto out_put_map;
f0a70c882 Daniel De Graaf         2011-01-07  1145  
aab8f11a6 Daniel De Graaf         2011-02-03  1146  	if (!use_ptemod) {
29222b665 Souptick Joarder        2019-02-13  1147  		err = vm_map_pages(vma, map->pages, map->count);
aab8f11a6 Daniel De Graaf         2011-02-03  1148  		if (err)
90b6f3054 Daniel De Graaf         2011-02-03  1149  			goto out_put_map;
923b2919e David Vrabel            2014-12-18  1150  	} else {
923b2919e David Vrabel            2014-12-18  1151  #ifdef CONFIG_X86
923b2919e David Vrabel            2014-12-18  1152  		/*
923b2919e David Vrabel            2014-12-18  1153  		 * If the PTEs were not made special by the grant map
923b2919e David Vrabel            2014-12-18  1154  		 * hypercall, do so here.
923b2919e David Vrabel            2014-12-18  1155  		 *
923b2919e David Vrabel            2014-12-18  1156  		 * This is racy since the mapping is already visible
923b2919e David Vrabel            2014-12-18  1157  		 * to userspace but userspace should be well-behaved
923b2919e David Vrabel            2014-12-18  1158  		 * enough to not touch it until the mmap() call
923b2919e David Vrabel            2014-12-18  1159  		 * returns.
923b2919e David Vrabel            2014-12-18  1160  		 */
923b2919e David Vrabel            2014-12-18  1161  		if (!xen_feature(XENFEAT_gnttab_map_avail_bits)) {
923b2919e David Vrabel            2014-12-18  1162  			apply_to_page_range(vma->vm_mm, vma->vm_start,
923b2919e David Vrabel            2014-12-18  1163  					    vma->vm_end - vma->vm_start,
923b2919e David Vrabel            2014-12-18  1164  					    set_grant_ptes_as_special, NULL);
923b2919e David Vrabel            2014-12-18  1165  		}
923b2919e David Vrabel            2014-12-18  1166  #endif
aab8f11a6 Daniel De Graaf         2011-02-03  1167  	}
aab8f11a6 Daniel De Graaf         2011-02-03  1168  
f0a70c882 Daniel De Graaf         2011-01-07  1169  	return 0;
f0a70c882 Daniel De Graaf         2011-01-07  1170  
ab31523c2 Gerd Hoffmann           2010-12-14  1171  unlock_out:
1401c00e5 David Vrabel            2015-01-09  1172  	mutex_unlock(&priv->lock);
ab31523c2 Gerd Hoffmann           2010-12-14  1173  	return err;
90b6f3054 Daniel De Graaf         2011-02-03  1174  
a93e20a83 Dan Carpenter           2011-03-19  1175  out_unlock_put:
1401c00e5 David Vrabel            2015-01-09  1176  	mutex_unlock(&priv->lock);
90b6f3054 Daniel De Graaf         2011-02-03  1177  out_put_map:
cf2acf66a Ross Lagerwall          2018-01-09  1178  	if (use_ptemod) {
84e4075d6 Daniel De Graaf         2011-02-09  1179  		map->vma = NULL;
cf2acf66a Ross Lagerwall          2018-01-09  1180  		unmap_grant_pages(map, 0, map->count);
cf2acf66a Ross Lagerwall          2018-01-09  1181  	}
16a1d0225 Daniel De Graaf         2013-01-02  1182  	gntdev_put_map(priv, map);
90b6f3054 Daniel De Graaf         2011-02-03  1183  	return err;
ab31523c2 Gerd Hoffmann           2010-12-14  1184  }
ab31523c2 Gerd Hoffmann           2010-12-14  1185  

:::::: The code at line 1091 was first introduced by commit
:::::: ab31523c2fcac557226bac72cbdf5fafe01f9a26 xen/gntdev: allow usermode to map granted pages

:::::: TO: Gerd Hoffmann <kraxel@redhat.com>
:::::: CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 5efc5ee..7f65ba3 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_map_pages(vma, map->pages, map->count);
+		if (err)
+			goto out_put_map;
 	} else {
 #ifdef CONFIG_X86
 		/*