| Message ID | 20191104173801.2972-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] dma_resv: prime lockdep annotations | expand |
On Mon, Nov 04, 2019 at 06:37:59PM +0100, Daniel Vetter wrote: > Full audit of everyone: > > - i915, radeon, amdgpu should be clean per their maintainers. > > - vram helpers should be fine, they don't do command submission, so > really no business holding struct_mutex while doing copy_*_user. But > I haven't checked them all. > > - panfrost seems to dma_resv_lock only in panfrost_job_push, which > looks clean. > > - v3d holds dma_resv locks in the tail of its v3d_submit_cl_ioctl(), > copying from/to userspace happens all in v3d_lookup_bos which is > outside of the critical section. > > - vmwgfx has a bunch of ioctls that do their own copy_*_user: > - vmw_execbuf_process: First this does some copies in > vmw_execbuf_cmdbuf() and also in the vmw_execbuf_process() itself. > Then comes the usual ttm reserve/validate sequence, then actual > submission/fencing, then unreserving, and finally some more > copy_to_user in vmw_execbuf_copy_fence_user. Glossing over tons of > details, but looks all safe. > - vmw_fence_event_ioctl: No ttm_reserve/dma_resv_lock anywhere to be > seen, seems to only create a fence and copy it out. > - a pile of smaller ioctl in vmwgfx_ioctl.c, no reservations to be > found there. > Summary: vmwgfx seems to be fine too. > > - virtio: There's virtio_gpu_execbuffer_ioctl, which does all the > copying from userspace before even looking up objects through their > handles, so safe. Plus the getparam/getcaps ioctl, also both safe. > > - qxl only has qxl_execbuffer_ioctl, which calls into > qxl_process_single_command. There's a lovely comment before the > __copy_from_user_inatomic that the slowpath should be copied from > i915, but I guess that never happened. Try not to be unlucky and get > your CS data evicted between when it's written and the kernel tries > to read it. The only other copy_from_user is for relocs, but those > are done before qxl_release_reserve_list(), which seems to be the > only thing reserving buffers (in the ttm/dma_resv sense) in that > code. So looks safe. > > - A debugfs file in nouveau_debugfs_pstate_set() and the usif ioctl in > usif_ioctl() look safe. nouveau_gem_ioctl_pushbuf() otoh breaks this > everywhere and needs to be fixed up. > > v2: Thomas pointed at that vmwgfx calls dma_resv_init while it holds a > dma_resv lock of a different object already. Christian mentioned that > ttm core does this too for ghost objects. intel-gfx-ci highlighted > that i915 has similar issues. > > Unfortunately we can't do this in the usual module init functions, > because kernel threads don't have an ->mm - we have to wait around for > some user thread to do this. > > Solution is to spawn a worker (but only once). It's horrible, but it > works. > > v3: We can allocate mm! (Chris). Horrible worker hack out, clean > initcall solution in. > > v4: Annotate with __init (Rob Herring) > > Cc: Rob Herring <robh@kernel.org> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Christian König <christian.koenig@amd.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Rob Herring <robh@kernel.org> > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Eric Anholt <eric@anholt.net> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Ben Skeggs <bskeggs@redhat.com> > Cc: "VMware Graphics" <linux-graphics-maintainer@vmware.com> > Cc: Thomas Hellstrom <thellstrom@vmware.com> > Reviewed-by: Christian König <christian.koenig@amd.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > Tested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> I lost the r-b from Thomas on the last round: Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com> > --- > drivers/dma-buf/dma-resv.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 709002515550..a05ff542be22 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -34,6 +34,7 @@ > > #include <linux/dma-resv.h> > #include <linux/export.h> > +#include <linux/sched/mm.h> > > /** > * DOC: Reservation Object Overview > @@ -95,6 +96,29 @@ static void dma_resv_list_free(struct dma_resv_list *list) > kfree_rcu(list, rcu); > } > > +#if IS_ENABLED(CONFIG_LOCKDEP) > +static void __init dma_resv_lockdep(void) > +{ > + struct mm_struct *mm = mm_alloc(); > + struct dma_resv obj; > + > + if (!mm) > + return; > + > + dma_resv_init(&obj); > + > + down_read(&mm->mmap_sem); > + ww_mutex_lock(&obj.lock, NULL); > + fs_reclaim_acquire(GFP_KERNEL); > + fs_reclaim_release(GFP_KERNEL); > + ww_mutex_unlock(&obj.lock); > + up_read(&mm->mmap_sem); > + > + mmput(mm); > +} > +subsys_initcall(dma_resv_lockdep); > +#endif > + > /** > * dma_resv_init - initialize a reservation object > * @obj: the reservation object > -- > 2.24.0.rc2 >
Am 04.11.19 um 18:37 schrieb Daniel Vetter: > Full audit of everyone: > > - i915, radeon, amdgpu should be clean per their maintainers. > > - vram helpers should be fine, they don't do command submission, so > really no business holding struct_mutex while doing copy_*_user. But > I haven't checked them all. > > - panfrost seems to dma_resv_lock only in panfrost_job_push, which > looks clean. > > - v3d holds dma_resv locks in the tail of its v3d_submit_cl_ioctl(), > copying from/to userspace happens all in v3d_lookup_bos which is > outside of the critical section. > > - vmwgfx has a bunch of ioctls that do their own copy_*_user: > - vmw_execbuf_process: First this does some copies in > vmw_execbuf_cmdbuf() and also in the vmw_execbuf_process() itself. > Then comes the usual ttm reserve/validate sequence, then actual > submission/fencing, then unreserving, and finally some more > copy_to_user in vmw_execbuf_copy_fence_user. Glossing over tons of > details, but looks all safe. > - vmw_fence_event_ioctl: No ttm_reserve/dma_resv_lock anywhere to be > seen, seems to only create a fence and copy it out. > - a pile of smaller ioctl in vmwgfx_ioctl.c, no reservations to be > found there. > Summary: vmwgfx seems to be fine too. > > - virtio: There's virtio_gpu_execbuffer_ioctl, which does all the > copying from userspace before even looking up objects through their > handles, so safe. Plus the getparam/getcaps ioctl, also both safe. > > - qxl only has qxl_execbuffer_ioctl, which calls into > qxl_process_single_command. There's a lovely comment before the > __copy_from_user_inatomic that the slowpath should be copied from > i915, but I guess that never happened. Try not to be unlucky and get > your CS data evicted between when it's written and the kernel tries > to read it. The only other copy_from_user is for relocs, but those > are done before qxl_release_reserve_list(), which seems to be the > only thing reserving buffers (in the ttm/dma_resv sense) in that > code. So looks safe. > > - A debugfs file in nouveau_debugfs_pstate_set() and the usif ioctl in > usif_ioctl() look safe. nouveau_gem_ioctl_pushbuf() otoh breaks this > everywhere and needs to be fixed up. > > v2: Thomas pointed at that vmwgfx calls dma_resv_init while it holds a > dma_resv lock of a different object already. Christian mentioned that > ttm core does this too for ghost objects. intel-gfx-ci highlighted > that i915 has similar issues. > > Unfortunately we can't do this in the usual module init functions, > because kernel threads don't have an ->mm - we have to wait around for > some user thread to do this. > > Solution is to spawn a worker (but only once). It's horrible, but it > works. > > v3: We can allocate mm! (Chris). Horrible worker hack out, clean > initcall solution in. > > v4: Annotate with __init (Rob Herring) > > Cc: Rob Herring <robh@kernel.org> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Christian König <christian.koenig@amd.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Rob Herring <robh@kernel.org> > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Eric Anholt <eric@anholt.net> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Ben Skeggs <bskeggs@redhat.com> > Cc: "VMware Graphics" <linux-graphics-maintainer@vmware.com> > Cc: Thomas Hellstrom <thellstrom@vmware.com> > Reviewed-by: Christian König <christian.koenig@amd.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > Tested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> What's holding you back to commit that? Christian. > --- > drivers/dma-buf/dma-resv.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 709002515550..a05ff542be22 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -34,6 +34,7 @@ > > #include <linux/dma-resv.h> > #include <linux/export.h> > +#include <linux/sched/mm.h> > > /** > * DOC: Reservation Object Overview > @@ -95,6 +96,29 @@ static void dma_resv_list_free(struct dma_resv_list *list) > kfree_rcu(list, rcu); > } > > +#if IS_ENABLED(CONFIG_LOCKDEP) > +static void __init dma_resv_lockdep(void) > +{ > + struct mm_struct *mm = mm_alloc(); > + struct dma_resv obj; > + > + if (!mm) > + return; > + > + dma_resv_init(&obj); > + > + down_read(&mm->mmap_sem); > + ww_mutex_lock(&obj.lock, NULL); > + fs_reclaim_acquire(GFP_KERNEL); > + fs_reclaim_release(GFP_KERNEL); > + ww_mutex_unlock(&obj.lock); > + up_read(&mm->mmap_sem); > + > + mmput(mm); > +} > +subsys_initcall(dma_resv_lockdep); > +#endif > + > /** > * dma_resv_init - initialize a reservation object > * @obj: the reservation object
On Mon, Nov 04, 2019 at 08:01:09PM +0000, Koenig, Christian wrote: > Am 04.11.19 um 18:37 schrieb Daniel Vetter: > > Full audit of everyone: > > > > - i915, radeon, amdgpu should be clean per their maintainers. > > > > - vram helpers should be fine, they don't do command submission, so > > really no business holding struct_mutex while doing copy_*_user. But > > I haven't checked them all. > > > > - panfrost seems to dma_resv_lock only in panfrost_job_push, which > > looks clean. > > > > - v3d holds dma_resv locks in the tail of its v3d_submit_cl_ioctl(), > > copying from/to userspace happens all in v3d_lookup_bos which is > > outside of the critical section. > > > > - vmwgfx has a bunch of ioctls that do their own copy_*_user: > > - vmw_execbuf_process: First this does some copies in > > vmw_execbuf_cmdbuf() and also in the vmw_execbuf_process() itself. > > Then comes the usual ttm reserve/validate sequence, then actual > > submission/fencing, then unreserving, and finally some more > > copy_to_user in vmw_execbuf_copy_fence_user. Glossing over tons of > > details, but looks all safe. > > - vmw_fence_event_ioctl: No ttm_reserve/dma_resv_lock anywhere to be > > seen, seems to only create a fence and copy it out. > > - a pile of smaller ioctl in vmwgfx_ioctl.c, no reservations to be > > found there. > > Summary: vmwgfx seems to be fine too. > > > > - virtio: There's virtio_gpu_execbuffer_ioctl, which does all the > > copying from userspace before even looking up objects through their > > handles, so safe. Plus the getparam/getcaps ioctl, also both safe. > > > > - qxl only has qxl_execbuffer_ioctl, which calls into > > qxl_process_single_command. There's a lovely comment before the > > __copy_from_user_inatomic that the slowpath should be copied from > > i915, but I guess that never happened. Try not to be unlucky and get > > your CS data evicted between when it's written and the kernel tries > > to read it. The only other copy_from_user is for relocs, but those > > are done before qxl_release_reserve_list(), which seems to be the > > only thing reserving buffers (in the ttm/dma_resv sense) in that > > code. So looks safe. > > > > - A debugfs file in nouveau_debugfs_pstate_set() and the usif ioctl in > > usif_ioctl() look safe. nouveau_gem_ioctl_pushbuf() otoh breaks this > > everywhere and needs to be fixed up. > > > > v2: Thomas pointed at that vmwgfx calls dma_resv_init while it holds a > > dma_resv lock of a different object already. Christian mentioned that > > ttm core does this too for ghost objects. intel-gfx-ci highlighted > > that i915 has similar issues. > > > > Unfortunately we can't do this in the usual module init functions, > > because kernel threads don't have an ->mm - we have to wait around for > > some user thread to do this. > > > > Solution is to spawn a worker (but only once). It's horrible, but it > > works. > > > > v3: We can allocate mm! (Chris). Horrible worker hack out, clean > > initcall solution in. > > > > v4: Annotate with __init (Rob Herring) > > > > Cc: Rob Herring <robh@kernel.org> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > Cc: Christian König <christian.koenig@amd.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: Rob Herring <robh@kernel.org> > > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > Cc: Eric Anholt <eric@anholt.net> > > Cc: Dave Airlie <airlied@redhat.com> > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Cc: Ben Skeggs <bskeggs@redhat.com> > > Cc: "VMware Graphics" <linux-graphics-maintainer@vmware.com> > > Cc: Thomas Hellstrom <thellstrom@vmware.com> > > Reviewed-by: Christian König <christian.koenig@amd.com> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Tested-by: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > What's holding you back to commit that? The nouveau patch, can't push this one without also fixing nouveau :-/ -Daniel > > Christian. > > > --- > > drivers/dma-buf/dma-resv.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > > index 709002515550..a05ff542be22 100644 > > --- a/drivers/dma-buf/dma-resv.c > > +++ b/drivers/dma-buf/dma-resv.c > > @@ -34,6 +34,7 @@ > > > > #include <linux/dma-resv.h> > > #include <linux/export.h> > > +#include <linux/sched/mm.h> > > > > /** > > * DOC: Reservation Object Overview > > @@ -95,6 +96,29 @@ static void dma_resv_list_free(struct dma_resv_list *list) > > kfree_rcu(list, rcu); > > } > > > > +#if IS_ENABLED(CONFIG_LOCKDEP) > > +static void __init dma_resv_lockdep(void) > > +{ > > + struct mm_struct *mm = mm_alloc(); > > + struct dma_resv obj; > > + > > + if (!mm) > > + return; > > + > > + dma_resv_init(&obj); > > + > > + down_read(&mm->mmap_sem); > > + ww_mutex_lock(&obj.lock, NULL); > > + fs_reclaim_acquire(GFP_KERNEL); > > + fs_reclaim_release(GFP_KERNEL); > > + ww_mutex_unlock(&obj.lock); > > + up_read(&mm->mmap_sem); > > + > > + mmput(mm); > > +} > > +subsys_initcall(dma_resv_lockdep); > > +#endif > > + > > /** > > * dma_resv_init - initialize a reservation object > > * @obj: the reservation object > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 04/11/2019 17:37, Daniel Vetter wrote: > Full audit of everyone: > > - i915, radeon, amdgpu should be clean per their maintainers. > > - vram helpers should be fine, they don't do command submission, so > really no business holding struct_mutex while doing copy_*_user. But > I haven't checked them all. > > - panfrost seems to dma_resv_lock only in panfrost_job_push, which > looks clean. > > - v3d holds dma_resv locks in the tail of its v3d_submit_cl_ioctl(), > copying from/to userspace happens all in v3d_lookup_bos which is > outside of the critical section. > > - vmwgfx has a bunch of ioctls that do their own copy_*_user: > - vmw_execbuf_process: First this does some copies in > vmw_execbuf_cmdbuf() and also in the vmw_execbuf_process() itself. > Then comes the usual ttm reserve/validate sequence, then actual > submission/fencing, then unreserving, and finally some more > copy_to_user in vmw_execbuf_copy_fence_user. Glossing over tons of > details, but looks all safe. > - vmw_fence_event_ioctl: No ttm_reserve/dma_resv_lock anywhere to be > seen, seems to only create a fence and copy it out. > - a pile of smaller ioctl in vmwgfx_ioctl.c, no reservations to be > found there. > Summary: vmwgfx seems to be fine too. > > - virtio: There's virtio_gpu_execbuffer_ioctl, which does all the > copying from userspace before even looking up objects through their > handles, so safe. Plus the getparam/getcaps ioctl, also both safe. > > - qxl only has qxl_execbuffer_ioctl, which calls into > qxl_process_single_command. There's a lovely comment before the > __copy_from_user_inatomic that the slowpath should be copied from > i915, but I guess that never happened. Try not to be unlucky and get > your CS data evicted between when it's written and the kernel tries > to read it. The only other copy_from_user is for relocs, but those > are done before qxl_release_reserve_list(), which seems to be the > only thing reserving buffers (in the ttm/dma_resv sense) in that > code. So looks safe. > > - A debugfs file in nouveau_debugfs_pstate_set() and the usif ioctl in > usif_ioctl() look safe. nouveau_gem_ioctl_pushbuf() otoh breaks this > everywhere and needs to be fixed up. > > v2: Thomas pointed at that vmwgfx calls dma_resv_init while it holds a > dma_resv lock of a different object already. Christian mentioned that > ttm core does this too for ghost objects. intel-gfx-ci highlighted > that i915 has similar issues. > > Unfortunately we can't do this in the usual module init functions, > because kernel threads don't have an ->mm - we have to wait around for > some user thread to do this. > > Solution is to spawn a worker (but only once). It's horrible, but it > works. > > v3: We can allocate mm! (Chris). Horrible worker hack out, clean > initcall solution in. > > v4: Annotate with __init (Rob Herring) > > Cc: Rob Herring <robh@kernel.org> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Christian König <christian.koenig@amd.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Rob Herring <robh@kernel.org> > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Eric Anholt <eric@anholt.net> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Ben Skeggs <bskeggs@redhat.com> > Cc: "VMware Graphics" <linux-graphics-maintainer@vmware.com> > Cc: Thomas Hellstrom <thellstrom@vmware.com> > Reviewed-by: Christian König <christian.koenig@amd.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > Tested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/dma-buf/dma-resv.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 709002515550..a05ff542be22 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -34,6 +34,7 @@ > > #include <linux/dma-resv.h> > #include <linux/export.h> > +#include <linux/sched/mm.h> > > /** > * DOC: Reservation Object Overview > @@ -95,6 +96,29 @@ static void dma_resv_list_free(struct dma_resv_list *list) > kfree_rcu(list, rcu); > } > > +#if IS_ENABLED(CONFIG_LOCKDEP) > +static void __init dma_resv_lockdep(void) > +{ > + struct mm_struct *mm = mm_alloc(); > + struct dma_resv obj; > + > + if (!mm) > + return; > + > + dma_resv_init(&obj); > + > + down_read(&mm->mmap_sem); > + ww_mutex_lock(&obj.lock, NULL); > + fs_reclaim_acquire(GFP_KERNEL); > + fs_reclaim_release(GFP_KERNEL); > + ww_mutex_unlock(&obj.lock); > + up_read(&mm->mmap_sem); > + Nit: trailing whitespace > + mmput(mm); > +} > +subsys_initcall(dma_resv_lockdep); This expects a function returning int, but dma_resv_lockdep() is void. Causing: drivers/dma-buf/dma-resv.c:119:17: error: initialization of ‘initcall_t’ {aka ‘int (*)(void)’} from incompatible pointer type ‘void (*)(void)’ [-Werror=incompatible-pointer-types] subsys_initcall(dma_resv_lockdep); The below fixes it for me. Steve ----8<---- From d07ea81611ed6e4fb8cc290f42d23dbcca2da2f8 Mon Sep 17 00:00:00 2001 From: Steven Price <steven.price@arm.com> Date: Mon, 11 Nov 2019 13:07:19 +0000 Subject: [PATCH] dma_resv: Correct return type of dma_resv_lockdep() subsys_initcall() expects a function which returns 'int'. Fix dma_resv_lockdep() so it returns an 'int' error code. Fixes: b2a8116e2592 ("dma_resv: prime lockdep annotations") Signed-off-by: Steven Price <steven.price@arm.com> --- drivers/dma-buf/dma-resv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index a05ff542be22..9918a6e5cf91 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -97,13 +97,13 @@ static void dma_resv_list_free(struct dma_resv_list *list) } #if IS_ENABLED(CONFIG_LOCKDEP) -static void __init dma_resv_lockdep(void) +static int __init dma_resv_lockdep(void) { struct mm_struct *mm = mm_alloc(); struct dma_resv obj; if (!mm) - return; + return -ENOMEM; dma_resv_init(&obj); @@ -115,6 +115,8 @@ static void __init dma_resv_lockdep(void) up_read(&mm->mmap_sem); mmput(mm); + + return 0; } subsys_initcall(dma_resv_lockdep); #endif
On Mon, Nov 11, 2019 at 2:11 PM Steven Price <steven.price@arm.com> wrote: > > On 04/11/2019 17:37, Daniel Vetter wrote: > > Full audit of everyone: > > > > - i915, radeon, amdgpu should be clean per their maintainers. > > > > - vram helpers should be fine, they don't do command submission, so > > really no business holding struct_mutex while doing copy_*_user. But > > I haven't checked them all. > > > > - panfrost seems to dma_resv_lock only in panfrost_job_push, which > > looks clean. > > > > - v3d holds dma_resv locks in the tail of its v3d_submit_cl_ioctl(), > > copying from/to userspace happens all in v3d_lookup_bos which is > > outside of the critical section. > > > > - vmwgfx has a bunch of ioctls that do their own copy_*_user: > > - vmw_execbuf_process: First this does some copies in > > vmw_execbuf_cmdbuf() and also in the vmw_execbuf_process() itself. > > Then comes the usual ttm reserve/validate sequence, then actual > > submission/fencing, then unreserving, and finally some more > > copy_to_user in vmw_execbuf_copy_fence_user. Glossing over tons of > > details, but looks all safe. > > - vmw_fence_event_ioctl: No ttm_reserve/dma_resv_lock anywhere to be > > seen, seems to only create a fence and copy it out. > > - a pile of smaller ioctl in vmwgfx_ioctl.c, no reservations to be > > found there. > > Summary: vmwgfx seems to be fine too. > > > > - virtio: There's virtio_gpu_execbuffer_ioctl, which does all the > > copying from userspace before even looking up objects through their > > handles, so safe. Plus the getparam/getcaps ioctl, also both safe. > > > > - qxl only has qxl_execbuffer_ioctl, which calls into > > qxl_process_single_command. There's a lovely comment before the > > __copy_from_user_inatomic that the slowpath should be copied from > > i915, but I guess that never happened. Try not to be unlucky and get > > your CS data evicted between when it's written and the kernel tries > > to read it. The only other copy_from_user is for relocs, but those > > are done before qxl_release_reserve_list(), which seems to be the > > only thing reserving buffers (in the ttm/dma_resv sense) in that > > code. So looks safe. > > > > - A debugfs file in nouveau_debugfs_pstate_set() and the usif ioctl in > > usif_ioctl() look safe. nouveau_gem_ioctl_pushbuf() otoh breaks this > > everywhere and needs to be fixed up. > > > > v2: Thomas pointed at that vmwgfx calls dma_resv_init while it holds a > > dma_resv lock of a different object already. Christian mentioned that > > ttm core does this too for ghost objects. intel-gfx-ci highlighted > > that i915 has similar issues. > > > > Unfortunately we can't do this in the usual module init functions, > > because kernel threads don't have an ->mm - we have to wait around for > > some user thread to do this. > > > > Solution is to spawn a worker (but only once). It's horrible, but it > > works. > > > > v3: We can allocate mm! (Chris). Horrible worker hack out, clean > > initcall solution in. > > > > v4: Annotate with __init (Rob Herring) > > > > Cc: Rob Herring <robh@kernel.org> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > Cc: Christian König <christian.koenig@amd.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: Rob Herring <robh@kernel.org> > > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > Cc: Eric Anholt <eric@anholt.net> > > Cc: Dave Airlie <airlied@redhat.com> > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Cc: Ben Skeggs <bskeggs@redhat.com> > > Cc: "VMware Graphics" <linux-graphics-maintainer@vmware.com> > > Cc: Thomas Hellstrom <thellstrom@vmware.com> > > Reviewed-by: Christian König <christian.koenig@amd.com> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Tested-by: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/dma-buf/dma-resv.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > > index 709002515550..a05ff542be22 100644 > > --- a/drivers/dma-buf/dma-resv.c > > +++ b/drivers/dma-buf/dma-resv.c > > @@ -34,6 +34,7 @@ > > > > #include <linux/dma-resv.h> > > #include <linux/export.h> > > +#include <linux/sched/mm.h> > > > > /** > > * DOC: Reservation Object Overview > > @@ -95,6 +96,29 @@ static void dma_resv_list_free(struct dma_resv_list *list) > > kfree_rcu(list, rcu); > > } > > > > +#if IS_ENABLED(CONFIG_LOCKDEP) > > +static void __init dma_resv_lockdep(void) > > +{ > > + struct mm_struct *mm = mm_alloc(); > > + struct dma_resv obj; > > + > > + if (!mm) > > + return; > > + > > + dma_resv_init(&obj); > > + > > + down_read(&mm->mmap_sem); > > + ww_mutex_lock(&obj.lock, NULL); > > + fs_reclaim_acquire(GFP_KERNEL); > > + fs_reclaim_release(GFP_KERNEL); > > + ww_mutex_unlock(&obj.lock); > > + up_read(&mm->mmap_sem); > > + > > Nit: trailing whitespace > > > + mmput(mm); > > +} > > +subsys_initcall(dma_resv_lockdep); > > This expects a function returning int, but dma_resv_lockdep() is void. > Causing: > > drivers/dma-buf/dma-resv.c:119:17: error: initialization of ‘initcall_t’ > {aka ‘int (*)(void)’} from incompatible pointer type ‘void (*)(void)’ > [-Werror=incompatible-pointer-types] > subsys_initcall(dma_resv_lockdep); > > The below fixes it for me. Uh, so _that_ was what the 0day thing was all about, I totally misread that completely. Thanks for the patch. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Aside, do you need commit rights for pushing this kind of stuff? -Daniel > > Steve > > ----8<---- > From d07ea81611ed6e4fb8cc290f42d23dbcca2da2f8 Mon Sep 17 00:00:00 2001 > From: Steven Price <steven.price@arm.com> > Date: Mon, 11 Nov 2019 13:07:19 +0000 > Subject: [PATCH] dma_resv: Correct return type of dma_resv_lockdep() > > subsys_initcall() expects a function which returns 'int'. Fix > dma_resv_lockdep() so it returns an 'int' error code. > > Fixes: b2a8116e2592 ("dma_resv: prime lockdep annotations") > Signed-off-by: Steven Price <steven.price@arm.com> > --- > drivers/dma-buf/dma-resv.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index a05ff542be22..9918a6e5cf91 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -97,13 +97,13 @@ static void dma_resv_list_free(struct dma_resv_list *list) > } > > #if IS_ENABLED(CONFIG_LOCKDEP) > -static void __init dma_resv_lockdep(void) > +static int __init dma_resv_lockdep(void) > { > struct mm_struct *mm = mm_alloc(); > struct dma_resv obj; > > if (!mm) > - return; > + return -ENOMEM; > > dma_resv_init(&obj); > > @@ -115,6 +115,8 @@ static void __init dma_resv_lockdep(void) > up_read(&mm->mmap_sem); > > mmput(mm); > + > + return 0; > } > subsys_initcall(dma_resv_lockdep); > #endif > -- > 2.20.1 >
On 11/11/2019 15:42, Daniel Vetter wrote: > On Mon, Nov 11, 2019 at 2:11 PM Steven Price <steven.price@arm.com> wrote: >> >> On 04/11/2019 17:37, Daniel Vetter wrote: >>> Full audit of everyone: >>> >>> - i915, radeon, amdgpu should be clean per their maintainers. >>> >>> - vram helpers should be fine, they don't do command submission, so >>> really no business holding struct_mutex while doing copy_*_user. But >>> I haven't checked them all. >>> >>> - panfrost seems to dma_resv_lock only in panfrost_job_push, which >>> looks clean. >>> >>> - v3d holds dma_resv locks in the tail of its v3d_submit_cl_ioctl(), >>> copying from/to userspace happens all in v3d_lookup_bos which is >>> outside of the critical section. >>> >>> - vmwgfx has a bunch of ioctls that do their own copy_*_user: >>> - vmw_execbuf_process: First this does some copies in >>> vmw_execbuf_cmdbuf() and also in the vmw_execbuf_process() itself. >>> Then comes the usual ttm reserve/validate sequence, then actual >>> submission/fencing, then unreserving, and finally some more >>> copy_to_user in vmw_execbuf_copy_fence_user. Glossing over tons of >>> details, but looks all safe. >>> - vmw_fence_event_ioctl: No ttm_reserve/dma_resv_lock anywhere to be >>> seen, seems to only create a fence and copy it out. >>> - a pile of smaller ioctl in vmwgfx_ioctl.c, no reservations to be >>> found there. >>> Summary: vmwgfx seems to be fine too. >>> >>> - virtio: There's virtio_gpu_execbuffer_ioctl, which does all the >>> copying from userspace before even looking up objects through their >>> handles, so safe. Plus the getparam/getcaps ioctl, also both safe. >>> >>> - qxl only has qxl_execbuffer_ioctl, which calls into >>> qxl_process_single_command. There's a lovely comment before the >>> __copy_from_user_inatomic that the slowpath should be copied from >>> i915, but I guess that never happened. Try not to be unlucky and get >>> your CS data evicted between when it's written and the kernel tries >>> to read it. The only other copy_from_user is for relocs, but those >>> are done before qxl_release_reserve_list(), which seems to be the >>> only thing reserving buffers (in the ttm/dma_resv sense) in that >>> code. So looks safe. >>> >>> - A debugfs file in nouveau_debugfs_pstate_set() and the usif ioctl in >>> usif_ioctl() look safe. nouveau_gem_ioctl_pushbuf() otoh breaks this >>> everywhere and needs to be fixed up. >>> >>> v2: Thomas pointed at that vmwgfx calls dma_resv_init while it holds a >>> dma_resv lock of a different object already. Christian mentioned that >>> ttm core does this too for ghost objects. intel-gfx-ci highlighted >>> that i915 has similar issues. >>> >>> Unfortunately we can't do this in the usual module init functions, >>> because kernel threads don't have an ->mm - we have to wait around for >>> some user thread to do this. >>> >>> Solution is to spawn a worker (but only once). It's horrible, but it >>> works. >>> >>> v3: We can allocate mm! (Chris). Horrible worker hack out, clean >>> initcall solution in. >>> >>> v4: Annotate with __init (Rob Herring) >>> >>> Cc: Rob Herring <robh@kernel.org> >>> Cc: Alex Deucher <alexander.deucher@amd.com> >>> Cc: Christian König <christian.koenig@amd.com> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Thomas Zimmermann <tzimmermann@suse.de> >>> Cc: Rob Herring <robh@kernel.org> >>> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> >>> Cc: Eric Anholt <eric@anholt.net> >>> Cc: Dave Airlie <airlied@redhat.com> >>> Cc: Gerd Hoffmann <kraxel@redhat.com> >>> Cc: Ben Skeggs <bskeggs@redhat.com> >>> Cc: "VMware Graphics" <linux-graphics-maintainer@vmware.com> >>> Cc: Thomas Hellstrom <thellstrom@vmware.com> >>> Reviewed-by: Christian König <christian.koenig@amd.com> >>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Tested-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >>> --- >>> drivers/dma-buf/dma-resv.c | 24 ++++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c >>> index 709002515550..a05ff542be22 100644 >>> --- a/drivers/dma-buf/dma-resv.c >>> +++ b/drivers/dma-buf/dma-resv.c >>> @@ -34,6 +34,7 @@ >>> >>> #include <linux/dma-resv.h> >>> #include <linux/export.h> >>> +#include <linux/sched/mm.h> >>> >>> /** >>> * DOC: Reservation Object Overview >>> @@ -95,6 +96,29 @@ static void dma_resv_list_free(struct dma_resv_list *list) >>> kfree_rcu(list, rcu); >>> } >>> >>> +#if IS_ENABLED(CONFIG_LOCKDEP) >>> +static void __init dma_resv_lockdep(void) >>> +{ >>> + struct mm_struct *mm = mm_alloc(); >>> + struct dma_resv obj; >>> + >>> + if (!mm) >>> + return; >>> + >>> + dma_resv_init(&obj); >>> + >>> + down_read(&mm->mmap_sem); >>> + ww_mutex_lock(&obj.lock, NULL); >>> + fs_reclaim_acquire(GFP_KERNEL); >>> + fs_reclaim_release(GFP_KERNEL); >>> + ww_mutex_unlock(&obj.lock); >>> + up_read(&mm->mmap_sem); >>> + >> >> Nit: trailing whitespace >> >>> + mmput(mm); >>> +} >>> +subsys_initcall(dma_resv_lockdep); >> >> This expects a function returning int, but dma_resv_lockdep() is void. >> Causing: >> >> drivers/dma-buf/dma-resv.c:119:17: error: initialization of ‘initcall_t’ >> {aka ‘int (*)(void)’} from incompatible pointer type ‘void (*)(void)’ >> [-Werror=incompatible-pointer-types] >> subsys_initcall(dma_resv_lockdep); >> >> The below fixes it for me. > > Uh, so _that_ was what the 0day thing was all about, I totally misread > that completely. Thanks for the patch. > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Aside, do you need commit rights for pushing this kind of stuff? I guess it's about time I got round to requesting that: https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/208 Thanks, Steve
On Thu, Nov 14, 2019 at 11:50:28AM +0000, Steven Price wrote: > On 11/11/2019 15:42, Daniel Vetter wrote: > > On Mon, Nov 11, 2019 at 2:11 PM Steven Price <steven.price@arm.com> wrote: > >> > >> On 04/11/2019 17:37, Daniel Vetter wrote: > >>> Full audit of everyone: > >>> > >>> - i915, radeon, amdgpu should be clean per their maintainers. > >>> > >>> - vram helpers should be fine, they don't do command submission, so > >>> really no business holding struct_mutex while doing copy_*_user. But > >>> I haven't checked them all. > >>> > >>> - panfrost seems to dma_resv_lock only in panfrost_job_push, which > >>> looks clean. > >>> > >>> - v3d holds dma_resv locks in the tail of its v3d_submit_cl_ioctl(), > >>> copying from/to userspace happens all in v3d_lookup_bos which is > >>> outside of the critical section. > >>> > >>> - vmwgfx has a bunch of ioctls that do their own copy_*_user: > >>> - vmw_execbuf_process: First this does some copies in > >>> vmw_execbuf_cmdbuf() and also in the vmw_execbuf_process() itself. > >>> Then comes the usual ttm reserve/validate sequence, then actual > >>> submission/fencing, then unreserving, and finally some more > >>> copy_to_user in vmw_execbuf_copy_fence_user. Glossing over tons of > >>> details, but looks all safe. > >>> - vmw_fence_event_ioctl: No ttm_reserve/dma_resv_lock anywhere to be > >>> seen, seems to only create a fence and copy it out. > >>> - a pile of smaller ioctl in vmwgfx_ioctl.c, no reservations to be > >>> found there. > >>> Summary: vmwgfx seems to be fine too. > >>> > >>> - virtio: There's virtio_gpu_execbuffer_ioctl, which does all the > >>> copying from userspace before even looking up objects through their > >>> handles, so safe. Plus the getparam/getcaps ioctl, also both safe. > >>> > >>> - qxl only has qxl_execbuffer_ioctl, which calls into > >>> qxl_process_single_command. There's a lovely comment before the > >>> __copy_from_user_inatomic that the slowpath should be copied from > >>> i915, but I guess that never happened. Try not to be unlucky and get > >>> your CS data evicted between when it's written and the kernel tries > >>> to read it. The only other copy_from_user is for relocs, but those > >>> are done before qxl_release_reserve_list(), which seems to be the > >>> only thing reserving buffers (in the ttm/dma_resv sense) in that > >>> code. So looks safe. > >>> > >>> - A debugfs file in nouveau_debugfs_pstate_set() and the usif ioctl in > >>> usif_ioctl() look safe. nouveau_gem_ioctl_pushbuf() otoh breaks this > >>> everywhere and needs to be fixed up. > >>> > >>> v2: Thomas pointed at that vmwgfx calls dma_resv_init while it holds a > >>> dma_resv lock of a different object already. Christian mentioned that > >>> ttm core does this too for ghost objects. intel-gfx-ci highlighted > >>> that i915 has similar issues. > >>> > >>> Unfortunately we can't do this in the usual module init functions, > >>> because kernel threads don't have an ->mm - we have to wait around for > >>> some user thread to do this. > >>> > >>> Solution is to spawn a worker (but only once). It's horrible, but it > >>> works. > >>> > >>> v3: We can allocate mm! (Chris). Horrible worker hack out, clean > >>> initcall solution in. > >>> > >>> v4: Annotate with __init (Rob Herring) > >>> > >>> Cc: Rob Herring <robh@kernel.org> > >>> Cc: Alex Deucher <alexander.deucher@amd.com> > >>> Cc: Christian König <christian.koenig@amd.com> > >>> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >>> Cc: Thomas Zimmermann <tzimmermann@suse.de> > >>> Cc: Rob Herring <robh@kernel.org> > >>> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > >>> Cc: Eric Anholt <eric@anholt.net> > >>> Cc: Dave Airlie <airlied@redhat.com> > >>> Cc: Gerd Hoffmann <kraxel@redhat.com> > >>> Cc: Ben Skeggs <bskeggs@redhat.com> > >>> Cc: "VMware Graphics" <linux-graphics-maintainer@vmware.com> > >>> Cc: Thomas Hellstrom <thellstrom@vmware.com> > >>> Reviewed-by: Christian König <christian.koenig@amd.com> > >>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > >>> Tested-by: Chris Wilson <chris@chris-wilson.co.uk> > >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > >>> --- > >>> drivers/dma-buf/dma-resv.c | 24 ++++++++++++++++++++++++ > >>> 1 file changed, 24 insertions(+) > >>> > >>> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > >>> index 709002515550..a05ff542be22 100644 > >>> --- a/drivers/dma-buf/dma-resv.c > >>> +++ b/drivers/dma-buf/dma-resv.c > >>> @@ -34,6 +34,7 @@ > >>> > >>> #include <linux/dma-resv.h> > >>> #include <linux/export.h> > >>> +#include <linux/sched/mm.h> > >>> > >>> /** > >>> * DOC: Reservation Object Overview > >>> @@ -95,6 +96,29 @@ static void dma_resv_list_free(struct dma_resv_list *list) > >>> kfree_rcu(list, rcu); > >>> } > >>> > >>> +#if IS_ENABLED(CONFIG_LOCKDEP) > >>> +static void __init dma_resv_lockdep(void) > >>> +{ > >>> + struct mm_struct *mm = mm_alloc(); > >>> + struct dma_resv obj; > >>> + > >>> + if (!mm) > >>> + return; > >>> + > >>> + dma_resv_init(&obj); > >>> + > >>> + down_read(&mm->mmap_sem); > >>> + ww_mutex_lock(&obj.lock, NULL); > >>> + fs_reclaim_acquire(GFP_KERNEL); > >>> + fs_reclaim_release(GFP_KERNEL); > >>> + ww_mutex_unlock(&obj.lock); > >>> + up_read(&mm->mmap_sem); > >>> + > >> > >> Nit: trailing whitespace > >> > >>> + mmput(mm); > >>> +} > >>> +subsys_initcall(dma_resv_lockdep); > >> > >> This expects a function returning int, but dma_resv_lockdep() is void. > >> Causing: > >> > >> drivers/dma-buf/dma-resv.c:119:17: error: initialization of ‘initcall_t’ > >> {aka ‘int (*)(void)’} from incompatible pointer type ‘void (*)(void)’ > >> [-Werror=incompatible-pointer-types] > >> subsys_initcall(dma_resv_lockdep); > >> > >> The below fixes it for me. > > > > Uh, so _that_ was what the 0day thing was all about, I totally misread > > that completely. Thanks for the patch. > > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Aside, do you need commit rights for pushing this kind of stuff? > > I guess it's about time I got round to requesting that: > > https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/208 Since this seems a bit stuck in processing I went ahead and merged your fix meanwhile. Thanks, Daniel
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 709002515550..a05ff542be22 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -34,6 +34,7 @@ #include <linux/dma-resv.h> #include <linux/export.h> +#include <linux/sched/mm.h> /** * DOC: Reservation Object Overview @@ -95,6 +96,29 @@ static void dma_resv_list_free(struct dma_resv_list *list) kfree_rcu(list, rcu); } +#if IS_ENABLED(CONFIG_LOCKDEP) +static void __init dma_resv_lockdep(void) +{ + struct mm_struct *mm = mm_alloc(); + struct dma_resv obj; + + if (!mm) + return; + + dma_resv_init(&obj); + + down_read(&mm->mmap_sem); + ww_mutex_lock(&obj.lock, NULL); + fs_reclaim_acquire(GFP_KERNEL); + fs_reclaim_release(GFP_KERNEL); + ww_mutex_unlock(&obj.lock); + up_read(&mm->mmap_sem); + + mmput(mm); +} +subsys_initcall(dma_resv_lockdep); +#endif + /** * dma_resv_init - initialize a reservation object * @obj: the reservation object
Full audit of everyone: - i915, radeon, amdgpu should be clean per their maintainers. - vram helpers should be fine, they don't do command submission, so really no business holding struct_mutex while doing copy_*_user. But I haven't checked them all. - panfrost seems to dma_resv_lock only in panfrost_job_push, which looks clean. - v3d holds dma_resv locks in the tail of its v3d_submit_cl_ioctl(), copying from/to userspace happens all in v3d_lookup_bos which is outside of the critical section. - vmwgfx has a bunch of ioctls that do their own copy_*_user: - vmw_execbuf_process: First this does some copies in vmw_execbuf_cmdbuf() and also in the vmw_execbuf_process() itself. Then comes the usual ttm reserve/validate sequence, then actual submission/fencing, then unreserving, and finally some more copy_to_user in vmw_execbuf_copy_fence_user. Glossing over tons of details, but looks all safe. - vmw_fence_event_ioctl: No ttm_reserve/dma_resv_lock anywhere to be seen, seems to only create a fence and copy it out. - a pile of smaller ioctl in vmwgfx_ioctl.c, no reservations to be found there. Summary: vmwgfx seems to be fine too. - virtio: There's virtio_gpu_execbuffer_ioctl, which does all the copying from userspace before even looking up objects through their handles, so safe. Plus the getparam/getcaps ioctl, also both safe. - qxl only has qxl_execbuffer_ioctl, which calls into qxl_process_single_command. There's a lovely comment before the __copy_from_user_inatomic that the slowpath should be copied from i915, but I guess that never happened. Try not to be unlucky and get your CS data evicted between when it's written and the kernel tries to read it. The only other copy_from_user is for relocs, but those are done before qxl_release_reserve_list(), which seems to be the only thing reserving buffers (in the ttm/dma_resv sense) in that code. So looks safe. - A debugfs file in nouveau_debugfs_pstate_set() and the usif ioctl in usif_ioctl() look safe. nouveau_gem_ioctl_pushbuf() otoh breaks this everywhere and needs to be fixed up. v2: Thomas pointed at that vmwgfx calls dma_resv_init while it holds a dma_resv lock of a different object already. Christian mentioned that ttm core does this too for ghost objects. intel-gfx-ci highlighted that i915 has similar issues. Unfortunately we can't do this in the usual module init functions, because kernel threads don't have an ->mm - we have to wait around for some user thread to do this. Solution is to spawn a worker (but only once). It's horrible, but it works. v3: We can allocate mm! (Chris). Horrible worker hack out, clean initcall solution in. v4: Annotate with __init (Rob Herring) Cc: Rob Herring <robh@kernel.org> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: Christian König <christian.koenig@amd.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: Rob Herring <robh@kernel.org> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> Cc: Eric Anholt <eric@anholt.net> Cc: Dave Airlie <airlied@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Ben Skeggs <bskeggs@redhat.com> Cc: "VMware Graphics" <linux-graphics-maintainer@vmware.com> Cc: Thomas Hellstrom <thellstrom@vmware.com> Reviewed-by: Christian König <christian.koenig@amd.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Tested-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/dma-buf/dma-resv.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)