Message ID | 1445533889-7661-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Daniel,
[auto build test ERROR on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-Update-GEM-refcounting-docs/20151023-011317
config: i386-randconfig-s1-201542 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_gem_dumb_map':
>> drivers/gpu/drm/vgem/vgem_drv.c:211:7: error: implicit declaration of function 'drm_vma_node_has_offset' [-Werror=implicit-function-declaration]
if (!drm_vma_node_has_offset(&obj->vma_node)) {
^
cc1: some warnings being treated as errors
vim +/drm_vma_node_has_offset +211 drivers/gpu/drm/vgem/vgem_drv.c
502e95c6 Zach Reizner 2015-03-04 205 obj = drm_gem_object_lookup(dev, file, handle);
502e95c6 Zach Reizner 2015-03-04 206 if (!obj) {
502e95c6 Zach Reizner 2015-03-04 207 ret = -ENOENT;
502e95c6 Zach Reizner 2015-03-04 208 goto unlock;
502e95c6 Zach Reizner 2015-03-04 209 }
502e95c6 Zach Reizner 2015-03-04 210
502e95c6 Zach Reizner 2015-03-04 @211 if (!drm_vma_node_has_offset(&obj->vma_node)) {
502e95c6 Zach Reizner 2015-03-04 212 ret = drm_gem_create_mmap_offset(obj);
502e95c6 Zach Reizner 2015-03-04 213 if (ret)
502e95c6 Zach Reizner 2015-03-04 214 goto unref;
:::::: The code at line 211 was first introduced by commit
:::::: 502e95c6678505474f1056480310cd9382bacbac drm/vgem: implement virtual GEM
:::::: TO: Zach Reizner <zachr@google.com>
:::::: CC: Dave Airlie <airlied@redhat.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Daniel,
[auto build test WARNING on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-Update-GEM-refcounting-docs/20151023-011317
config: x86_64-randconfig-s1-10230205 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/list.h:4,
from include/linux/module.h:9,
from drivers/gpu/drm/vgem/vgem_drv.c:33:
drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_gem_dumb_map':
drivers/gpu/drm/vgem/vgem_drv.c:211:7: error: implicit declaration of function 'drm_vma_node_has_offset' [-Werror=implicit-function-declaration]
if (!drm_vma_node_has_offset(&obj->vma_node)) {
^
include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
if (__builtin_constant_p((cond)) ? !!(cond) : \
^
>> drivers/gpu/drm/vgem/vgem_drv.c:211:2: note: in expansion of macro 'if'
if (!drm_vma_node_has_offset(&obj->vma_node)) {
^
cc1: some warnings being treated as errors
vim +/if +211 drivers/gpu/drm/vgem/vgem_drv.c
502e95c6 Zach Reizner 2015-03-04 27
502e95c6 Zach Reizner 2015-03-04 28 /**
502e95c6 Zach Reizner 2015-03-04 29 * This is vgem, a (non-hardware-backed) GEM service. This is used by Mesa's
502e95c6 Zach Reizner 2015-03-04 30 * software renderer and the X server for efficient buffer sharing.
502e95c6 Zach Reizner 2015-03-04 31 */
502e95c6 Zach Reizner 2015-03-04 32
502e95c6 Zach Reizner 2015-03-04 @33 #include <linux/module.h>
502e95c6 Zach Reizner 2015-03-04 34 #include <linux/ramfs.h>
502e95c6 Zach Reizner 2015-03-04 35 #include <linux/shmem_fs.h>
502e95c6 Zach Reizner 2015-03-04 36 #include <linux/dma-buf.h>
502e95c6 Zach Reizner 2015-03-04 37 #include "vgem_drv.h"
502e95c6 Zach Reizner 2015-03-04 38
502e95c6 Zach Reizner 2015-03-04 39 #define DRIVER_NAME "vgem"
502e95c6 Zach Reizner 2015-03-04 40 #define DRIVER_DESC "Virtual GEM provider"
502e95c6 Zach Reizner 2015-03-04 41 #define DRIVER_DATE "20120112"
502e95c6 Zach Reizner 2015-03-04 42 #define DRIVER_MAJOR 1
502e95c6 Zach Reizner 2015-03-04 43 #define DRIVER_MINOR 0
502e95c6 Zach Reizner 2015-03-04 44
502e95c6 Zach Reizner 2015-03-04 45 void vgem_gem_put_pages(struct drm_vgem_gem_object *obj)
502e95c6 Zach Reizner 2015-03-04 46 {
502e95c6 Zach Reizner 2015-03-04 47 drm_gem_put_pages(&obj->base, obj->pages, false, false);
502e95c6 Zach Reizner 2015-03-04 48 obj->pages = NULL;
502e95c6 Zach Reizner 2015-03-04 49 }
502e95c6 Zach Reizner 2015-03-04 50
502e95c6 Zach Reizner 2015-03-04 51 static void vgem_gem_free_object(struct drm_gem_object *obj)
502e95c6 Zach Reizner 2015-03-04 52 {
502e95c6 Zach Reizner 2015-03-04 53 struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
502e95c6 Zach Reizner 2015-03-04 54
502e95c6 Zach Reizner 2015-03-04 55 drm_gem_free_mmap_offset(obj);
502e95c6 Zach Reizner 2015-03-04 56
502e95c6 Zach Reizner 2015-03-04 57 if (vgem_obj->use_dma_buf && obj->dma_buf) {
502e95c6 Zach Reizner 2015-03-04 58 dma_buf_put(obj->dma_buf);
502e95c6 Zach Reizner 2015-03-04 59 obj->dma_buf = NULL;
502e95c6 Zach Reizner 2015-03-04 60 }
502e95c6 Zach Reizner 2015-03-04 61
502e95c6 Zach Reizner 2015-03-04 62 drm_gem_object_release(obj);
502e95c6 Zach Reizner 2015-03-04 63
502e95c6 Zach Reizner 2015-03-04 64 if (vgem_obj->pages)
502e95c6 Zach Reizner 2015-03-04 65 vgem_gem_put_pages(vgem_obj);
502e95c6 Zach Reizner 2015-03-04 66
502e95c6 Zach Reizner 2015-03-04 67 vgem_obj->pages = NULL;
502e95c6 Zach Reizner 2015-03-04 68
502e95c6 Zach Reizner 2015-03-04 69 kfree(vgem_obj);
502e95c6 Zach Reizner 2015-03-04 70 }
502e95c6 Zach Reizner 2015-03-04 71
502e95c6 Zach Reizner 2015-03-04 72 int vgem_gem_get_pages(struct drm_vgem_gem_object *obj)
502e95c6 Zach Reizner 2015-03-04 73 {
502e95c6 Zach Reizner 2015-03-04 74 struct page **pages;
502e95c6 Zach Reizner 2015-03-04 75
502e95c6 Zach Reizner 2015-03-04 76 if (obj->pages || obj->use_dma_buf)
502e95c6 Zach Reizner 2015-03-04 77 return 0;
502e95c6 Zach Reizner 2015-03-04 78
502e95c6 Zach Reizner 2015-03-04 79 pages = drm_gem_get_pages(&obj->base);
502e95c6 Zach Reizner 2015-03-04 80 if (IS_ERR(pages)) {
502e95c6 Zach Reizner 2015-03-04 81 return PTR_ERR(pages);
502e95c6 Zach Reizner 2015-03-04 82 }
502e95c6 Zach Reizner 2015-03-04 83
502e95c6 Zach Reizner 2015-03-04 84 obj->pages = pages;
502e95c6 Zach Reizner 2015-03-04 85
502e95c6 Zach Reizner 2015-03-04 86 return 0;
502e95c6 Zach Reizner 2015-03-04 87 }
502e95c6 Zach Reizner 2015-03-04 88
502e95c6 Zach Reizner 2015-03-04 89 static int vgem_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
502e95c6 Zach Reizner 2015-03-04 90 {
502e95c6 Zach Reizner 2015-03-04 91 struct drm_vgem_gem_object *obj = vma->vm_private_data;
502e95c6 Zach Reizner 2015-03-04 92 struct drm_device *dev = obj->base.dev;
502e95c6 Zach Reizner 2015-03-04 93 loff_t num_pages;
502e95c6 Zach Reizner 2015-03-04 94 pgoff_t page_offset;
502e95c6 Zach Reizner 2015-03-04 95 int ret;
502e95c6 Zach Reizner 2015-03-04 96
502e95c6 Zach Reizner 2015-03-04 97 /* We don't use vmf->pgoff since that has the fake offset */
502e95c6 Zach Reizner 2015-03-04 98 page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
502e95c6 Zach Reizner 2015-03-04 99 PAGE_SHIFT;
502e95c6 Zach Reizner 2015-03-04 100
502e95c6 Zach Reizner 2015-03-04 101 num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
502e95c6 Zach Reizner 2015-03-04 102
502e95c6 Zach Reizner 2015-03-04 103 if (page_offset > num_pages)
502e95c6 Zach Reizner 2015-03-04 104 return VM_FAULT_SIGBUS;
502e95c6 Zach Reizner 2015-03-04 105
502e95c6 Zach Reizner 2015-03-04 106 mutex_lock(&dev->struct_mutex);
502e95c6 Zach Reizner 2015-03-04 107
502e95c6 Zach Reizner 2015-03-04 108 ret = vm_insert_page(vma, (unsigned long)vmf->virtual_address,
502e95c6 Zach Reizner 2015-03-04 109 obj->pages[page_offset]);
502e95c6 Zach Reizner 2015-03-04 110
502e95c6 Zach Reizner 2015-03-04 111 mutex_unlock(&dev->struct_mutex);
502e95c6 Zach Reizner 2015-03-04 112 switch (ret) {
502e95c6 Zach Reizner 2015-03-04 113 case 0:
502e95c6 Zach Reizner 2015-03-04 114 return VM_FAULT_NOPAGE;
502e95c6 Zach Reizner 2015-03-04 115 case -ENOMEM:
502e95c6 Zach Reizner 2015-03-04 116 return VM_FAULT_OOM;
502e95c6 Zach Reizner 2015-03-04 117 case -EBUSY:
502e95c6 Zach Reizner 2015-03-04 118 return VM_FAULT_RETRY;
502e95c6 Zach Reizner 2015-03-04 119 case -EFAULT:
502e95c6 Zach Reizner 2015-03-04 120 case -EINVAL:
502e95c6 Zach Reizner 2015-03-04 121 return VM_FAULT_SIGBUS;
502e95c6 Zach Reizner 2015-03-04 122 default:
502e95c6 Zach Reizner 2015-03-04 123 WARN_ON(1);
502e95c6 Zach Reizner 2015-03-04 124 return VM_FAULT_SIGBUS;
502e95c6 Zach Reizner 2015-03-04 125 }
502e95c6 Zach Reizner 2015-03-04 126 }
502e95c6 Zach Reizner 2015-03-04 127
7cbea8dc Kirill A. Shutemov 2015-09-09 128 static const struct vm_operations_struct vgem_gem_vm_ops = {
502e95c6 Zach Reizner 2015-03-04 129 .fault = vgem_gem_fault,
502e95c6 Zach Reizner 2015-03-04 130 .open = drm_gem_vm_open,
502e95c6 Zach Reizner 2015-03-04 131 .close = drm_gem_vm_close,
502e95c6 Zach Reizner 2015-03-04 132 };
502e95c6 Zach Reizner 2015-03-04 133
502e95c6 Zach Reizner 2015-03-04 134 /* ioctls */
502e95c6 Zach Reizner 2015-03-04 135
502e95c6 Zach Reizner 2015-03-04 136 static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
502e95c6 Zach Reizner 2015-03-04 137 struct drm_file *file,
502e95c6 Zach Reizner 2015-03-04 138 unsigned int *handle,
502e95c6 Zach Reizner 2015-03-04 139 unsigned long size)
502e95c6 Zach Reizner 2015-03-04 140 {
502e95c6 Zach Reizner 2015-03-04 141 struct drm_vgem_gem_object *obj;
502e95c6 Zach Reizner 2015-03-04 142 struct drm_gem_object *gem_object;
502e95c6 Zach Reizner 2015-03-04 143 int err;
502e95c6 Zach Reizner 2015-03-04 144
502e95c6 Zach Reizner 2015-03-04 145 size = roundup(size, PAGE_SIZE);
502e95c6 Zach Reizner 2015-03-04 146
502e95c6 Zach Reizner 2015-03-04 147 obj = kzalloc(sizeof(*obj), GFP_KERNEL);
502e95c6 Zach Reizner 2015-03-04 148 if (!obj)
502e95c6 Zach Reizner 2015-03-04 149 return ERR_PTR(-ENOMEM);
502e95c6 Zach Reizner 2015-03-04 150
502e95c6 Zach Reizner 2015-03-04 151 gem_object = &obj->base;
502e95c6 Zach Reizner 2015-03-04 152
502e95c6 Zach Reizner 2015-03-04 153 err = drm_gem_object_init(dev, gem_object, size);
502e95c6 Zach Reizner 2015-03-04 154 if (err)
502e95c6 Zach Reizner 2015-03-04 155 goto out;
502e95c6 Zach Reizner 2015-03-04 156
502e95c6 Zach Reizner 2015-03-04 157 err = drm_gem_handle_create(file, gem_object, handle);
502e95c6 Zach Reizner 2015-03-04 158 if (err)
502e95c6 Zach Reizner 2015-03-04 159 goto handle_out;
502e95c6 Zach Reizner 2015-03-04 160
502e95c6 Zach Reizner 2015-03-04 161 drm_gem_object_unreference_unlocked(gem_object);
502e95c6 Zach Reizner 2015-03-04 162
502e95c6 Zach Reizner 2015-03-04 163 return gem_object;
502e95c6 Zach Reizner 2015-03-04 164
502e95c6 Zach Reizner 2015-03-04 165 handle_out:
502e95c6 Zach Reizner 2015-03-04 166 drm_gem_object_release(gem_object);
502e95c6 Zach Reizner 2015-03-04 167 out:
502e95c6 Zach Reizner 2015-03-04 168 kfree(obj);
502e95c6 Zach Reizner 2015-03-04 169 return ERR_PTR(err);
502e95c6 Zach Reizner 2015-03-04 170 }
502e95c6 Zach Reizner 2015-03-04 171
502e95c6 Zach Reizner 2015-03-04 172 static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
502e95c6 Zach Reizner 2015-03-04 173 struct drm_mode_create_dumb *args)
502e95c6 Zach Reizner 2015-03-04 174 {
502e95c6 Zach Reizner 2015-03-04 175 struct drm_gem_object *gem_object;
502e95c6 Zach Reizner 2015-03-04 176 uint64_t size;
502e95c6 Zach Reizner 2015-03-04 177 uint64_t pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
502e95c6 Zach Reizner 2015-03-04 178
502e95c6 Zach Reizner 2015-03-04 179 size = args->height * pitch;
502e95c6 Zach Reizner 2015-03-04 180 if (size == 0)
502e95c6 Zach Reizner 2015-03-04 181 return -EINVAL;
502e95c6 Zach Reizner 2015-03-04 182
502e95c6 Zach Reizner 2015-03-04 183 gem_object = vgem_gem_create(dev, file, &args->handle, size);
502e95c6 Zach Reizner 2015-03-04 184
502e95c6 Zach Reizner 2015-03-04 185 if (IS_ERR(gem_object)) {
502e95c6 Zach Reizner 2015-03-04 186 DRM_DEBUG_DRIVER("object creation failed\n");
502e95c6 Zach Reizner 2015-03-04 187 return PTR_ERR(gem_object);
502e95c6 Zach Reizner 2015-03-04 188 }
502e95c6 Zach Reizner 2015-03-04 189
502e95c6 Zach Reizner 2015-03-04 190 args->size = gem_object->size;
502e95c6 Zach Reizner 2015-03-04 191 args->pitch = pitch;
502e95c6 Zach Reizner 2015-03-04 192
502e95c6 Zach Reizner 2015-03-04 193 DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
502e95c6 Zach Reizner 2015-03-04 194
502e95c6 Zach Reizner 2015-03-04 195 return 0;
502e95c6 Zach Reizner 2015-03-04 196 }
502e95c6 Zach Reizner 2015-03-04 197
502e95c6 Zach Reizner 2015-03-04 198 int vgem_gem_dumb_map(struct drm_file *file, struct drm_device *dev,
502e95c6 Zach Reizner 2015-03-04 199 uint32_t handle, uint64_t *offset)
502e95c6 Zach Reizner 2015-03-04 200 {
502e95c6 Zach Reizner 2015-03-04 201 int ret = 0;
502e95c6 Zach Reizner 2015-03-04 202 struct drm_gem_object *obj;
502e95c6 Zach Reizner 2015-03-04 203
502e95c6 Zach Reizner 2015-03-04 204 mutex_lock(&dev->struct_mutex);
502e95c6 Zach Reizner 2015-03-04 205 obj = drm_gem_object_lookup(dev, file, handle);
502e95c6 Zach Reizner 2015-03-04 206 if (!obj) {
502e95c6 Zach Reizner 2015-03-04 207 ret = -ENOENT;
502e95c6 Zach Reizner 2015-03-04 208 goto unlock;
502e95c6 Zach Reizner 2015-03-04 209 }
502e95c6 Zach Reizner 2015-03-04 210
502e95c6 Zach Reizner 2015-03-04 @211 if (!drm_vma_node_has_offset(&obj->vma_node)) {
502e95c6 Zach Reizner 2015-03-04 212 ret = drm_gem_create_mmap_offset(obj);
502e95c6 Zach Reizner 2015-03-04 213 if (ret)
502e95c6 Zach Reizner 2015-03-04 214 goto unref;
:::::: The code at line 211 was first introduced by commit
:::::: 502e95c6678505474f1056480310cd9382bacbac drm/vgem: implement virtual GEM
:::::: TO: Zach Reizner <zachr@google.com>
:::::: CC: Dave Airlie <airlied@redhat.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Fri, Oct 23, 2015 at 02:47:49AM +0800, kbuild test robot wrote: > Hi Daniel, > > [auto build test WARNING on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base] > > url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-Update-GEM-refcounting-docs/20151023-011317 > config: x86_64-randconfig-s1-10230205 (attached as .config) > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All warnings (new ones prefixed by >>): > > In file included from include/uapi/linux/stddef.h:1:0, > from include/linux/stddef.h:4, > from include/uapi/linux/posix_types.h:4, > from include/uapi/linux/types.h:13, > from include/linux/types.h:5, > from include/linux/list.h:4, > from include/linux/module.h:9, > from drivers/gpu/drm/vgem/vgem_drv.c:33: > drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_gem_dumb_map': > drivers/gpu/drm/vgem/vgem_drv.c:211:7: error: implicit declaration of function 'drm_vma_node_has_offset' [-Werror=implicit-function-declaration] > if (!drm_vma_node_has_offset(&obj->vma_node)) { > ^ > include/linux/compiler.h:147:28: note: in definition of macro '__trace_if' > if (__builtin_constant_p((cond)) ? !!(cond) : \ > ^ > >> drivers/gpu/drm/vgem/vgem_drv.c:211:2: note: in expansion of macro 'if' > if (!drm_vma_node_has_offset(&obj->vma_node)) { > ^ > cc1: some warnings being treated as errors Argh, I've forgotten that the vgem patch in my tree isn't merged yet. So that one needs to go in (to remove another dubious use of has_offset()) before we can pull in this one. -Daniel > > vim +/if +211 drivers/gpu/drm/vgem/vgem_drv.c > > 502e95c6 Zach Reizner 2015-03-04 27 > 502e95c6 Zach Reizner 2015-03-04 28 /** > 502e95c6 Zach Reizner 2015-03-04 29 * This is vgem, a (non-hardware-backed) GEM service. This is used by Mesa's > 502e95c6 Zach Reizner 2015-03-04 30 * software renderer and the X server for efficient buffer sharing. > 502e95c6 Zach Reizner 2015-03-04 31 */ > 502e95c6 Zach Reizner 2015-03-04 32 > 502e95c6 Zach Reizner 2015-03-04 @33 #include <linux/module.h> > 502e95c6 Zach Reizner 2015-03-04 34 #include <linux/ramfs.h> > 502e95c6 Zach Reizner 2015-03-04 35 #include <linux/shmem_fs.h> > 502e95c6 Zach Reizner 2015-03-04 36 #include <linux/dma-buf.h> > 502e95c6 Zach Reizner 2015-03-04 37 #include "vgem_drv.h" > 502e95c6 Zach Reizner 2015-03-04 38 > 502e95c6 Zach Reizner 2015-03-04 39 #define DRIVER_NAME "vgem" > 502e95c6 Zach Reizner 2015-03-04 40 #define DRIVER_DESC "Virtual GEM provider" > 502e95c6 Zach Reizner 2015-03-04 41 #define DRIVER_DATE "20120112" > 502e95c6 Zach Reizner 2015-03-04 42 #define DRIVER_MAJOR 1 > 502e95c6 Zach Reizner 2015-03-04 43 #define DRIVER_MINOR 0 > 502e95c6 Zach Reizner 2015-03-04 44 > 502e95c6 Zach Reizner 2015-03-04 45 void vgem_gem_put_pages(struct drm_vgem_gem_object *obj) > 502e95c6 Zach Reizner 2015-03-04 46 { > 502e95c6 Zach Reizner 2015-03-04 47 drm_gem_put_pages(&obj->base, obj->pages, false, false); > 502e95c6 Zach Reizner 2015-03-04 48 obj->pages = NULL; > 502e95c6 Zach Reizner 2015-03-04 49 } > 502e95c6 Zach Reizner 2015-03-04 50 > 502e95c6 Zach Reizner 2015-03-04 51 static void vgem_gem_free_object(struct drm_gem_object *obj) > 502e95c6 Zach Reizner 2015-03-04 52 { > 502e95c6 Zach Reizner 2015-03-04 53 struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj); > 502e95c6 Zach Reizner 2015-03-04 54 > 502e95c6 Zach Reizner 2015-03-04 55 drm_gem_free_mmap_offset(obj); > 502e95c6 Zach Reizner 2015-03-04 56 > 502e95c6 Zach Reizner 2015-03-04 57 if (vgem_obj->use_dma_buf && obj->dma_buf) { > 502e95c6 Zach Reizner 2015-03-04 58 dma_buf_put(obj->dma_buf); > 502e95c6 Zach Reizner 2015-03-04 59 obj->dma_buf = NULL; > 502e95c6 Zach Reizner 2015-03-04 60 } > 502e95c6 Zach Reizner 2015-03-04 61 > 502e95c6 Zach Reizner 2015-03-04 62 drm_gem_object_release(obj); > 502e95c6 Zach Reizner 2015-03-04 63 > 502e95c6 Zach Reizner 2015-03-04 64 if (vgem_obj->pages) > 502e95c6 Zach Reizner 2015-03-04 65 vgem_gem_put_pages(vgem_obj); > 502e95c6 Zach Reizner 2015-03-04 66 > 502e95c6 Zach Reizner 2015-03-04 67 vgem_obj->pages = NULL; > 502e95c6 Zach Reizner 2015-03-04 68 > 502e95c6 Zach Reizner 2015-03-04 69 kfree(vgem_obj); > 502e95c6 Zach Reizner 2015-03-04 70 } > 502e95c6 Zach Reizner 2015-03-04 71 > 502e95c6 Zach Reizner 2015-03-04 72 int vgem_gem_get_pages(struct drm_vgem_gem_object *obj) > 502e95c6 Zach Reizner 2015-03-04 73 { > 502e95c6 Zach Reizner 2015-03-04 74 struct page **pages; > 502e95c6 Zach Reizner 2015-03-04 75 > 502e95c6 Zach Reizner 2015-03-04 76 if (obj->pages || obj->use_dma_buf) > 502e95c6 Zach Reizner 2015-03-04 77 return 0; > 502e95c6 Zach Reizner 2015-03-04 78 > 502e95c6 Zach Reizner 2015-03-04 79 pages = drm_gem_get_pages(&obj->base); > 502e95c6 Zach Reizner 2015-03-04 80 if (IS_ERR(pages)) { > 502e95c6 Zach Reizner 2015-03-04 81 return PTR_ERR(pages); > 502e95c6 Zach Reizner 2015-03-04 82 } > 502e95c6 Zach Reizner 2015-03-04 83 > 502e95c6 Zach Reizner 2015-03-04 84 obj->pages = pages; > 502e95c6 Zach Reizner 2015-03-04 85 > 502e95c6 Zach Reizner 2015-03-04 86 return 0; > 502e95c6 Zach Reizner 2015-03-04 87 } > 502e95c6 Zach Reizner 2015-03-04 88 > 502e95c6 Zach Reizner 2015-03-04 89 static int vgem_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) > 502e95c6 Zach Reizner 2015-03-04 90 { > 502e95c6 Zach Reizner 2015-03-04 91 struct drm_vgem_gem_object *obj = vma->vm_private_data; > 502e95c6 Zach Reizner 2015-03-04 92 struct drm_device *dev = obj->base.dev; > 502e95c6 Zach Reizner 2015-03-04 93 loff_t num_pages; > 502e95c6 Zach Reizner 2015-03-04 94 pgoff_t page_offset; > 502e95c6 Zach Reizner 2015-03-04 95 int ret; > 502e95c6 Zach Reizner 2015-03-04 96 > 502e95c6 Zach Reizner 2015-03-04 97 /* We don't use vmf->pgoff since that has the fake offset */ > 502e95c6 Zach Reizner 2015-03-04 98 page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >> > 502e95c6 Zach Reizner 2015-03-04 99 PAGE_SHIFT; > 502e95c6 Zach Reizner 2015-03-04 100 > 502e95c6 Zach Reizner 2015-03-04 101 num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE); > 502e95c6 Zach Reizner 2015-03-04 102 > 502e95c6 Zach Reizner 2015-03-04 103 if (page_offset > num_pages) > 502e95c6 Zach Reizner 2015-03-04 104 return VM_FAULT_SIGBUS; > 502e95c6 Zach Reizner 2015-03-04 105 > 502e95c6 Zach Reizner 2015-03-04 106 mutex_lock(&dev->struct_mutex); > 502e95c6 Zach Reizner 2015-03-04 107 > 502e95c6 Zach Reizner 2015-03-04 108 ret = vm_insert_page(vma, (unsigned long)vmf->virtual_address, > 502e95c6 Zach Reizner 2015-03-04 109 obj->pages[page_offset]); > 502e95c6 Zach Reizner 2015-03-04 110 > 502e95c6 Zach Reizner 2015-03-04 111 mutex_unlock(&dev->struct_mutex); > 502e95c6 Zach Reizner 2015-03-04 112 switch (ret) { > 502e95c6 Zach Reizner 2015-03-04 113 case 0: > 502e95c6 Zach Reizner 2015-03-04 114 return VM_FAULT_NOPAGE; > 502e95c6 Zach Reizner 2015-03-04 115 case -ENOMEM: > 502e95c6 Zach Reizner 2015-03-04 116 return VM_FAULT_OOM; > 502e95c6 Zach Reizner 2015-03-04 117 case -EBUSY: > 502e95c6 Zach Reizner 2015-03-04 118 return VM_FAULT_RETRY; > 502e95c6 Zach Reizner 2015-03-04 119 case -EFAULT: > 502e95c6 Zach Reizner 2015-03-04 120 case -EINVAL: > 502e95c6 Zach Reizner 2015-03-04 121 return VM_FAULT_SIGBUS; > 502e95c6 Zach Reizner 2015-03-04 122 default: > 502e95c6 Zach Reizner 2015-03-04 123 WARN_ON(1); > 502e95c6 Zach Reizner 2015-03-04 124 return VM_FAULT_SIGBUS; > 502e95c6 Zach Reizner 2015-03-04 125 } > 502e95c6 Zach Reizner 2015-03-04 126 } > 502e95c6 Zach Reizner 2015-03-04 127 > 7cbea8dc Kirill A. Shutemov 2015-09-09 128 static const struct vm_operations_struct vgem_gem_vm_ops = { > 502e95c6 Zach Reizner 2015-03-04 129 .fault = vgem_gem_fault, > 502e95c6 Zach Reizner 2015-03-04 130 .open = drm_gem_vm_open, > 502e95c6 Zach Reizner 2015-03-04 131 .close = drm_gem_vm_close, > 502e95c6 Zach Reizner 2015-03-04 132 }; > 502e95c6 Zach Reizner 2015-03-04 133 > 502e95c6 Zach Reizner 2015-03-04 134 /* ioctls */ > 502e95c6 Zach Reizner 2015-03-04 135 > 502e95c6 Zach Reizner 2015-03-04 136 static struct drm_gem_object *vgem_gem_create(struct drm_device *dev, > 502e95c6 Zach Reizner 2015-03-04 137 struct drm_file *file, > 502e95c6 Zach Reizner 2015-03-04 138 unsigned int *handle, > 502e95c6 Zach Reizner 2015-03-04 139 unsigned long size) > 502e95c6 Zach Reizner 2015-03-04 140 { > 502e95c6 Zach Reizner 2015-03-04 141 struct drm_vgem_gem_object *obj; > 502e95c6 Zach Reizner 2015-03-04 142 struct drm_gem_object *gem_object; > 502e95c6 Zach Reizner 2015-03-04 143 int err; > 502e95c6 Zach Reizner 2015-03-04 144 > 502e95c6 Zach Reizner 2015-03-04 145 size = roundup(size, PAGE_SIZE); > 502e95c6 Zach Reizner 2015-03-04 146 > 502e95c6 Zach Reizner 2015-03-04 147 obj = kzalloc(sizeof(*obj), GFP_KERNEL); > 502e95c6 Zach Reizner 2015-03-04 148 if (!obj) > 502e95c6 Zach Reizner 2015-03-04 149 return ERR_PTR(-ENOMEM); > 502e95c6 Zach Reizner 2015-03-04 150 > 502e95c6 Zach Reizner 2015-03-04 151 gem_object = &obj->base; > 502e95c6 Zach Reizner 2015-03-04 152 > 502e95c6 Zach Reizner 2015-03-04 153 err = drm_gem_object_init(dev, gem_object, size); > 502e95c6 Zach Reizner 2015-03-04 154 if (err) > 502e95c6 Zach Reizner 2015-03-04 155 goto out; > 502e95c6 Zach Reizner 2015-03-04 156 > 502e95c6 Zach Reizner 2015-03-04 157 err = drm_gem_handle_create(file, gem_object, handle); > 502e95c6 Zach Reizner 2015-03-04 158 if (err) > 502e95c6 Zach Reizner 2015-03-04 159 goto handle_out; > 502e95c6 Zach Reizner 2015-03-04 160 > 502e95c6 Zach Reizner 2015-03-04 161 drm_gem_object_unreference_unlocked(gem_object); > 502e95c6 Zach Reizner 2015-03-04 162 > 502e95c6 Zach Reizner 2015-03-04 163 return gem_object; > 502e95c6 Zach Reizner 2015-03-04 164 > 502e95c6 Zach Reizner 2015-03-04 165 handle_out: > 502e95c6 Zach Reizner 2015-03-04 166 drm_gem_object_release(gem_object); > 502e95c6 Zach Reizner 2015-03-04 167 out: > 502e95c6 Zach Reizner 2015-03-04 168 kfree(obj); > 502e95c6 Zach Reizner 2015-03-04 169 return ERR_PTR(err); > 502e95c6 Zach Reizner 2015-03-04 170 } > 502e95c6 Zach Reizner 2015-03-04 171 > 502e95c6 Zach Reizner 2015-03-04 172 static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev, > 502e95c6 Zach Reizner 2015-03-04 173 struct drm_mode_create_dumb *args) > 502e95c6 Zach Reizner 2015-03-04 174 { > 502e95c6 Zach Reizner 2015-03-04 175 struct drm_gem_object *gem_object; > 502e95c6 Zach Reizner 2015-03-04 176 uint64_t size; > 502e95c6 Zach Reizner 2015-03-04 177 uint64_t pitch = args->width * DIV_ROUND_UP(args->bpp, 8); > 502e95c6 Zach Reizner 2015-03-04 178 > 502e95c6 Zach Reizner 2015-03-04 179 size = args->height * pitch; > 502e95c6 Zach Reizner 2015-03-04 180 if (size == 0) > 502e95c6 Zach Reizner 2015-03-04 181 return -EINVAL; > 502e95c6 Zach Reizner 2015-03-04 182 > 502e95c6 Zach Reizner 2015-03-04 183 gem_object = vgem_gem_create(dev, file, &args->handle, size); > 502e95c6 Zach Reizner 2015-03-04 184 > 502e95c6 Zach Reizner 2015-03-04 185 if (IS_ERR(gem_object)) { > 502e95c6 Zach Reizner 2015-03-04 186 DRM_DEBUG_DRIVER("object creation failed\n"); > 502e95c6 Zach Reizner 2015-03-04 187 return PTR_ERR(gem_object); > 502e95c6 Zach Reizner 2015-03-04 188 } > 502e95c6 Zach Reizner 2015-03-04 189 > 502e95c6 Zach Reizner 2015-03-04 190 args->size = gem_object->size; > 502e95c6 Zach Reizner 2015-03-04 191 args->pitch = pitch; > 502e95c6 Zach Reizner 2015-03-04 192 > 502e95c6 Zach Reizner 2015-03-04 193 DRM_DEBUG_DRIVER("Created object of size %lld\n", size); > 502e95c6 Zach Reizner 2015-03-04 194 > 502e95c6 Zach Reizner 2015-03-04 195 return 0; > 502e95c6 Zach Reizner 2015-03-04 196 } > 502e95c6 Zach Reizner 2015-03-04 197 > 502e95c6 Zach Reizner 2015-03-04 198 int vgem_gem_dumb_map(struct drm_file *file, struct drm_device *dev, > 502e95c6 Zach Reizner 2015-03-04 199 uint32_t handle, uint64_t *offset) > 502e95c6 Zach Reizner 2015-03-04 200 { > 502e95c6 Zach Reizner 2015-03-04 201 int ret = 0; > 502e95c6 Zach Reizner 2015-03-04 202 struct drm_gem_object *obj; > 502e95c6 Zach Reizner 2015-03-04 203 > 502e95c6 Zach Reizner 2015-03-04 204 mutex_lock(&dev->struct_mutex); > 502e95c6 Zach Reizner 2015-03-04 205 obj = drm_gem_object_lookup(dev, file, handle); > 502e95c6 Zach Reizner 2015-03-04 206 if (!obj) { > 502e95c6 Zach Reizner 2015-03-04 207 ret = -ENOENT; > 502e95c6 Zach Reizner 2015-03-04 208 goto unlock; > 502e95c6 Zach Reizner 2015-03-04 209 } > 502e95c6 Zach Reizner 2015-03-04 210 > 502e95c6 Zach Reizner 2015-03-04 @211 if (!drm_vma_node_has_offset(&obj->vma_node)) { > 502e95c6 Zach Reizner 2015-03-04 212 ret = drm_gem_create_mmap_offset(obj); > 502e95c6 Zach Reizner 2015-03-04 213 if (ret) > 502e95c6 Zach Reizner 2015-03-04 214 goto unref; > > :::::: The code at line 211 was first introduced by commit > :::::: 502e95c6678505474f1056480310cd9382bacbac drm/vgem: implement virtual GEM > > :::::: TO: Zach Reizner <zachr@google.com> > :::::: CC: Dave Airlie <airlied@redhat.com> > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi On Thu, Oct 22, 2015 at 7:11 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > It's racy, creating mmap offsets is a slowpath, so better to remove it > to avoid drivers doing broken things. > > The only user is i915, and it's ok there because everything (well > almost) is protected by dev->struct_mutex in i915-gem. > > While at it add a note in the create_mmap_offset kerneldoc that > drivers must release it again. And then I also noticed that > drm_gem_object_release entirely lacks kerneldoc. > > Cc: David Herrmann <dh.herrmann@gmail.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> I'd even argue that no driver should ever call into drm_vma_offset_add() nor drm_vma_offset_remove(). For TTM this is already the case, for plain old gem drivers still call drm_vma_offset_add() (which is fine to me, but could be turned into a gem helper). Anyway, if TTM wasn't a module, we should drop the export of drm_vma_offset_remove(), but that'll never happen, I guess. Long story short: If anyone calls drm_vma_offset_remove() somewhere but in the destructor, it's probably racy, so I fully support this patch. The vma helpers are also made fail-safe on purpose, it has never been a fast-path. Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Thanks David > --- > drivers/gpu/drm/drm_gem.c | 17 +++++++++++++++++ > drivers/gpu/drm/i915/i915_gem.c | 3 --- > include/drm/drm_vma_manager.h | 15 +-------------- > 3 files changed, 18 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 64353d40db53..38680380c6b3 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -387,6 +387,10 @@ EXPORT_SYMBOL(drm_gem_handle_create); > * @obj: obj in question > * > * This routine frees fake offsets allocated by drm_gem_create_mmap_offset(). > + * > + * Note that drm_gem_object_release() already calls this function, so drivers > + * don't have to take care of releasing the mmap offset themselves when freeing > + * the GEM object. > */ > void > drm_gem_free_mmap_offset(struct drm_gem_object *obj) > @@ -410,6 +414,9 @@ EXPORT_SYMBOL(drm_gem_free_mmap_offset); > * This routine allocates and attaches a fake offset for @obj, in cases where > * the virtual size differs from the physical size (ie. obj->size). Otherwise > * just use drm_gem_create_mmap_offset(). > + * > + * This function is idempotent and handles an already allocated mmap offset > + * transparently. Drivers do not need to check for this case. > */ > int > drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size) > @@ -431,6 +438,9 @@ EXPORT_SYMBOL(drm_gem_create_mmap_offset_size); > * structures. > * > * This routine allocates and attaches a fake offset for @obj. > + * > + * Drivers can call drm_gem_free_mmap_offset() before freeing @obj to release > + * the fake offset again. > */ > int drm_gem_create_mmap_offset(struct drm_gem_object *obj) > { > @@ -739,6 +749,13 @@ drm_gem_release(struct drm_device *dev, struct drm_file *file_private) > idr_destroy(&file_private->object_idr); > } > > +/** > + * drm_gem_object_release - release GEM buffer object resources > + * @obj: GEM buffer object > + * > + * This releases any structures and resources used by @obj and is the invers of > + * drm_gem_object_init(). > + */ > void > drm_gem_object_release(struct drm_gem_object *obj) > { > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index d0fa5481543c..01fef54ecb2d 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1972,9 +1972,6 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) > struct drm_i915_private *dev_priv = obj->base.dev->dev_private; > int ret; > > - if (drm_vma_node_has_offset(&obj->base.vma_node)) > - return 0; > - > dev_priv->mm.shrinker_no_lock_stealing = true; > > ret = drm_gem_create_mmap_offset(&obj->base); > diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h > index 2f63dd5e05eb..06ea8e077ec2 100644 > --- a/include/drm/drm_vma_manager.h > +++ b/include/drm/drm_vma_manager.h > @@ -176,19 +176,6 @@ static inline unsigned long drm_vma_node_size(struct drm_vma_offset_node *node) > } > > /** > - * drm_vma_node_has_offset() - Check whether node is added to offset manager > - * @node: Node to be checked > - * > - * RETURNS: > - * true iff the node was previously allocated an offset and added to > - * an vma offset manager. > - */ > -static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node) > -{ > - return drm_mm_node_allocated(&node->vm_node); > -} > - > -/** > * drm_vma_node_offset_addr() - Return sanitized offset for user-space mmaps > * @node: Linked offset node > * > @@ -220,7 +207,7 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node) > static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node, > struct address_space *file_mapping) > { > - if (drm_vma_node_has_offset(node)) > + if (drm_mm_node_allocated(&node->vm_node)) > unmap_mapping_range(file_mapping, > drm_vma_node_offset_addr(node), > drm_vma_node_size(node) << PAGE_SHIFT, 1); > -- > 2.5.1 >
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 64353d40db53..38680380c6b3 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -387,6 +387,10 @@ EXPORT_SYMBOL(drm_gem_handle_create); * @obj: obj in question * * This routine frees fake offsets allocated by drm_gem_create_mmap_offset(). + * + * Note that drm_gem_object_release() already calls this function, so drivers + * don't have to take care of releasing the mmap offset themselves when freeing + * the GEM object. */ void drm_gem_free_mmap_offset(struct drm_gem_object *obj) @@ -410,6 +414,9 @@ EXPORT_SYMBOL(drm_gem_free_mmap_offset); * This routine allocates and attaches a fake offset for @obj, in cases where * the virtual size differs from the physical size (ie. obj->size). Otherwise * just use drm_gem_create_mmap_offset(). + * + * This function is idempotent and handles an already allocated mmap offset + * transparently. Drivers do not need to check for this case. */ int drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size) @@ -431,6 +438,9 @@ EXPORT_SYMBOL(drm_gem_create_mmap_offset_size); * structures. * * This routine allocates and attaches a fake offset for @obj. + * + * Drivers can call drm_gem_free_mmap_offset() before freeing @obj to release + * the fake offset again. */ int drm_gem_create_mmap_offset(struct drm_gem_object *obj) { @@ -739,6 +749,13 @@ drm_gem_release(struct drm_device *dev, struct drm_file *file_private) idr_destroy(&file_private->object_idr); } +/** + * drm_gem_object_release - release GEM buffer object resources + * @obj: GEM buffer object + * + * This releases any structures and resources used by @obj and is the invers of + * drm_gem_object_init(). + */ void drm_gem_object_release(struct drm_gem_object *obj) { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index d0fa5481543c..01fef54ecb2d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1972,9 +1972,6 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj) struct drm_i915_private *dev_priv = obj->base.dev->dev_private; int ret; - if (drm_vma_node_has_offset(&obj->base.vma_node)) - return 0; - dev_priv->mm.shrinker_no_lock_stealing = true; ret = drm_gem_create_mmap_offset(&obj->base); diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h index 2f63dd5e05eb..06ea8e077ec2 100644 --- a/include/drm/drm_vma_manager.h +++ b/include/drm/drm_vma_manager.h @@ -176,19 +176,6 @@ static inline unsigned long drm_vma_node_size(struct drm_vma_offset_node *node) } /** - * drm_vma_node_has_offset() - Check whether node is added to offset manager - * @node: Node to be checked - * - * RETURNS: - * true iff the node was previously allocated an offset and added to - * an vma offset manager. - */ -static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node) -{ - return drm_mm_node_allocated(&node->vm_node); -} - -/** * drm_vma_node_offset_addr() - Return sanitized offset for user-space mmaps * @node: Linked offset node * @@ -220,7 +207,7 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node) static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node, struct address_space *file_mapping) { - if (drm_vma_node_has_offset(node)) + if (drm_mm_node_allocated(&node->vm_node)) unmap_mapping_range(file_mapping, drm_vma_node_offset_addr(node), drm_vma_node_size(node) << PAGE_SHIFT, 1);
It's racy, creating mmap offsets is a slowpath, so better to remove it to avoid drivers doing broken things. The only user is i915, and it's ok there because everything (well almost) is protected by dev->struct_mutex in i915-gem. While at it add a note in the create_mmap_offset kerneldoc that drivers must release it again. And then I also noticed that drm_gem_object_release entirely lacks kerneldoc. Cc: David Herrmann <dh.herrmann@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/drm_gem.c | 17 +++++++++++++++++ drivers/gpu/drm/i915/i915_gem.c | 3 --- include/drm/drm_vma_manager.h | 15 +-------------- 3 files changed, 18 insertions(+), 17 deletions(-)