Re: [PATCH 1/2] gpu/radeon: use HMM mirror instead of mmu_notifier

From: kbuild test robot
Date: Mon Sep 10 2018 - 14:35:52 EST

Next message: Jarkko Sakkinen: "Re: [PATCH v13 11/13] platform/x86: Intel SGX driver"
Previous message: kbuild test robot: "Re: [PATCH 2/2] gpu/radeon: use HMM mirror for userptr buffer object."
In reply to: jglisse: "[PATCH 1/2] gpu/radeon: use HMM mirror instead of mmu_notifier"
Next in thread: kbuild test robot: "Re: [PATCH 1/2] gpu/radeon: use HMM mirror instead of mmu_notifier"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Jérôme,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc3 next-20180910]
[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/jglisse-redhat-com/Getting-rid-of-GUP-and-use-HMM-for-user-ptr-features/20180911-020741
config: x86_64-randconfig-x017-201836 (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 error/warnings (new ones prefixed by >>):

>> drivers/gpu/drm/radeon/radeon_mn.c:43:20: error: field 'mirror' has incomplete type
struct hmm_mirror mirror;
^~~~~~
drivers/gpu/drm/radeon/radeon_mn.c: In function 'radeon_mn_destroy':
>> drivers/gpu/drm/radeon/radeon_mn.c:90:2: error: implicit declaration of function 'hmm_mirror_unregister'; did you mean 'drm_dp_aux_unregister'? [-Werror=implicit-function-declaration]
hmm_mirror_unregister(&rmn->mirror);
^~~~~~~~~~~~~~~~~~~~~
drm_dp_aux_unregister
In file included from include/linux/firmware.h:6:0,
from drivers/gpu/drm/radeon/radeon_mn.c:31:
drivers/gpu/drm/radeon/radeon_mn.c: In function 'radeon_mirror_release':
>> include/linux/kernel.h:997:32: error: dereferencing pointer to incomplete type 'struct hmm_mirror'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~
include/linux/compiler.h:335:18: note: in definition of macro '__compiletime_assert'
int __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:358:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:997:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
include/linux/kernel.h:997:20: note: in expansion of macro '__same_type'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
>> drivers/gpu/drm/radeon/radeon_mn.c:103:26: note: in expansion of macro 'container_of'
struct radeon_mn *rmn = container_of(mirror, struct radeon_mn, mirror);
^~~~~~~~~~~~
drivers/gpu/drm/radeon/radeon_mn.c: At top level:
>> drivers/gpu/drm/radeon/radeon_mn.c:119:24: warning: 'struct hmm_update' declared inside parameter list will not be visible outside of this definition or declaration
const struct hmm_update *update)
^~~~~~~~~~
drivers/gpu/drm/radeon/radeon_mn.c: In function 'radeon_sync_cpu_device_pagetables':
>> drivers/gpu/drm/radeon/radeon_mn.c:128:14: error: dereferencing pointer to incomplete type 'const struct hmm_update'
end = update->end - 1;
^~
drivers/gpu/drm/radeon/radeon_mn.c: At top level:
>> drivers/gpu/drm/radeon/radeon_mn.c:183:21: error: variable 'radeon_mirror_ops' has initializer but incomplete type
static const struct hmm_mirror_ops radeon_mirror_ops = {
^~~~~~~~~~~~~~
>> drivers/gpu/drm/radeon/radeon_mn.c:184:3: error: 'const struct hmm_mirror_ops' has no member named 'sync_cpu_device_pagetables'
.sync_cpu_device_pagetables = &radeon_sync_cpu_device_pagetables,
^~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/radeon/radeon_mn.c:184:32: warning: excess elements in struct initializer
.sync_cpu_device_pagetables = &radeon_sync_cpu_device_pagetables,
^
drivers/gpu/drm/radeon/radeon_mn.c:184:32: note: (near initialization for 'radeon_mirror_ops')
>> drivers/gpu/drm/radeon/radeon_mn.c:185:3: error: 'const struct hmm_mirror_ops' has no member named 'release'
.release = &radeon_mirror_release,
^~~~~~~
drivers/gpu/drm/radeon/radeon_mn.c:185:13: warning: excess elements in struct initializer
.release = &radeon_mirror_release,
^
drivers/gpu/drm/radeon/radeon_mn.c:185:13: note: (near initialization for 'radeon_mirror_ops')
drivers/gpu/drm/radeon/radeon_mn.c: In function 'radeon_mn_get':
>> drivers/gpu/drm/radeon/radeon_mn.c:224:6: error: implicit declaration of function 'hmm_mirror_register'; did you mean 'drm_dp_aux_register'? [-Werror=implicit-function-declaration]
r = hmm_mirror_register(&new->mirror, mm);
^~~~~~~~~~~~~~~~~~~
drm_dp_aux_register
drivers/gpu/drm/radeon/radeon_mn.c: At top level:
>> drivers/gpu/drm/radeon/radeon_mn.c:183:36: error: storage size of 'radeon_mirror_ops' isn't known
static const struct hmm_mirror_ops radeon_mirror_ops = {
^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
drivers/gpu//drm/radeon/radeon_mn.c:43:20: error: field 'mirror' has incomplete type
struct hmm_mirror mirror;
^~~~~~
drivers/gpu//drm/radeon/radeon_mn.c: In function 'radeon_mn_destroy':
drivers/gpu//drm/radeon/radeon_mn.c:90:2: error: implicit declaration of function 'hmm_mirror_unregister'; did you mean 'drm_dp_aux_unregister'? [-Werror=implicit-function-declaration]
hmm_mirror_unregister(&rmn->mirror);
^~~~~~~~~~~~~~~~~~~~~
drm_dp_aux_unregister
In file included from include/linux/firmware.h:6:0,
from drivers/gpu//drm/radeon/radeon_mn.c:31:
drivers/gpu//drm/radeon/radeon_mn.c: In function 'radeon_mirror_release':
>> include/linux/kernel.h:997:32: error: dereferencing pointer to incomplete type 'struct hmm_mirror'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~
include/linux/compiler.h:335:18: note: in definition of macro '__compiletime_assert'
int __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:358:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:997:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
include/linux/kernel.h:997:20: note: in expansion of macro '__same_type'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
drivers/gpu//drm/radeon/radeon_mn.c:103:26: note: in expansion of macro 'container_of'
struct radeon_mn *rmn = container_of(mirror, struct radeon_mn, mirror);
^~~~~~~~~~~~
drivers/gpu//drm/radeon/radeon_mn.c: At top level:
drivers/gpu//drm/radeon/radeon_mn.c:119:24: warning: 'struct hmm_update' declared inside parameter list will not be visible outside of this definition or declaration
const struct hmm_update *update)
^~~~~~~~~~
drivers/gpu//drm/radeon/radeon_mn.c: In function 'radeon_sync_cpu_device_pagetables':
drivers/gpu//drm/radeon/radeon_mn.c:128:14: error: dereferencing pointer to incomplete type 'const struct hmm_update'
end = update->end - 1;
^~
drivers/gpu//drm/radeon/radeon_mn.c: At top level:
drivers/gpu//drm/radeon/radeon_mn.c:183:21: error: variable 'radeon_mirror_ops' has initializer but incomplete type
static const struct hmm_mirror_ops radeon_mirror_ops = {
^~~~~~~~~~~~~~
drivers/gpu//drm/radeon/radeon_mn.c:184:3: error: 'const struct hmm_mirror_ops' has no member named 'sync_cpu_device_pagetables'
.sync_cpu_device_pagetables = &radeon_sync_cpu_device_pagetables,
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu//drm/radeon/radeon_mn.c:184:32: warning: excess elements in struct initializer
.sync_cpu_device_pagetables = &radeon_sync_cpu_device_pagetables,
^
drivers/gpu//drm/radeon/radeon_mn.c:184:32: note: (near initialization for 'radeon_mirror_ops')
drivers/gpu//drm/radeon/radeon_mn.c:185:3: error: 'const struct hmm_mirror_ops' has no member named 'release'
.release = &radeon_mirror_release,
^~~~~~~
drivers/gpu//drm/radeon/radeon_mn.c:185:13: warning: excess elements in struct initializer
.release = &radeon_mirror_release,
^
drivers/gpu//drm/radeon/radeon_mn.c:185:13: note: (near initialization for 'radeon_mirror_ops')
drivers/gpu//drm/radeon/radeon_mn.c: In function 'radeon_mn_get':
drivers/gpu//drm/radeon/radeon_mn.c:224:6: error: implicit declaration of function 'hmm_mirror_register'; did you mean 'drm_dp_aux_register'? [-Werror=implicit-function-declaration]
r = hmm_mirror_register(&new->mirror, mm);
^~~~~~~~~~~~~~~~~~~
drm_dp_aux_register
drivers/gpu//drm/radeon/radeon_mn.c: At top level:
drivers/gpu//drm/radeon/radeon_mn.c:183:36: error: storage size of 'radeon_mirror_ops' isn't known
static const struct hmm_mirror_ops radeon_mirror_ops = {
^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

vim +/mirror +43 drivers/gpu/drm/radeon/radeon_mn.c

38
39 struct radeon_mn {
40 /* constant after initialisation */
41 struct radeon_device *rdev;
42 struct mm_struct *mm;
> 43 struct hmm_mirror mirror;
44
45 /* only used on destruction */
46 struct work_struct work;
47
48 /* protected by rdev->mn_lock */
49 struct hlist_node node;
50
51 /* objects protected by lock */
52 struct mutex lock;
53 struct rb_root_cached objects;
54 };
55
56 struct radeon_mn_node {
57 struct interval_tree_node it;
58 struct list_head bos;
59 };
60
61 /**
62 * radeon_mn_destroy - destroy the rmn
63 *
64 * @work: previously sheduled work item
65 *
66 * Lazy destroys the notifier from a work item
67 */
68 static void radeon_mn_destroy(struct work_struct *work)
69 {
70 struct radeon_mn *rmn = container_of(work, struct radeon_mn, work);
71 struct radeon_device *rdev = rmn->rdev;
72 struct radeon_mn_node *node, *next_node;
73 struct radeon_bo *bo, *next_bo;
74
75 mutex_lock(&rdev->mn_lock);
76 mutex_lock(&rmn->lock);
77 hash_del(&rmn->node);
78 rbtree_postorder_for_each_entry_safe(node, next_node,
79 &rmn->objects.rb_root, it.rb) {
80
81 interval_tree_remove(&node->it, &rmn->objects);
82 list_for_each_entry_safe(bo, next_bo, &node->bos, mn_list) {
83 bo->mn = NULL;
84 list_del_init(&bo->mn_list);
85 }
86 kfree(node);
87 }
88 mutex_unlock(&rmn->lock);
89 mutex_unlock(&rdev->mn_lock);
> 90 hmm_mirror_unregister(&rmn->mirror);
91 kfree(rmn);
92 }
93
94 /**
95 * radeon_mn_release - callback to notify about mm destruction
96 *
97 * @mirror: our mirror struct
98 *
99 * Shedule a work item to lazy destroy our notifier.
100 */
101 static void radeon_mirror_release(struct hmm_mirror *mirror)
102 {
> 103 struct radeon_mn *rmn = container_of(mirror, struct radeon_mn, mirror);
104 INIT_WORK(&rmn->work, radeon_mn_destroy);
105 schedule_work(&rmn->work);
106 }
107
108 /**
109 * radeon_sync_cpu_device_pagetables - callback to synchronize with mm changes
110 *
111 * @mirror: our HMM mirror
112 * @update: update informations (start, end, event, blockable, ...)
113 *
114 * We block for all BOs between start and end to be idle and unmap them by
115 * moving them into system domain again (trigger a call to ttm_backend_func.
116 * unbind see radeon_ttm.c).
117 */
118 static int radeon_sync_cpu_device_pagetables(struct hmm_mirror *mirror,
> 119 const struct hmm_update *update)
120 {
121 struct radeon_mn *rmn = container_of(mirror, struct radeon_mn, mirror);
122 struct ttm_operation_ctx ctx = { false, false };
123 struct interval_tree_node *it;
124 unsigned long end;
125 int ret = 0;
126
127 /* notification is exclusive, but interval is inclusive */
> 128 end = update->end - 1;
129
130 /* TODO we should be able to split locking for interval tree and
131 * the tear down.
132 */
133 if (update->blockable)
134 mutex_lock(&rmn->lock);
135 else if (!mutex_trylock(&rmn->lock))
136 return -EAGAIN;
137
138 it = interval_tree_iter_first(&rmn->objects, update->start, end);
139 while (it) {
140 struct radeon_mn_node *node;
141 struct radeon_bo *bo;
142 long r;
143
144 if (!update->blockable) {
145 ret = -EAGAIN;
146 goto out_unlock;
147 }
148
149 node = container_of(it, struct radeon_mn_node, it);
150 it = interval_tree_iter_next(it, update->start, end);
151
152 list_for_each_entry(bo, &node->bos, mn_list) {
153
154 if (!bo->tbo.ttm || bo->tbo.ttm->state != tt_bound)
155 continue;
156
157 r = radeon_bo_reserve(bo, true);
158 if (r) {
159 DRM_ERROR("(%ld) failed to reserve user bo\n", r);
160 continue;
161 }
162
163 r = reservation_object_wait_timeout_rcu(bo->tbo.resv,
164 true, false, MAX_SCHEDULE_TIMEOUT);
165 if (r <= 0)
166 DRM_ERROR("(%ld) failed to wait for user bo\n", r);
167
168 radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU);
169 r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
170 if (r)
171 DRM_ERROR("(%ld) failed to validate user bo\n", r);
172
173 radeon_bo_unreserve(bo);
174 }
175 }
176
177 out_unlock:
178 mutex_unlock(&rmn->lock);
179
180 return ret;
181 }
182
> 183 static const struct hmm_mirror_ops radeon_mirror_ops = {
> 184 .sync_cpu_device_pagetables = &radeon_sync_cpu_device_pagetables,
> 185 .release = &radeon_mirror_release,
186 };
187
188 /**
189 * radeon_mn_get - create notifier context
190 *
191 * @rdev: radeon device pointer
192 *
193 * Creates a notifier context for current->mm.
194 */
195 static struct radeon_mn *radeon_mn_get(struct radeon_device *rdev)
196 {
197 struct mm_struct *mm = current->mm;
198 struct radeon_mn *rmn, *new;
199 int r;
200
201 mutex_lock(&rdev->mn_lock);
202 hash_for_each_possible(rdev->mn_hash, rmn, node, (unsigned long)mm) {
203 if (rmn->mm == mm) {
204 mutex_unlock(&rdev->mn_lock);
205 return rmn;
206 }
207 }
208 mutex_unlock(&rdev->mn_lock);
209
210 new = kzalloc(sizeof(*rmn), GFP_KERNEL);
211 if (!new) {
212 return ERR_PTR(-ENOMEM);
213 }
214 new->mm = mm;
215 new->rdev = rdev;
216 mutex_init(&new->lock);
217 new->objects = RB_ROOT_CACHED;
218 new->mirror.ops = &radeon_mirror_ops;
219
220 if (down_write_killable(&mm->mmap_sem)) {
221 kfree(new);
222 return ERR_PTR(-EINTR);
223 }
> 224 r = hmm_mirror_register(&new->mirror, mm);
225 up_write(&mm->mmap_sem);
226 if (r) {
227 kfree(new);
228 return ERR_PTR(r);
229 }
230
231 mutex_lock(&rdev->mn_lock);
232 /* Check again in case some other thread raced with us ... */
233 hash_for_each_possible(rdev->mn_hash, rmn, node, (unsigned long)mm) {
234 if (rmn->mm == mm) {
235 mutex_unlock(&rdev->mn_lock);
236 hmm_mirror_unregister(&new->mirror);
237 kfree(new);
238 return rmn;
239 }
240 }
241 hash_add(rdev->mn_hash, &new->node, (unsigned long)mm);
242 mutex_unlock(&rdev->mn_lock);
243
244 return new;
245 }
246

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

Attachment: .config.gz
Description: application/gzip

Next message: Jarkko Sakkinen: "Re: [PATCH v13 11/13] platform/x86: Intel SGX driver"
Previous message: kbuild test robot: "Re: [PATCH 2/2] gpu/radeon: use HMM mirror for userptr buffer object."
In reply to: jglisse: "[PATCH 1/2] gpu/radeon: use HMM mirror instead of mmu_notifier"
Next in thread: kbuild test robot: "Re: [PATCH 1/2] gpu/radeon: use HMM mirror instead of mmu_notifier"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]