* [PATCH 1/3] lib: Always build and export interval_tree
@ 2014-01-21 15:07 Chris Wilson
2014-01-21 15:07 ` [PATCH 2/3] drm/i915: Do not call retire_requests from wait_for_rendering Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Chris Wilson @ 2014-01-21 15:07 UTC (permalink / raw)
To: intel-gfx
Cc: Andrea Arcangeli, Rik van Riel, Peter Zijlstra, Akash Goel,
Andrew Morton, Michel Lespinasse
lib/interval_tree.c provides a simple interface for an interval-tree
(an augmented red-black tree) but is only built when testing the generic
macros for building interval-trees. For drivers with modest needs,
export the simple interval-tree library as is.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Lespinasse <walken@google.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
lib/Makefile | 5 +++--
lib/interval_tree.c | 5 +++++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/Makefile b/lib/Makefile
index a459c31e8c6b..6b9cc0c4cc36 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -8,7 +8,8 @@ KBUILD_CFLAGS = $(subst -pg,,$(ORIG_CFLAGS))
endif
lib-y := ctype.o string.o vsprintf.o cmdline.o \
- rbtree.o radix-tree.o dump_stack.o timerqueue.o\
+ rbtree.o radix-tree.o interval_tree.o \
+ dump_stack.o timerqueue.o\
idr.o int_sqrt.o extable.o \
sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
proportions.o flex_proportions.o prio_heap.o ratelimit.o show_mem.o \
@@ -152,7 +153,7 @@ lib-$(CONFIG_LIBFDT) += $(libfdt_files)
obj-$(CONFIG_RBTREE_TEST) += rbtree_test.o
obj-$(CONFIG_INTERVAL_TREE_TEST) += interval_tree_test.o
-interval_tree_test-objs := interval_tree_test_main.o interval_tree.o
+interval_tree_test-objs := interval_tree_test_main.o
obj-$(CONFIG_PERCPU_TEST) += percpu_test.o
diff --git a/lib/interval_tree.c b/lib/interval_tree.c
index e6eb406f2d65..7b466e419740 100644
--- a/lib/interval_tree.c
+++ b/lib/interval_tree.c
@@ -8,3 +8,8 @@
INTERVAL_TREE_DEFINE(struct interval_tree_node, rb,
unsigned long, __subtree_last,
START, LAST,, interval_tree)
+
+EXPORT_SYMBOL(interval_tree_insert);
+EXPORT_SYMBOL(interval_tree_remove);
+EXPORT_SYMBOL(interval_tree_iter_first);
+EXPORT_SYMBOL(interval_tree_iter_next);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/3] drm/i915: Do not call retire_requests from wait_for_rendering 2014-01-21 15:07 [PATCH 1/3] lib: Always build and export interval_tree Chris Wilson @ 2014-01-21 15:07 ` Chris Wilson 2014-01-21 15:07 ` [PATCH 3/3] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl Chris Wilson ` (2 subsequent siblings) 3 siblings, 0 replies; 16+ messages in thread From: Chris Wilson @ 2014-01-21 15:07 UTC (permalink / raw) To: intel-gfx; +Cc: Akash Goel A common issue we have is that retiring requests causes recursion through GTT manipulation or page table manipulation which we can only handle at very specific points. However, to maintain internal consistency (enforced through our sanity checks on write_domain at various points in the GEM object lifecycle) we do need to retire the object prior to marking it with a new write_domain, and also clear the write_domain for the implicit flush following a batch. Note that this then allows the unbound objects to still be on the active lists, and so care must be taken when removing objects from unbound lists (similar to the caveats we face processing the bound lists). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem.c | 100 ++++++++++++++++++----------- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 + 2 files changed, 64 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0e8c9bb6c3da..8912aaa7118a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -43,12 +43,15 @@ static void i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *o static __must_check int i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj, bool readonly); +static void +i915_gem_object_retire(struct drm_i915_gem_object *obj); static __must_check int i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, struct i915_address_space *vm, unsigned alignment, bool map_and_fenceable, bool nonblocking); + static int i915_gem_phys_pwrite(struct drm_device *dev, struct drm_i915_gem_object *obj, struct drm_i915_gem_pwrite *args, @@ -535,6 +538,8 @@ i915_gem_shmem_pread(struct drm_device *dev, ret = i915_gem_object_wait_rendering(obj, true); if (ret) return ret; + + i915_gem_object_retire(obj); } ret = i915_gem_object_get_pages(obj); @@ -849,6 +854,8 @@ i915_gem_shmem_pwrite(struct drm_device *dev, ret = i915_gem_object_wait_rendering(obj, false); if (ret) return ret; + + i915_gem_object_retire(obj); } /* Same trick applies to invalidate partially written cachelines read * before writing. */ @@ -1238,7 +1245,8 @@ static int i915_gem_object_wait_rendering__tail(struct drm_i915_gem_object *obj, struct intel_ring_buffer *ring) { - i915_gem_retire_requests_ring(ring); + if (!obj->active) + return 0; /* Manually manage the write flush as we may have not yet * retired the buffer. @@ -1248,7 +1256,6 @@ i915_gem_object_wait_rendering__tail(struct drm_i915_gem_object *obj, * we know we have passed the last write. */ obj->last_write_seqno = 0; - obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS; return 0; } @@ -1856,58 +1863,58 @@ static unsigned long __i915_gem_shrink(struct drm_i915_private *dev_priv, long target, bool purgeable_only) { - struct list_head still_bound_list; - struct drm_i915_gem_object *obj, *next; + struct list_head still_in_list; + struct drm_i915_gem_object *obj; unsigned long count = 0; - list_for_each_entry_safe(obj, next, - &dev_priv->mm.unbound_list, - global_list) { - if ((i915_gem_object_is_purgeable(obj) || !purgeable_only) && - i915_gem_object_put_pages(obj) == 0) { - count += obj->base.size >> PAGE_SHIFT; - if (count >= target) - return count; - } - } - /* - * As we may completely rewrite the bound list whilst unbinding + * As we may completely rewrite the (un)bound list whilst unbinding * (due to retiring requests) we have to strictly process only * one element of the list at the time, and recheck the list * on every iteration. + * + * In particular, we must hold a reference whilst removing the + * object as we may end up waiting for and/or retiring the objects. + * This might release the final reference (held by the active list) + * and result in the object being freed from under us. This is + * similar to the precautions the eviction code must take whilst + * removing objects. + * + * Also note that although these lists do not hold a reference to + * the object we can safely grab one here: The final object + * unreferencing and the bound_list are both protected by the + * dev->struct_mutex and so we won't ever be able to observe an + * object on the bound_list with a reference count equals 0. */ - INIT_LIST_HEAD(&still_bound_list); + INIT_LIST_HEAD(&still_in_list); + while (count < target && !list_empty(&dev_priv->mm.unbound_list)) { + obj = list_first_entry(&dev_priv->mm.unbound_list, + typeof(*obj), global_list); + list_move_tail(&obj->global_list, &still_in_list); + + if (!i915_gem_object_is_purgeable(obj) && purgeable_only) + continue; + + drm_gem_object_reference(&obj->base); + + if (i915_gem_object_put_pages(obj) == 0) + count += obj->base.size >> PAGE_SHIFT; + + drm_gem_object_unreference(&obj->base); + } + list_splice(&still_in_list, &dev_priv->mm.unbound_list); + + INIT_LIST_HEAD(&still_in_list); while (count < target && !list_empty(&dev_priv->mm.bound_list)) { struct i915_vma *vma, *v; obj = list_first_entry(&dev_priv->mm.bound_list, typeof(*obj), global_list); - list_move_tail(&obj->global_list, &still_bound_list); + list_move_tail(&obj->global_list, &still_in_list); if (!i915_gem_object_is_purgeable(obj) && purgeable_only) continue; - /* - * Hold a reference whilst we unbind this object, as we may - * end up waiting for and retiring requests. This might - * release the final reference (held by the active list) - * and result in the object being freed from under us. - * in this object being freed. - * - * Note 1: Shrinking the bound list is special since only active - * (and hence bound objects) can contain such limbo objects, so - * we don't need special tricks for shrinking the unbound list. - * The only other place where we have to be careful with active - * objects suddenly disappearing due to retiring requests is the - * eviction code. - * - * Note 2: Even though the bound list doesn't hold a reference - * to the object we can safely grab one here: The final object - * unreferencing and the bound_list are both protected by the - * dev->struct_mutex and so we won't ever be able to observe an - * object on the bound_list with a reference count equals 0. - */ drm_gem_object_reference(&obj->base); list_for_each_entry_safe(vma, v, &obj->vma_list, vma_link) @@ -1919,7 +1926,7 @@ __i915_gem_shrink(struct drm_i915_private *dev_priv, long target, drm_gem_object_unreference(&obj->base); } - list_splice(&still_bound_list, &dev_priv->mm.bound_list); + list_splice(&still_in_list, &dev_priv->mm.bound_list); return count; } @@ -2160,6 +2167,19 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj) WARN_ON(i915_verify_lists(dev)); } +static void +i915_gem_object_retire(struct drm_i915_gem_object *obj) +{ + struct intel_ring_buffer *ring = obj->ring; + + if (ring == NULL) + return; + + if (i915_seqno_passed(ring->get_seqno(ring, true), + obj->last_read_seqno)) + i915_gem_object_move_to_inactive(obj); +} + static int i915_gem_init_seqno(struct drm_device *dev, u32 seqno) { @@ -3581,6 +3601,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write) if (ret) return ret; + i915_gem_object_retire(obj); i915_gem_object_flush_cpu_write_domain(obj, false); /* Serialise direct access to this object with the barriers for @@ -3901,6 +3922,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write) if (ret) return ret; + i915_gem_object_retire(obj); i915_gem_object_flush_gtt_write_domain(obj); old_write_domain = obj->base.write_domain; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0c6bcff740c2..ce0529d48864 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -953,6 +953,9 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas, if (i915_gem_obj_ggtt_bound(obj) && i915_gem_obj_to_ggtt(obj)->pin_count) intel_mark_fb_busy(obj, ring); + + /* update for the implicit flush after a batch */ + obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS; } trace_i915_gem_object_change_domain(obj, old_read, old_write); -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl 2014-01-21 15:07 [PATCH 1/3] lib: Always build and export interval_tree Chris Wilson 2014-01-21 15:07 ` [PATCH 2/3] drm/i915: Do not call retire_requests from wait_for_rendering Chris Wilson @ 2014-01-21 15:07 ` Chris Wilson 2014-01-22 9:46 ` [PATCH] " Chris Wilson 2014-01-25 4:42 ` [PATCH 1/3] lib: Always build and export interval_tree Michel Lespinasse 2014-01-26 12:24 ` [PATCH] lib: Export interval_tree Chris Wilson 3 siblings, 1 reply; 16+ messages in thread From: Chris Wilson @ 2014-01-21 15:07 UTC (permalink / raw) To: intel-gfx; +Cc: Akash Goel By exporting the ability to map user address and inserting PTEs representing their backing pages into the GTT, we can exploit UMA in order to utilize normal application data as a texture source or even as a render target (depending upon the capabilities of the chipset). This has a number of uses, with zero-copy downloads to the GPU and efficient readback making the intermixed streaming of CPU and GPU operations fairly efficient. This ability has many widespread implications from faster rendering of client-side software rasterisers (chromium), mitigation of stalls due to read back (firefox) and to faster pipelining of texture data (such as pixel buffer objects in GL or data blobs in CL). v2: Compile with CONFIG_MMU_NOTIFIER v3: We can sleep while performing invalidate-range, which we can utilise to drop our page references prior to the kernel manipulating the vma (for either discard or cloning) and so protect normal users. v4: Only run the invalidate notifier if the range intercepts the bo. v5: Prevent userspace from attempting to GTT mmap non-page aligned buffers v6: Recheck after reacquire mutex for lost mmu. v7: Fix implicit padding of ioctl struct by rounding to next 64bit boundary. v8: Fix rebasing error after forwarding porting the back port. v9: Limit the userptr to page aligned entries. We now expect userspace to handle all the offset-in-page adjustments itself. v10: Prevent vma from being copied across fork to avoid issues with cow. v11: Drop vma behaviour changes -- locking is nigh on impossible. Use a worker to load user pages to avoid lock inversions. v12: Use get_task_mm()/mmput() for correct refcounting of mm. v13: Use a worker to release the mmu_notifier to avoid lock inversion v14: Decouple mmu_notifier from struct_mutex using a custom mmu_notifer with its own locking and tree of objects for each mm/mmu_notifier. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: "Gong, Zhipeng" <zhipeng.gong@intel.com> Cc: Akash Goel <akash.goel@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 24 +- drivers/gpu/drm/i915/i915_gem.c | 4 + drivers/gpu/drm/i915/i915_gem_userptr.c | 553 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gpu_error.c | 2 + include/uapi/drm/i915_drm.h | 16 + 7 files changed, 600 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 45071d18c730..1dc14e51f3bd 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -15,6 +15,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ i915_gem_gtt.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ + i915_gem_userptr.o \ i915_sysfs.o \ i915_trace_points.o \ i915_ums.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f053d1099d3b..c34f2e7ae6d9 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1919,6 +1919,7 @@ const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CREATE2, i915_gem_create2_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), }; int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 35a2e339c298..fe46be1c48cc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -40,6 +40,7 @@ #include <linux/i2c-algo-bit.h> #include <drm/intel-gtt.h> #include <linux/backlight.h> +#include <linux/hashtable.h> #include <linux/intel-iommu.h> #include <linux/kref.h> #include <linux/pm_qos.h> @@ -163,6 +164,7 @@ enum hpd_pin { if ((intel_encoder)->base.crtc == (__crtc)) struct drm_i915_private; +struct i915_mmu_notifier; enum intel_dpll_id { DPLL_ID_PRIVATE = -1, /* non-shared dpll in use */ @@ -354,6 +356,7 @@ struct drm_i915_error_state { u32 tiling:2; u32 dirty:1; u32 purgeable:1; + u32 userptr:1; s32 ring:4; u32 cache_level:3; } **active_bo, **pinned_bo; @@ -1465,6 +1468,9 @@ typedef struct drm_i915_private { struct i915_gtt gtt; /* VMA representing the global address space */ struct i915_gem_mm mm; +#if defined(CONFIG_MMU_NOTIFIER) + DECLARE_HASHTABLE(mmu_notifiers, 7); +#endif /* Kernel Modesetting */ @@ -1596,6 +1602,7 @@ struct drm_i915_gem_object_ops { */ int (*get_pages)(struct drm_i915_gem_object *); void (*put_pages)(struct drm_i915_gem_object *); + void (*release)(struct drm_i915_gem_object *); }; struct drm_i915_gem_object { @@ -1709,9 +1716,21 @@ struct drm_i915_gem_object { /** for phy allocated objects */ struct drm_i915_gem_phys_object *phys_obj; + + union { + struct i915_gem_userptr { + uintptr_t ptr; + unsigned read_only :1; + + struct mm_struct *mm; +#if defined(CONFIG_MMU_NOTIFIER) + struct i915_mmu_object *mn; +#endif + struct work_struct *work; + } userptr; + }; }; #define to_gem_object(obj) (&((struct drm_i915_gem_object *)(obj))->base) - #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) /** @@ -2012,6 +2031,9 @@ int i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +int i915_gem_init_userptr(struct drm_device *dev); +int i915_gem_userptr_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_wait_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8912aaa7118a..0d20af63b876 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4380,6 +4380,9 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) if (obj->base.import_attach) drm_prime_gem_destroy(&obj->base, NULL); + if (obj->ops->release) + obj->ops->release(obj); + drm_gem_object_release(&obj->base); i915_gem_info_remove_obj(dev_priv, obj->base.size); @@ -4638,6 +4641,7 @@ int i915_gem_init(struct drm_device *dev) DRM_DEBUG_DRIVER("allow wake ack timed out\n"); } + i915_gem_init_userptr(dev); i915_gem_init_global_gtt(dev); ret = i915_gem_context_init(dev); diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c new file mode 100644 index 000000000000..a92f37df2298 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -0,0 +1,553 @@ +/* + * Copyright © 2012-2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include "drmP.h" +#include "i915_drm.h" +#include "i915_drv.h" +#include "i915_trace.h" +#include "intel_drv.h" +#include <linux/mmu_context.h> +#include <linux/mmu_notifier.h> +#include <linux/mempolicy.h> +#include <linux/swap.h> + +#if defined(CONFIG_MMU_NOTIFIER) +#include <linux/interval_tree.h> + +struct i915_mmu_notifier { + spinlock_t lock; + struct hlist_node node; + struct mmu_notifier mn; + struct rb_root objects; + struct drm_device *dev; + struct mm_struct *mm; + struct work_struct work; + int count; +}; + +struct i915_mmu_object { + struct i915_mmu_notifier *mmu; + struct interval_tree_node it; + struct drm_i915_gem_object *obj; +}; + +static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, + struct mm_struct *mm, + unsigned long start, + unsigned long end) +{ + struct i915_mmu_notifier *mn = container_of(_mn, struct i915_mmu_notifier, mn); + struct interval_tree_node *it; + struct drm_i915_gem_object *obj; + + obj = NULL; + spin_lock(&mn->lock); + it = interval_tree_iter_first(&mn->objects, start, end); + if (it != NULL) { + obj = container_of(it, struct i915_mmu_object, it)->obj; + drm_gem_object_reference(&obj->base); + } + spin_unlock(&mn->lock); + if (obj == NULL) + return; + + mutex_lock(&mn->dev->struct_mutex); + if (obj->pages != NULL) { + struct drm_i915_private *dev_priv = to_i915(mn->dev); + struct i915_vma *vma, *tmp; + bool was_interruptible; + + was_interruptible = dev_priv->mm.interruptible; + dev_priv->mm.interruptible = false; + + list_for_each_entry_safe(vma, tmp, &obj->vma_list, vma_link) { + int ret = i915_vma_unbind(vma); + WARN_ON(ret && ret != -EIO); + } + WARN_ON(i915_gem_object_put_pages(obj)); + + dev_priv->mm.interruptible = was_interruptible; + } + drm_gem_object_unreference(&obj->base); + mutex_unlock(&mn->dev->struct_mutex); +} + +static const struct mmu_notifier_ops i915_gem_userptr_notifier = { + .invalidate_range_start = i915_gem_userptr_mn_invalidate_range_start, +}; + +static struct i915_mmu_notifier * +__i915_mmu_notifier_lookup(struct drm_device *dev, struct mm_struct *mm) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct i915_mmu_notifier *mmu; + + hash_for_each_possible(dev_priv->mmu_notifiers, mmu, node, (unsigned long)mm) + if (mmu->mm == mm) + return mmu; + + return NULL; +} + +static struct i915_mmu_notifier * +i915_mmu_notifier_get(struct drm_device *dev, struct mm_struct *mm) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct i915_mmu_notifier *mmu; + int ret; + + mmu = __i915_mmu_notifier_lookup(dev, mm); + if (mmu) + return mmu; + + mmu = kmalloc(sizeof(*mmu), GFP_KERNEL); + if (mmu == NULL) + return ERR_PTR(-ENOMEM); + + spin_lock_init(&mmu->lock); + mmu->dev = dev; + mmu->mn.ops = &i915_gem_userptr_notifier; + mmu->mm = mm; + mmu->objects = RB_ROOT; + mmu->count = 0; + + ret = mmu_notifier_register(&mmu->mn, mm); + if (ret) { + kfree(mmu); + return ERR_PTR(ret); + } + + hash_add(dev_priv->mmu_notifiers, &mmu->node, (unsigned long)mm); + return mmu; +} + +static void +__i915_mmu_notifier_destroy(struct work_struct *work) +{ + struct i915_mmu_notifier *mmu = container_of(work, typeof(*mmu), work); + mmu_notifier_unregister(&mmu->mn, mmu->mm); + kfree(mmu); +} + +static void +i915_mmu_notifier_del(struct i915_mmu_notifier *mmu, + struct i915_mmu_object *mn) +{ + bool destroy; + + spin_lock(&mmu->lock); + interval_tree_remove(&mn->it, &mmu->objects); + destroy = --mmu->count == 0; + spin_unlock(&mmu->lock); + + if (destroy) { /* protected against _add() by struct_mutex */ + hash_del(&mmu->node); + INIT_WORK(&mmu->work, __i915_mmu_notifier_destroy); + schedule_work(&mmu->work); + } +} + +static void +i915_mmu_notifier_add(struct i915_mmu_notifier *mmu, + struct i915_mmu_object *mn) +{ + spin_lock(&mmu->lock); + interval_tree_insert(&mn->it, &mmu->objects); + mmu->count++; + spin_unlock(&mmu->lock); +} + +static void +i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj) +{ + struct i915_mmu_object *mn; + + mn = obj->userptr.mn; + if (mn == NULL) + return; + + i915_mmu_notifier_del(mn->mmu, mn); + obj->userptr.mn = NULL; +} + +static int +i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, + unsigned flags) +{ + struct i915_mmu_notifier *mmu; + struct i915_mmu_object *mn; + + if (flags & I915_USERPTR_UNSYNCHRONIZED) + return capable(CAP_SYS_ADMIN) ? 0 : -EPERM; + + mmu = i915_mmu_notifier_get(obj->base.dev, obj->userptr.mm); + if (IS_ERR(mmu)) + return PTR_ERR(mmu); + + mn = kzalloc(sizeof(*mn), GFP_KERNEL); + if (mn == NULL) + return -ENOMEM; + + mn->mmu = mmu; + mn->it.start = obj->userptr.ptr; + mn->it.last = mn->it.start + obj->base.size; + mn->obj = obj; + + i915_mmu_notifier_add(mmu, mn); + + obj->userptr.mn = mn; + return 0; +} + +#else + +static void +i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj) +{ +} + +static int +i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, + unsigned flags) +{ + if ((flags & I915_USERPTR_UNSYNCHRONIZED) == 0) + return -ENODEV; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + return 0; +} +#endif + +struct get_pages_work { + struct work_struct work; + struct drm_i915_gem_object *obj; + struct task_struct *task; +}; + +static void +__i915_gem_userptr_get_pages_worker(struct work_struct *_work) +{ + struct get_pages_work *work = container_of(_work, typeof(*work), work); + struct drm_i915_gem_object *obj = work->obj; + struct drm_device *dev = obj->base.dev; + const int num_pages = obj->base.size >> PAGE_SHIFT; + struct page **pvec; + int pinned, ret; + + ret = -ENOMEM; + pinned = 0; + + pvec = kmalloc(num_pages*sizeof(struct page *), + GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); + if (pvec == NULL) + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); + if (pvec != NULL) { + struct mm_struct *mm = obj->userptr.mm; + + use_mm(mm); + down_read(&mm->mmap_sem); + while (pinned < num_pages) { + ret = get_user_pages(work->task, mm, + obj->userptr.ptr + pinned * PAGE_SIZE, + num_pages - pinned, + !obj->userptr.read_only, 0, + pvec + pinned, NULL); + if (ret < 0) + break; + + pinned += ret; + } + up_read(&mm->mmap_sem); + unuse_mm(mm); + } + + mutex_lock(&dev->struct_mutex); + if (obj->pages) { + ret = 0; + } else if (pinned == num_pages) { + struct sg_table *st; + + st = kmalloc(sizeof(*st), GFP_KERNEL); + if (st == NULL || sg_alloc_table(st, num_pages, GFP_KERNEL)) { + kfree(st); + ret = -ENOMEM; + } else { + struct scatterlist *sg; + int n; + + for_each_sg(st->sgl, sg, num_pages, n) + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); + + obj->pages = st; + list_add_tail(&obj->global_list, &to_i915(dev)->mm.unbound_list); + pinned = 0; + ret = 0; + } + } + + obj->userptr.work = ERR_PTR(ret); + drm_gem_object_unreference(&obj->base); + mutex_unlock(&dev->struct_mutex); + + release_pages(pvec, pinned, 0); + drm_free_large(pvec); + + put_task_struct(work->task); + kfree(work); +} + +static int +i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) +{ + const int num_pages = obj->base.size >> PAGE_SHIFT; + struct page **pvec; + int pinned, ret; + + /* If userspace should engineer that these pages are replaced in + * the vma between us binding this page into the GTT and completion + * of rendering... Their loss. If they change the mapping of their + * pages they need to create a new bo to point to the new vma. + * + * However, that still leaves open the possibility of the vma + * being copied upon fork. Which falls under the same userspace + * synchronisation issue as a regular bo, except that this time + * the process may not be expecting that a particular piece of + * memory is tied to the GPU. + * + * Fortunately, we can hook into the mmu_notifier in order to + * discard the page references prior to anything nasty happening + * to the vma (discard or cloning) which should prevent the more + * egregious cases from causing harm. + */ + + pvec = NULL; + pinned = 0; + if (obj->userptr.mm == current->mm) { + pvec = kmalloc(num_pages*sizeof(struct page *), + GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); + if (pvec == NULL) { + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); + if (pvec == NULL) + return -ENOMEM; + } + + pinned = __get_user_pages_fast(obj->userptr.ptr, num_pages, + !obj->userptr.read_only, pvec); + } + if (pinned < num_pages) { + if (pinned < 0) { + ret = pinned; + pinned = 0; + } else { + /* Spawn a worker so that we can acquire the + * user pages without holding our mutex. + */ + ret = -EAGAIN; + if (obj->userptr.work == NULL) { + struct get_pages_work *work; + + work = kmalloc(sizeof(*work), GFP_KERNEL); + if (work != NULL) { + obj->userptr.work = &work->work; + + work->obj = obj; + drm_gem_object_reference(&obj->base); + + work->task = current; + get_task_struct(work->task); + + INIT_WORK(&work->work, __i915_gem_userptr_get_pages_worker); + schedule_work(&work->work); + } else + ret = -ENOMEM; + } else { + if (IS_ERR(obj->userptr.work)) { + ret = PTR_ERR(obj->userptr.work); + obj->userptr.work = NULL; + } + } + } + } else { + struct sg_table *st; + + st = kmalloc(sizeof(*st), GFP_KERNEL); + if (st == NULL || sg_alloc_table(st, num_pages, GFP_KERNEL)) { + kfree(st); + ret = -ENOMEM; + } else { + struct scatterlist *sg; + int n; + + for_each_sg(st->sgl, sg, num_pages, n) + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); + + obj->pages = st; + pinned = 0; + ret = 0; + } + } + + release_pages(pvec, pinned, 0); + drm_free_large(pvec); + return ret; +} + +static void +i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj) +{ + struct scatterlist *sg; + int i; + + if (obj->madv != I915_MADV_WILLNEED) + obj->dirty = 0; + + for_each_sg(obj->pages->sgl, sg, obj->pages->nents, i) { + struct page *page = sg_page(sg); + + if (obj->dirty) + set_page_dirty(page); + + mark_page_accessed(page); + page_cache_release(page); + } + obj->dirty = 0; + + sg_free_table(obj->pages); + kfree(obj->pages); +} + +static void +i915_gem_userptr_release(struct drm_i915_gem_object *obj) +{ + i915_gem_userptr_release__mmu_notifier(obj); + + if (obj->userptr.mm) { + mmput(obj->userptr.mm); + obj->userptr.mm = NULL; + } +} + +static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = { + .get_pages = i915_gem_userptr_get_pages, + .put_pages = i915_gem_userptr_put_pages, + .release = i915_gem_userptr_release, +}; + +/** + * Creates a new mm object that wraps some normal memory from the process + * context - user memory. + * + * We impose several restrictions upon the memory being mapped + * into the GPU. + * 1. It must be page aligned (both start/end addresses, i.e ptr and size). + * 2. We only allow a bo as large as we could in theory map into the GTT, + * that is we limit the size to the total size of the GTT. + * 3. The bo is marked as being snoopable. The backing pages are left + * accessible directly by the CPU, but reads by the GPU may incur the cost + * of a snoop (unless you have an LLC architecture). + * + * Synchronisation between multiple users and the GPU is left to userspace + * through the normal set-domain-ioctl. The kernel will enforce that the + * GPU relinquishes the VMA before it is returned back to the system + * i.e. upon free(), munmap() or process termination. However, the userspace + * malloc() library may not immediately relinquish the VMA after free() and + * instead reuse it whilst the GPU is still reading and writing to the VMA. + * Caveat emptor. + * + * Also note, that the object created here is not currently a "first class" + * object, in that several ioctls are banned. These are the CPU access + * ioctls: mmap(), pwrite and pread. In practice, you are expected to use + * direct access via your pointer rather than use those ioctls. + * + * If you think this is a good interface to use to pass GPU memory between + * drivers, please use dma-buf instead. In fact, wherever possible use + * dma-buf instead. + */ +int +i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_gem_userptr *args = data; + struct drm_i915_gem_object *obj; + int ret; + u32 handle; + + if (args->flags & ~(I915_USERPTR_READ_ONLY | + I915_USERPTR_UNSYNCHRONIZED)) + return -EINVAL; + + if (offset_in_page(args->user_ptr | args->user_size)) + return -EINVAL; + + if (args->user_size > dev_priv->gtt.base.total) + return -E2BIG; + + if (!access_ok(args->flags & I915_USERPTR_READ_ONLY ? VERIFY_READ : VERIFY_WRITE, + (char __user *)(unsigned long)args->user_ptr, args->user_size)) + return -EFAULT; + + /* Allocate the new object */ + obj = i915_gem_object_alloc(dev); + if (obj == NULL) + return -ENOMEM; + + drm_gem_private_object_init(dev, &obj->base, args->user_size); + i915_gem_object_init(obj, &i915_gem_userptr_ops); + obj->cache_level = I915_CACHE_LLC; + obj->base.write_domain = I915_GEM_DOMAIN_CPU; + obj->base.read_domains = I915_GEM_DOMAIN_CPU; + + obj->userptr.ptr = args->user_ptr; + obj->userptr.read_only = !!(args->flags & I915_USERPTR_READ_ONLY); + + /* And keep a pointer to the current->mm for resolving the user pages + * at binding. This means that we need to hook into the mmu_notifier + * in order to detect if the mmu is destroyed. + */ + ret = -ENOMEM; + if ((obj->userptr.mm = get_task_mm(current))) + ret = i915_gem_userptr_init__mmu_notifier(obj, args->flags); + if (ret == 0) + ret = drm_gem_handle_create(file, &obj->base, &handle); + + /* drop reference from allocate - handle holds it now */ + drm_gem_object_unreference_unlocked(&obj->base); + if (ret) + return ret; + + args->handle = handle; + return 0; +} + +int +i915_gem_init_userptr(struct drm_device *dev) +{ +#if defined(CONFIG_MMU_NOTIFIER) + struct drm_i915_private *dev_priv = to_i915(dev); + hash_init(dev_priv->mmu_notifiers); +#endif + return 0; +} diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index ae8cf61b8ce3..cce9f559e3d7 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -201,6 +201,7 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m, err_puts(m, tiling_flag(err->tiling)); err_puts(m, dirty_flag(err->dirty)); err_puts(m, purgeable_flag(err->purgeable)); + err_puts(m, err->userptr ? " userptr" : ""); err_puts(m, err->ring != -1 ? " " : ""); err_puts(m, ring_str(err->ring)); err_puts(m, i915_cache_level_str(err->cache_level)); @@ -584,6 +585,7 @@ static void capture_bo(struct drm_i915_error_buffer *err, err->tiling = obj->tiling_mode; err->dirty = obj->dirty; err->purgeable = obj->madv != I915_MADV_WILLNEED; + err->userptr = obj->userptr.mm != 0; err->ring = obj->ring ? obj->ring->id : -1; err->cache_level = obj->cache_level; } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 37c8073a8246..6c145a0be250 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -224,6 +224,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_REG_READ 0x31 #define DRM_I915_GET_RESET_STATS 0x32 #define DRM_I915_GEM_CREATE2 0x33 +#define DRM_I915_GEM_USERPTR 0x34 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -275,6 +276,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy) #define DRM_IOCTL_I915_REG_READ DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read) #define DRM_IOCTL_I915_GET_RESET_STATS DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GET_RESET_STATS, struct drm_i915_reset_stats) +#define DRM_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1129,4 +1131,18 @@ struct drm_i915_reset_stats { __u32 pad; }; +struct drm_i915_gem_userptr { + __u64 user_ptr; + __u64 user_size; + __u32 flags; +#define I915_USERPTR_READ_ONLY 0x1 +#define I915_USERPTR_UNSYNCHRONIZED 0x80000000 + /** + * Returned handle for the object. + * + * Object handles are nonzero. + */ + __u32 handle; +}; + #endif /* _UAPI_I915_DRM_H_ */ -- 1.8.5.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl 2014-01-21 15:07 ` [PATCH 3/3] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl Chris Wilson @ 2014-01-22 9:46 ` Chris Wilson 2014-01-24 9:00 ` Chris Wilson 0 siblings, 1 reply; 16+ messages in thread From: Chris Wilson @ 2014-01-22 9:46 UTC (permalink / raw) To: intel-gfx; +Cc: Akash Goel By exporting the ability to map user address and inserting PTEs representing their backing pages into the GTT, we can exploit UMA in order to utilize normal application data as a texture source or even as a render target (depending upon the capabilities of the chipset). This has a number of uses, with zero-copy downloads to the GPU and efficient readback making the intermixed streaming of CPU and GPU operations fairly efficient. This ability has many widespread implications from faster rendering of client-side software rasterisers (chromium), mitigation of stalls due to read back (firefox) and to faster pipelining of texture data (such as pixel buffer objects in GL or data blobs in CL). v2: Compile with CONFIG_MMU_NOTIFIER v3: We can sleep while performing invalidate-range, which we can utilise to drop our page references prior to the kernel manipulating the vma (for either discard or cloning) and so protect normal users. v4: Only run the invalidate notifier if the range intercepts the bo. v5: Prevent userspace from attempting to GTT mmap non-page aligned buffers v6: Recheck after reacquire mutex for lost mmu. v7: Fix implicit padding of ioctl struct by rounding to next 64bit boundary. v8: Fix rebasing error after forwarding porting the back port. v9: Limit the userptr to page aligned entries. We now expect userspace to handle all the offset-in-page adjustments itself. v10: Prevent vma from being copied across fork to avoid issues with cow. v11: Drop vma behaviour changes -- locking is nigh on impossible. Use a worker to load user pages to avoid lock inversions. v12: Use get_task_mm()/mmput() for correct refcounting of mm. v13: Use a worker to release the mmu_notifier to avoid lock inversion v14: Decouple mmu_notifier from struct_mutex using a custom mmu_notifer with its own locking and tree of objects for each mm/mmu_notifier. v15: Prevent overlapping userptr objects, and invalidate all objects within the mmu_notifier range Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: "Gong, Zhipeng" <zhipeng.gong@intel.com> Cc: Akash Goel <akash.goel@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 24 +- drivers/gpu/drm/i915/i915_gem.c | 4 + drivers/gpu/drm/i915/i915_gem_userptr.c | 599 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gpu_error.c | 2 + include/uapi/drm/i915_drm.h | 16 + 7 files changed, 646 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 45071d18c730..1dc14e51f3bd 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -15,6 +15,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ i915_gem_gtt.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ + i915_gem_userptr.o \ i915_sysfs.o \ i915_trace_points.o \ i915_ums.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f053d1099d3b..c34f2e7ae6d9 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1919,6 +1919,7 @@ const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CREATE2, i915_gem_create2_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), }; int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 35a2e339c298..fe46be1c48cc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -40,6 +40,7 @@ #include <linux/i2c-algo-bit.h> #include <drm/intel-gtt.h> #include <linux/backlight.h> +#include <linux/hashtable.h> #include <linux/intel-iommu.h> #include <linux/kref.h> #include <linux/pm_qos.h> @@ -163,6 +164,7 @@ enum hpd_pin { if ((intel_encoder)->base.crtc == (__crtc)) struct drm_i915_private; +struct i915_mmu_notifier; enum intel_dpll_id { DPLL_ID_PRIVATE = -1, /* non-shared dpll in use */ @@ -354,6 +356,7 @@ struct drm_i915_error_state { u32 tiling:2; u32 dirty:1; u32 purgeable:1; + u32 userptr:1; s32 ring:4; u32 cache_level:3; } **active_bo, **pinned_bo; @@ -1465,6 +1468,9 @@ typedef struct drm_i915_private { struct i915_gtt gtt; /* VMA representing the global address space */ struct i915_gem_mm mm; +#if defined(CONFIG_MMU_NOTIFIER) + DECLARE_HASHTABLE(mmu_notifiers, 7); +#endif /* Kernel Modesetting */ @@ -1596,6 +1602,7 @@ struct drm_i915_gem_object_ops { */ int (*get_pages)(struct drm_i915_gem_object *); void (*put_pages)(struct drm_i915_gem_object *); + void (*release)(struct drm_i915_gem_object *); }; struct drm_i915_gem_object { @@ -1709,9 +1716,21 @@ struct drm_i915_gem_object { /** for phy allocated objects */ struct drm_i915_gem_phys_object *phys_obj; + + union { + struct i915_gem_userptr { + uintptr_t ptr; + unsigned read_only :1; + + struct mm_struct *mm; +#if defined(CONFIG_MMU_NOTIFIER) + struct i915_mmu_object *mn; +#endif + struct work_struct *work; + } userptr; + }; }; #define to_gem_object(obj) (&((struct drm_i915_gem_object *)(obj))->base) - #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) /** @@ -2012,6 +2031,9 @@ int i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +int i915_gem_init_userptr(struct drm_device *dev); +int i915_gem_userptr_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_wait_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8912aaa7118a..0d20af63b876 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4380,6 +4380,9 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) if (obj->base.import_attach) drm_prime_gem_destroy(&obj->base, NULL); + if (obj->ops->release) + obj->ops->release(obj); + drm_gem_object_release(&obj->base); i915_gem_info_remove_obj(dev_priv, obj->base.size); @@ -4638,6 +4641,7 @@ int i915_gem_init(struct drm_device *dev) DRM_DEBUG_DRIVER("allow wake ack timed out\n"); } + i915_gem_init_userptr(dev); i915_gem_init_global_gtt(dev); ret = i915_gem_context_init(dev); diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c new file mode 100644 index 000000000000..79d07166c1aa --- /dev/null +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -0,0 +1,599 @@ +/* + * Copyright © 2012-2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include "drmP.h" +#include "i915_drm.h" +#include "i915_drv.h" +#include "i915_trace.h" +#include "intel_drv.h" +#include <linux/mmu_context.h> +#include <linux/mmu_notifier.h> +#include <linux/mempolicy.h> +#include <linux/swap.h> + +#if defined(CONFIG_MMU_NOTIFIER) +#include <linux/interval_tree.h> + +struct i915_mmu_notifier { + spinlock_t lock; + struct hlist_node node; + struct mmu_notifier mn; + struct rb_root objects; + struct drm_device *dev; + struct mm_struct *mm; + struct work_struct work; + unsigned long count; + unsigned long serial; +}; + +struct i915_mmu_object { + struct i915_mmu_notifier *mmu; + struct interval_tree_node it; + struct drm_i915_gem_object *obj; +}; + +static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, + struct mm_struct *mm, + unsigned long start, + unsigned long end) +{ + struct i915_mmu_notifier *mn = container_of(_mn, struct i915_mmu_notifier, mn); + struct interval_tree_node *it = NULL; + unsigned long serial = 0; + + while (start < end) { + struct drm_i915_gem_object *obj; + + obj = NULL; + spin_lock(&mn->lock); + if (serial == mn->serial) + interval_tree_iter_next(it, start, end); + else + it = interval_tree_iter_first(&mn->objects, start, end); + if (it != NULL) { + obj = container_of(it, struct i915_mmu_object, it)->obj; + drm_gem_object_reference(&obj->base); + serial = mn->serial; + } + spin_unlock(&mn->lock); + if (obj == NULL) + return; + + mutex_lock(&mn->dev->struct_mutex); + if (obj->pages != NULL) { + struct drm_i915_private *dev_priv = to_i915(mn->dev); + struct i915_vma *vma, *tmp; + bool was_interruptible; + + was_interruptible = dev_priv->mm.interruptible; + dev_priv->mm.interruptible = false; + + list_for_each_entry_safe(vma, tmp, &obj->vma_list, vma_link) { + int ret = i915_vma_unbind(vma); + WARN_ON(ret && ret != -EIO); + } + WARN_ON(i915_gem_object_put_pages(obj)); + + dev_priv->mm.interruptible = was_interruptible; + } + drm_gem_object_unreference(&obj->base); + mutex_unlock(&mn->dev->struct_mutex); + + start = obj->userptr.ptr + obj->base.size; + } +} + +static const struct mmu_notifier_ops i915_gem_userptr_notifier = { + .invalidate_range_start = i915_gem_userptr_mn_invalidate_range_start, +}; + +static struct i915_mmu_notifier * +__i915_mmu_notifier_lookup(struct drm_device *dev, struct mm_struct *mm) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct i915_mmu_notifier *mmu; + + hash_for_each_possible(dev_priv->mmu_notifiers, mmu, node, (unsigned long)mm) + if (mmu->mm == mm) + return mmu; + + return NULL; +} + +static struct i915_mmu_notifier * +i915_mmu_notifier_get(struct drm_device *dev, struct mm_struct *mm) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct i915_mmu_notifier *mmu; + int ret; + + mmu = __i915_mmu_notifier_lookup(dev, mm); + if (mmu) + return mmu; + + mmu = kmalloc(sizeof(*mmu), GFP_KERNEL); + if (mmu == NULL) + return ERR_PTR(-ENOMEM); + + spin_lock_init(&mmu->lock); + mmu->dev = dev; + mmu->mn.ops = &i915_gem_userptr_notifier; + mmu->mm = mm; + mmu->objects = RB_ROOT; + mmu->count = 0; + mmu->serial = 0; + + ret = mmu_notifier_register(&mmu->mn, mm); + if (ret) { + kfree(mmu); + return ERR_PTR(ret); + } + + hash_add(dev_priv->mmu_notifiers, &mmu->node, (unsigned long)mm); + return mmu; +} + +static void +__i915_mmu_notifier_destroy(struct i915_mmu_notifier *mmu) +{ + mmu_notifier_unregister(&mmu->mn, mmu->mm); + kfree(mmu); +} + +static void +__i915_mmu_notifier_destroy_worker(struct work_struct *work) +{ + __i915_mmu_notifier_destroy(container_of(work, typeof(struct i915_mmu_notifier), work)); +} + +static void __i915_mmu_notifier_update_serial(struct i915_mmu_notifier *mmu) +{ + if (++mmu->serial == 0) + mmu->serial = 1; +} + +static void +i915_mmu_notifier_del(struct i915_mmu_notifier *mmu, + struct i915_mmu_object *mn) +{ + bool destroy; + + spin_lock(&mmu->lock); + interval_tree_remove(&mn->it, &mmu->objects); + destroy = --mmu->count == 0; + __i915_mmu_notifier_update_serial(mmu); + spin_unlock(&mmu->lock); + + if (destroy) { /* protected against _add() by struct_mutex */ + hash_del(&mmu->node); + INIT_WORK(&mmu->work, __i915_mmu_notifier_destroy_worker); + schedule_work(&mmu->work); + } +} + +static int +i915_mmu_notifier_add(struct i915_mmu_notifier *mmu, + struct i915_mmu_object *mn) +{ + int ret = -EINVAL; + + spin_lock(&mmu->lock); + /* Disallow overlapping userptr objects */ + if (!interval_tree_iter_first(&mmu->objects, + mn->it.start, mn->it.last)) { + interval_tree_insert(&mn->it, &mmu->objects); + mmu->count++; + __i915_mmu_notifier_update_serial(mmu); + ret = 0; + } + spin_unlock(&mmu->lock); + + return ret; +} + +static void +i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj) +{ + struct i915_mmu_object *mn; + + mn = obj->userptr.mn; + if (mn == NULL) + return; + + i915_mmu_notifier_del(mn->mmu, mn); + obj->userptr.mn = NULL; +} + +static int +i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, + unsigned flags) +{ + struct i915_mmu_notifier *mmu; + struct i915_mmu_object *mn; + int ret; + + if (flags & I915_USERPTR_UNSYNCHRONIZED) + return capable(CAP_SYS_ADMIN) ? 0 : -EPERM; + + mmu = i915_mmu_notifier_get(obj->base.dev, obj->userptr.mm); + if (IS_ERR(mmu)) + return PTR_ERR(mmu); + + mn = kzalloc(sizeof(*mn), GFP_KERNEL); + if (mn == NULL) { + ret = -ENOMEM; + goto put_mmu; + } + + mn->mmu = mmu; + mn->it.start = obj->userptr.ptr; + mn->it.last = mn->it.start + obj->base.size - 1; + mn->obj = obj; + + ret = i915_mmu_notifier_add(mmu, mn); + if (ret) + goto free_mn; + + obj->userptr.mn = mn; + return 0; + +free_mn: + kfree(mn); +put_mmu: + if (mmu->count == 0) + __i915_mmu_notifier_destroy(mmu); + return ret; +} + +#else + +static void +i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj) +{ +} + +static int +i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, + unsigned flags) +{ + if ((flags & I915_USERPTR_UNSYNCHRONIZED) == 0) + return -ENODEV; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + return 0; +} +#endif + +struct get_pages_work { + struct work_struct work; + struct drm_i915_gem_object *obj; + struct task_struct *task; +}; + +static void +__i915_gem_userptr_get_pages_worker(struct work_struct *_work) +{ + struct get_pages_work *work = container_of(_work, typeof(*work), work); + struct drm_i915_gem_object *obj = work->obj; + struct drm_device *dev = obj->base.dev; + const int num_pages = obj->base.size >> PAGE_SHIFT; + struct page **pvec; + int pinned, ret; + + ret = -ENOMEM; + pinned = 0; + + pvec = kmalloc(num_pages*sizeof(struct page *), + GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); + if (pvec == NULL) + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); + if (pvec != NULL) { + struct mm_struct *mm = obj->userptr.mm; + + use_mm(mm); + down_read(&mm->mmap_sem); + while (pinned < num_pages) { + ret = get_user_pages(work->task, mm, + obj->userptr.ptr + pinned * PAGE_SIZE, + num_pages - pinned, + !obj->userptr.read_only, 0, + pvec + pinned, NULL); + if (ret < 0) + break; + + pinned += ret; + } + up_read(&mm->mmap_sem); + unuse_mm(mm); + } + + mutex_lock(&dev->struct_mutex); + if (obj->pages) { + ret = 0; + } else if (pinned == num_pages) { + struct sg_table *st; + + st = kmalloc(sizeof(*st), GFP_KERNEL); + if (st == NULL || sg_alloc_table(st, num_pages, GFP_KERNEL)) { + kfree(st); + ret = -ENOMEM; + } else { + struct scatterlist *sg; + int n; + + for_each_sg(st->sgl, sg, num_pages, n) + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); + + obj->pages = st; + list_add_tail(&obj->global_list, &to_i915(dev)->mm.unbound_list); + pinned = 0; + ret = 0; + } + } + + obj->userptr.work = ERR_PTR(ret); + drm_gem_object_unreference(&obj->base); + mutex_unlock(&dev->struct_mutex); + + release_pages(pvec, pinned, 0); + drm_free_large(pvec); + + put_task_struct(work->task); + kfree(work); +} + +static int +i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) +{ + const int num_pages = obj->base.size >> PAGE_SHIFT; + struct page **pvec; + int pinned, ret; + + /* If userspace should engineer that these pages are replaced in + * the vma between us binding this page into the GTT and completion + * of rendering... Their loss. If they change the mapping of their + * pages they need to create a new bo to point to the new vma. + * + * However, that still leaves open the possibility of the vma + * being copied upon fork. Which falls under the same userspace + * synchronisation issue as a regular bo, except that this time + * the process may not be expecting that a particular piece of + * memory is tied to the GPU. + * + * Fortunately, we can hook into the mmu_notifier in order to + * discard the page references prior to anything nasty happening + * to the vma (discard or cloning) which should prevent the more + * egregious cases from causing harm. + */ + + pvec = NULL; + pinned = 0; + if (obj->userptr.mm == current->mm) { + pvec = kmalloc(num_pages*sizeof(struct page *), + GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); + if (pvec == NULL) { + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); + if (pvec == NULL) + return -ENOMEM; + } + + pinned = __get_user_pages_fast(obj->userptr.ptr, num_pages, + !obj->userptr.read_only, pvec); + } + if (pinned < num_pages) { + if (pinned < 0) { + ret = pinned; + pinned = 0; + } else { + /* Spawn a worker so that we can acquire the + * user pages without holding our mutex. + */ + ret = -EAGAIN; + if (obj->userptr.work == NULL) { + struct get_pages_work *work; + + work = kmalloc(sizeof(*work), GFP_KERNEL); + if (work != NULL) { + obj->userptr.work = &work->work; + + work->obj = obj; + drm_gem_object_reference(&obj->base); + + work->task = current; + get_task_struct(work->task); + + INIT_WORK(&work->work, __i915_gem_userptr_get_pages_worker); + schedule_work(&work->work); + } else + ret = -ENOMEM; + } else { + if (IS_ERR(obj->userptr.work)) { + ret = PTR_ERR(obj->userptr.work); + obj->userptr.work = NULL; + } + } + } + } else { + struct sg_table *st; + + st = kmalloc(sizeof(*st), GFP_KERNEL); + if (st == NULL || sg_alloc_table(st, num_pages, GFP_KERNEL)) { + kfree(st); + ret = -ENOMEM; + } else { + struct scatterlist *sg; + int n; + + for_each_sg(st->sgl, sg, num_pages, n) + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); + + obj->pages = st; + pinned = 0; + ret = 0; + } + } + + release_pages(pvec, pinned, 0); + drm_free_large(pvec); + return ret; +} + +static void +i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj) +{ + struct scatterlist *sg; + int i; + + if (obj->madv != I915_MADV_WILLNEED) + obj->dirty = 0; + + for_each_sg(obj->pages->sgl, sg, obj->pages->nents, i) { + struct page *page = sg_page(sg); + + if (obj->dirty) + set_page_dirty(page); + + mark_page_accessed(page); + page_cache_release(page); + } + obj->dirty = 0; + + sg_free_table(obj->pages); + kfree(obj->pages); +} + +static void +i915_gem_userptr_release(struct drm_i915_gem_object *obj) +{ + i915_gem_userptr_release__mmu_notifier(obj); + + if (obj->userptr.mm) { + mmput(obj->userptr.mm); + obj->userptr.mm = NULL; + } +} + +static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = { + .get_pages = i915_gem_userptr_get_pages, + .put_pages = i915_gem_userptr_put_pages, + .release = i915_gem_userptr_release, +}; + +/** + * Creates a new mm object that wraps some normal memory from the process + * context - user memory. + * + * We impose several restrictions upon the memory being mapped + * into the GPU. + * 1. It must be page aligned (both start/end addresses, i.e ptr and size). + * 2. We only allow a bo as large as we could in theory map into the GTT, + * that is we limit the size to the total size of the GTT. + * 3. The bo is marked as being snoopable. The backing pages are left + * accessible directly by the CPU, but reads by the GPU may incur the cost + * of a snoop (unless you have an LLC architecture). + * + * Synchronisation between multiple users and the GPU is left to userspace + * through the normal set-domain-ioctl. The kernel will enforce that the + * GPU relinquishes the VMA before it is returned back to the system + * i.e. upon free(), munmap() or process termination. However, the userspace + * malloc() library may not immediately relinquish the VMA after free() and + * instead reuse it whilst the GPU is still reading and writing to the VMA. + * Caveat emptor. + * + * Also note, that the object created here is not currently a "first class" + * object, in that several ioctls are banned. These are the CPU access + * ioctls: mmap(), pwrite and pread. In practice, you are expected to use + * direct access via your pointer rather than use those ioctls. + * + * If you think this is a good interface to use to pass GPU memory between + * drivers, please use dma-buf instead. In fact, wherever possible use + * dma-buf instead. + */ +int +i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_gem_userptr *args = data; + struct drm_i915_gem_object *obj; + int ret; + u32 handle; + + if (args->flags & ~(I915_USERPTR_READ_ONLY | + I915_USERPTR_UNSYNCHRONIZED)) + return -EINVAL; + + if (offset_in_page(args->user_ptr | args->user_size)) + return -EINVAL; + + if (args->user_size > dev_priv->gtt.base.total) + return -E2BIG; + + if (!access_ok(args->flags & I915_USERPTR_READ_ONLY ? VERIFY_READ : VERIFY_WRITE, + (char __user *)(unsigned long)args->user_ptr, args->user_size)) + return -EFAULT; + + /* Allocate the new object */ + obj = i915_gem_object_alloc(dev); + if (obj == NULL) + return -ENOMEM; + + drm_gem_private_object_init(dev, &obj->base, args->user_size); + i915_gem_object_init(obj, &i915_gem_userptr_ops); + obj->cache_level = I915_CACHE_LLC; + obj->base.write_domain = I915_GEM_DOMAIN_CPU; + obj->base.read_domains = I915_GEM_DOMAIN_CPU; + + obj->userptr.ptr = args->user_ptr; + obj->userptr.read_only = !!(args->flags & I915_USERPTR_READ_ONLY); + + /* And keep a pointer to the current->mm for resolving the user pages + * at binding. This means that we need to hook into the mmu_notifier + * in order to detect if the mmu is destroyed. + */ + ret = -ENOMEM; + if ((obj->userptr.mm = get_task_mm(current))) + ret = i915_gem_userptr_init__mmu_notifier(obj, args->flags); + if (ret == 0) + ret = drm_gem_handle_create(file, &obj->base, &handle); + + /* drop reference from allocate - handle holds it now */ + drm_gem_object_unreference_unlocked(&obj->base); + if (ret) + return ret; + + args->handle = handle; + return 0; +} + +int +i915_gem_init_userptr(struct drm_device *dev) +{ +#if defined(CONFIG_MMU_NOTIFIER) + struct drm_i915_private *dev_priv = to_i915(dev); + hash_init(dev_priv->mmu_notifiers); +#endif + return 0; +} diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index ae8cf61b8ce3..cce9f559e3d7 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -201,6 +201,7 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m, err_puts(m, tiling_flag(err->tiling)); err_puts(m, dirty_flag(err->dirty)); err_puts(m, purgeable_flag(err->purgeable)); + err_puts(m, err->userptr ? " userptr" : ""); err_puts(m, err->ring != -1 ? " " : ""); err_puts(m, ring_str(err->ring)); err_puts(m, i915_cache_level_str(err->cache_level)); @@ -584,6 +585,7 @@ static void capture_bo(struct drm_i915_error_buffer *err, err->tiling = obj->tiling_mode; err->dirty = obj->dirty; err->purgeable = obj->madv != I915_MADV_WILLNEED; + err->userptr = obj->userptr.mm != 0; err->ring = obj->ring ? obj->ring->id : -1; err->cache_level = obj->cache_level; } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 37c8073a8246..6c145a0be250 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -224,6 +224,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_REG_READ 0x31 #define DRM_I915_GET_RESET_STATS 0x32 #define DRM_I915_GEM_CREATE2 0x33 +#define DRM_I915_GEM_USERPTR 0x34 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -275,6 +276,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy) #define DRM_IOCTL_I915_REG_READ DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read) #define DRM_IOCTL_I915_GET_RESET_STATS DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GET_RESET_STATS, struct drm_i915_reset_stats) +#define DRM_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1129,4 +1131,18 @@ struct drm_i915_reset_stats { __u32 pad; }; +struct drm_i915_gem_userptr { + __u64 user_ptr; + __u64 user_size; + __u32 flags; +#define I915_USERPTR_READ_ONLY 0x1 +#define I915_USERPTR_UNSYNCHRONIZED 0x80000000 + /** + * Returned handle for the object. + * + * Object handles are nonzero. + */ + __u32 handle; +}; + #endif /* _UAPI_I915_DRM_H_ */ -- 1.8.5.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl 2014-01-22 9:46 ` [PATCH] " Chris Wilson @ 2014-01-24 9:00 ` Chris Wilson 2014-01-27 17:56 ` Volkin, Bradley D 0 siblings, 1 reply; 16+ messages in thread From: Chris Wilson @ 2014-01-24 9:00 UTC (permalink / raw) To: intel-gfx; +Cc: Akash Goel By exporting the ability to map user address and inserting PTEs representing their backing pages into the GTT, we can exploit UMA in order to utilize normal application data as a texture source or even as a render target (depending upon the capabilities of the chipset). This has a number of uses, with zero-copy downloads to the GPU and efficient readback making the intermixed streaming of CPU and GPU operations fairly efficient. This ability has many widespread implications from faster rendering of client-side software rasterisers (chromium), mitigation of stalls due to read back (firefox) and to faster pipelining of texture data (such as pixel buffer objects in GL or data blobs in CL). v2: Compile with CONFIG_MMU_NOTIFIER v3: We can sleep while performing invalidate-range, which we can utilise to drop our page references prior to the kernel manipulating the vma (for either discard or cloning) and so protect normal users. v4: Only run the invalidate notifier if the range intercepts the bo. v5: Prevent userspace from attempting to GTT mmap non-page aligned buffers v6: Recheck after reacquire mutex for lost mmu. v7: Fix implicit padding of ioctl struct by rounding to next 64bit boundary. v8: Fix rebasing error after forwarding porting the back port. v9: Limit the userptr to page aligned entries. We now expect userspace to handle all the offset-in-page adjustments itself. v10: Prevent vma from being copied across fork to avoid issues with cow. v11: Drop vma behaviour changes -- locking is nigh on impossible. Use a worker to load user pages to avoid lock inversions. v12: Use get_task_mm()/mmput() for correct refcounting of mm. v13: Use a worker to release the mmu_notifier to avoid lock inversion v14: Decouple mmu_notifier from struct_mutex using a custom mmu_notifer with its own locking and tree of objects for each mm/mmu_notifier. v15: Prevent overlapping userptr objects, and invalidate all objects within the mmu_notifier range v16: Fix a typo for iterating over multiple objects in the range and rearrange error path to destroy the mmu_notifier locklessly. Also close a race between invalidate_range and the get_pages_worker. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: "Gong, Zhipeng" <zhipeng.gong@intel.com> Cc: Akash Goel <akash.goel@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 24 +- drivers/gpu/drm/i915/i915_gem.c | 4 + drivers/gpu/drm/i915/i915_gem_userptr.c | 608 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gpu_error.c | 2 + include/uapi/drm/i915_drm.h | 16 + 7 files changed, 655 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 45071d18c730..1dc14e51f3bd 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -15,6 +15,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ i915_gem_gtt.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ + i915_gem_userptr.o \ i915_sysfs.o \ i915_trace_points.o \ i915_ums.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index f053d1099d3b..c34f2e7ae6d9 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1919,6 +1919,7 @@ const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CREATE2, i915_gem_create2_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), }; int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 73cff1119247..dac8aeecfa3e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -40,6 +40,7 @@ #include <linux/i2c-algo-bit.h> #include <drm/intel-gtt.h> #include <linux/backlight.h> +#include <linux/hashtable.h> #include <linux/intel-iommu.h> #include <linux/kref.h> #include <linux/pm_qos.h> @@ -163,6 +164,7 @@ enum hpd_pin { if ((intel_encoder)->base.crtc == (__crtc)) struct drm_i915_private; +struct i915_mmu_notifier; enum intel_dpll_id { DPLL_ID_PRIVATE = -1, /* non-shared dpll in use */ @@ -354,6 +356,7 @@ struct drm_i915_error_state { u32 tiling:2; u32 dirty:1; u32 purgeable:1; + u32 userptr:1; s32 ring:4; u32 cache_level:3; } **active_bo, **pinned_bo; @@ -1463,6 +1466,9 @@ typedef struct drm_i915_private { struct i915_gtt gtt; /* VMA representing the global address space */ struct i915_gem_mm mm; +#if defined(CONFIG_MMU_NOTIFIER) + DECLARE_HASHTABLE(mmu_notifiers, 7); +#endif /* Kernel Modesetting */ @@ -1594,6 +1600,7 @@ struct drm_i915_gem_object_ops { */ int (*get_pages)(struct drm_i915_gem_object *); void (*put_pages)(struct drm_i915_gem_object *); + void (*release)(struct drm_i915_gem_object *); }; struct drm_i915_gem_object { @@ -1707,9 +1714,21 @@ struct drm_i915_gem_object { /** for phy allocated objects */ struct drm_i915_gem_phys_object *phys_obj; + + union { + struct i915_gem_userptr { + uintptr_t ptr; + unsigned read_only :1; + + struct mm_struct *mm; +#if defined(CONFIG_MMU_NOTIFIER) + struct i915_mmu_object *mn; +#endif + struct work_struct *work; + } userptr; + }; }; #define to_gem_object(obj) (&((struct drm_i915_gem_object *)(obj))->base) - #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) /** @@ -2010,6 +2029,9 @@ int i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_get_tiling_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +int i915_gem_init_userptr(struct drm_device *dev); +int i915_gem_userptr_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_wait_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8912aaa7118a..0d20af63b876 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4380,6 +4380,9 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) if (obj->base.import_attach) drm_prime_gem_destroy(&obj->base, NULL); + if (obj->ops->release) + obj->ops->release(obj); + drm_gem_object_release(&obj->base); i915_gem_info_remove_obj(dev_priv, obj->base.size); @@ -4638,6 +4641,7 @@ int i915_gem_init(struct drm_device *dev) DRM_DEBUG_DRIVER("allow wake ack timed out\n"); } + i915_gem_init_userptr(dev); i915_gem_init_global_gtt(dev); ret = i915_gem_context_init(dev); diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c new file mode 100644 index 000000000000..82c33d1bcb50 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -0,0 +1,608 @@ +/* + * Copyright © 2012-2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include "drmP.h" +#include "i915_drm.h" +#include "i915_drv.h" +#include "i915_trace.h" +#include "intel_drv.h" +#include <linux/mmu_context.h> +#include <linux/mmu_notifier.h> +#include <linux/mempolicy.h> +#include <linux/swap.h> + +#if defined(CONFIG_MMU_NOTIFIER) +#include <linux/interval_tree.h> + +struct i915_mmu_notifier { + spinlock_t lock; + struct hlist_node node; + struct mmu_notifier mn; + struct rb_root objects; + struct drm_device *dev; + struct mm_struct *mm; + struct work_struct work; + unsigned long count; + unsigned long serial; +}; + +struct i915_mmu_object { + struct i915_mmu_notifier *mmu; + struct interval_tree_node it; + struct drm_i915_gem_object *obj; +}; + +static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, + struct mm_struct *mm, + unsigned long start, + unsigned long end) +{ + struct i915_mmu_notifier *mn = container_of(_mn, struct i915_mmu_notifier, mn); + struct interval_tree_node *it = NULL; + unsigned long serial = 0; + + end--; /* interval ranges are inclusive, but invalidate range is exclusive */ + while (start < end) { + struct drm_i915_gem_object *obj; + + obj = NULL; + spin_lock(&mn->lock); + if (serial == mn->serial) + it = interval_tree_iter_next(it, start, end); + else + it = interval_tree_iter_first(&mn->objects, start, end); + if (it != NULL) { + obj = container_of(it, struct i915_mmu_object, it)->obj; + drm_gem_object_reference(&obj->base); + serial = mn->serial; + } + spin_unlock(&mn->lock); + if (obj == NULL) + return; + + mutex_lock(&mn->dev->struct_mutex); + /* Cancel any active worker and force us to re-evaluate gup */ + obj->userptr.work = NULL; + + if (obj->pages != NULL) { + struct drm_i915_private *dev_priv = to_i915(mn->dev); + struct i915_vma *vma, *tmp; + bool was_interruptible; + + was_interruptible = dev_priv->mm.interruptible; + dev_priv->mm.interruptible = false; + + list_for_each_entry_safe(vma, tmp, &obj->vma_list, vma_link) { + int ret = i915_vma_unbind(vma); + WARN_ON(ret && ret != -EIO); + } + WARN_ON(i915_gem_object_put_pages(obj)); + + dev_priv->mm.interruptible = was_interruptible; + } + + start = obj->userptr.ptr + obj->base.size; + + drm_gem_object_unreference(&obj->base); + mutex_unlock(&mn->dev->struct_mutex); + } +} + +static const struct mmu_notifier_ops i915_gem_userptr_notifier = { + .invalidate_range_start = i915_gem_userptr_mn_invalidate_range_start, +}; + +static struct i915_mmu_notifier * +__i915_mmu_notifier_lookup(struct drm_device *dev, struct mm_struct *mm) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct i915_mmu_notifier *mmu; + + hash_for_each_possible(dev_priv->mmu_notifiers, mmu, node, (unsigned long)mm) + if (mmu->mm == mm) + return mmu; + + return NULL; +} + +static struct i915_mmu_notifier * +i915_mmu_notifier_get(struct drm_device *dev, struct mm_struct *mm) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct i915_mmu_notifier *mmu; + int ret; + + mmu = __i915_mmu_notifier_lookup(dev, mm); + if (mmu) + return mmu; + + mmu = kmalloc(sizeof(*mmu), GFP_KERNEL); + if (mmu == NULL) + return ERR_PTR(-ENOMEM); + + spin_lock_init(&mmu->lock); + mmu->dev = dev; + mmu->mn.ops = &i915_gem_userptr_notifier; + mmu->mm = mm; + mmu->objects = RB_ROOT; + mmu->count = 0; + mmu->serial = 0; + + ret = mmu_notifier_register(&mmu->mn, mm); + if (ret) { + kfree(mmu); + return ERR_PTR(ret); + } + + hash_add(dev_priv->mmu_notifiers, &mmu->node, (unsigned long)mm); + return mmu; +} + +static void +__i915_mmu_notifier_destroy_worker(struct work_struct *work) +{ + struct i915_mmu_notifier *mmu = container_of(work, typeof(*mmu), work); + mmu_notifier_unregister(&mmu->mn, mmu->mm); + kfree(mmu); +} + +static void +__i915_mmu_notifier_destroy(struct i915_mmu_notifier *mmu) +{ + hash_del(&mmu->node); + INIT_WORK(&mmu->work, __i915_mmu_notifier_destroy_worker); + schedule_work(&mmu->work); +} + +static void __i915_mmu_notifier_update_serial(struct i915_mmu_notifier *mmu) +{ + if (++mmu->serial == 0) + mmu->serial = 1; +} + +static void +i915_mmu_notifier_del(struct i915_mmu_notifier *mmu, + struct i915_mmu_object *mn) +{ + bool destroy; + + spin_lock(&mmu->lock); + interval_tree_remove(&mn->it, &mmu->objects); + destroy = --mmu->count == 0; + __i915_mmu_notifier_update_serial(mmu); + spin_unlock(&mmu->lock); + + if (destroy) /* protected against _add() by struct_mutex */ + __i915_mmu_notifier_destroy(mmu); +} + +static int +i915_mmu_notifier_add(struct i915_mmu_notifier *mmu, + struct i915_mmu_object *mn) +{ + int ret = -EINVAL; + + spin_lock(&mmu->lock); + /* Disallow overlapping userptr objects */ + if (!interval_tree_iter_first(&mmu->objects, + mn->it.start, mn->it.last)) { + interval_tree_insert(&mn->it, &mmu->objects); + mmu->count++; + __i915_mmu_notifier_update_serial(mmu); + ret = 0; + } + spin_unlock(&mmu->lock); + + return ret; +} + +static void +i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj) +{ + struct i915_mmu_object *mn; + + mn = obj->userptr.mn; + if (mn == NULL) + return; + + i915_mmu_notifier_del(mn->mmu, mn); + obj->userptr.mn = NULL; +} + +static int +i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, + unsigned flags) +{ + struct i915_mmu_notifier *mmu; + struct i915_mmu_object *mn; + int ret; + + if (flags & I915_USERPTR_UNSYNCHRONIZED) + return capable(CAP_SYS_ADMIN) ? 0 : -EPERM; + + mmu = i915_mmu_notifier_get(obj->base.dev, obj->userptr.mm); + if (IS_ERR(mmu)) + return PTR_ERR(mmu); + + mn = kzalloc(sizeof(*mn), GFP_KERNEL); + if (mn == NULL) { + ret = -ENOMEM; + goto destroy_mmu; + } + + mn->mmu = mmu; + mn->it.start = obj->userptr.ptr; + mn->it.last = mn->it.start + obj->base.size - 1; + mn->obj = obj; + + ret = i915_mmu_notifier_add(mmu, mn); + if (ret) + goto free_mn; + + obj->userptr.mn = mn; + return 0; + +free_mn: + kfree(mn); +destroy_mmu: + if (mmu->count == 0) + __i915_mmu_notifier_destroy(mmu); + return ret; +} + +#else + +static void +i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj) +{ +} + +static int +i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, + unsigned flags) +{ + if ((flags & I915_USERPTR_UNSYNCHRONIZED) == 0) + return -ENODEV; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + return 0; +} +#endif + +struct get_pages_work { + struct work_struct work; + struct drm_i915_gem_object *obj; + struct task_struct *task; +}; + +static void +__i915_gem_userptr_get_pages_worker(struct work_struct *_work) +{ + struct get_pages_work *work = container_of(_work, typeof(*work), work); + struct drm_i915_gem_object *obj = work->obj; + struct drm_device *dev = obj->base.dev; + const int num_pages = obj->base.size >> PAGE_SHIFT; + struct page **pvec; + int pinned, ret; + + ret = -ENOMEM; + pinned = 0; + + pvec = kmalloc(num_pages*sizeof(struct page *), + GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); + if (pvec == NULL) + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); + if (pvec != NULL) { + struct mm_struct *mm = obj->userptr.mm; + + use_mm(mm); + down_read(&mm->mmap_sem); + while (pinned < num_pages) { + ret = get_user_pages(work->task, mm, + obj->userptr.ptr + pinned * PAGE_SIZE, + num_pages - pinned, + !obj->userptr.read_only, 0, + pvec + pinned, NULL); + if (ret < 0) + break; + + pinned += ret; + } + up_read(&mm->mmap_sem); + unuse_mm(mm); + } + + mutex_lock(&dev->struct_mutex); + if (obj->userptr.work != &work->work) { + ret = 0; + } else if (pinned == num_pages) { + struct sg_table *st; + + st = kmalloc(sizeof(*st), GFP_KERNEL); + if (st == NULL || sg_alloc_table(st, num_pages, GFP_KERNEL)) { + kfree(st); + ret = -ENOMEM; + } else { + struct scatterlist *sg; + int n; + + for_each_sg(st->sgl, sg, num_pages, n) + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); + + obj->pages = st; + list_add_tail(&obj->global_list, &to_i915(dev)->mm.unbound_list); + pinned = 0; + ret = 0; + } + } + + obj->userptr.work = ERR_PTR(ret); + drm_gem_object_unreference(&obj->base); + mutex_unlock(&dev->struct_mutex); + + release_pages(pvec, pinned, 0); + drm_free_large(pvec); + + put_task_struct(work->task); + kfree(work); +} + +static int +i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) +{ + const int num_pages = obj->base.size >> PAGE_SHIFT; + struct page **pvec; + int pinned, ret; + + /* If userspace should engineer that these pages are replaced in + * the vma between us binding this page into the GTT and completion + * of rendering... Their loss. If they change the mapping of their + * pages they need to create a new bo to point to the new vma. + * + * However, that still leaves open the possibility of the vma + * being copied upon fork. Which falls under the same userspace + * synchronisation issue as a regular bo, except that this time + * the process may not be expecting that a particular piece of + * memory is tied to the GPU. + * + * Fortunately, we can hook into the mmu_notifier in order to + * discard the page references prior to anything nasty happening + * to the vma (discard or cloning) which should prevent the more + * egregious cases from causing harm. + */ + + pvec = NULL; + pinned = 0; + if (obj->userptr.mm == current->mm) { + pvec = kmalloc(num_pages*sizeof(struct page *), + GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); + if (pvec == NULL) { + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); + if (pvec == NULL) + return -ENOMEM; + } + + pinned = __get_user_pages_fast(obj->userptr.ptr, num_pages, + !obj->userptr.read_only, pvec); + } + if (pinned < num_pages) { + if (pinned < 0) { + ret = pinned; + pinned = 0; + } else { + /* Spawn a worker so that we can acquire the + * user pages without holding our mutex. + */ + ret = -EAGAIN; + if (obj->userptr.work == NULL) { + struct get_pages_work *work; + + work = kmalloc(sizeof(*work), GFP_KERNEL); + if (work != NULL) { + obj->userptr.work = &work->work; + + work->obj = obj; + drm_gem_object_reference(&obj->base); + + work->task = current; + get_task_struct(work->task); + + INIT_WORK(&work->work, __i915_gem_userptr_get_pages_worker); + schedule_work(&work->work); + } else + ret = -ENOMEM; + } else { + if (IS_ERR(obj->userptr.work)) { + ret = PTR_ERR(obj->userptr.work); + obj->userptr.work = NULL; + } + } + } + } else { + struct sg_table *st; + + st = kmalloc(sizeof(*st), GFP_KERNEL); + if (st == NULL || sg_alloc_table(st, num_pages, GFP_KERNEL)) { + kfree(st); + ret = -ENOMEM; + } else { + struct scatterlist *sg; + int n; + + for_each_sg(st->sgl, sg, num_pages, n) + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); + + obj->pages = st; + obj->userptr.work = NULL; + + pinned = 0; + ret = 0; + } + } + + release_pages(pvec, pinned, 0); + drm_free_large(pvec); + return ret; +} + +static void +i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj) +{ + struct scatterlist *sg; + int i; + + if (obj->madv != I915_MADV_WILLNEED) + obj->dirty = 0; + + for_each_sg(obj->pages->sgl, sg, obj->pages->nents, i) { + struct page *page = sg_page(sg); + + if (obj->dirty) + set_page_dirty(page); + + mark_page_accessed(page); + page_cache_release(page); + } + obj->dirty = 0; + + sg_free_table(obj->pages); + kfree(obj->pages); + + BUG_ON(obj->userptr.work != NULL); +} + +static void +i915_gem_userptr_release(struct drm_i915_gem_object *obj) +{ + i915_gem_userptr_release__mmu_notifier(obj); + + if (obj->userptr.mm) { + mmput(obj->userptr.mm); + obj->userptr.mm = NULL; + } +} + +static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = { + .get_pages = i915_gem_userptr_get_pages, + .put_pages = i915_gem_userptr_put_pages, + .release = i915_gem_userptr_release, +}; + +/** + * Creates a new mm object that wraps some normal memory from the process + * context - user memory. + * + * We impose several restrictions upon the memory being mapped + * into the GPU. + * 1. It must be page aligned (both start/end addresses, i.e ptr and size). + * 2. We only allow a bo as large as we could in theory map into the GTT, + * that is we limit the size to the total size of the GTT. + * 3. The bo is marked as being snoopable. The backing pages are left + * accessible directly by the CPU, but reads by the GPU may incur the cost + * of a snoop (unless you have an LLC architecture). + * + * Synchronisation between multiple users and the GPU is left to userspace + * through the normal set-domain-ioctl. The kernel will enforce that the + * GPU relinquishes the VMA before it is returned back to the system + * i.e. upon free(), munmap() or process termination. However, the userspace + * malloc() library may not immediately relinquish the VMA after free() and + * instead reuse it whilst the GPU is still reading and writing to the VMA. + * Caveat emptor. + * + * Also note, that the object created here is not currently a "first class" + * object, in that several ioctls are banned. These are the CPU access + * ioctls: mmap(), pwrite and pread. In practice, you are expected to use + * direct access via your pointer rather than use those ioctls. + * + * If you think this is a good interface to use to pass GPU memory between + * drivers, please use dma-buf instead. In fact, wherever possible use + * dma-buf instead. + */ +int +i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_gem_userptr *args = data; + struct drm_i915_gem_object *obj; + int ret; + u32 handle; + + if (args->flags & ~(I915_USERPTR_READ_ONLY | + I915_USERPTR_UNSYNCHRONIZED)) + return -EINVAL; + + if (offset_in_page(args->user_ptr | args->user_size)) + return -EINVAL; + + if (args->user_size > dev_priv->gtt.base.total) + return -E2BIG; + + if (!access_ok(args->flags & I915_USERPTR_READ_ONLY ? VERIFY_READ : VERIFY_WRITE, + (char __user *)(unsigned long)args->user_ptr, args->user_size)) + return -EFAULT; + + /* Allocate the new object */ + obj = i915_gem_object_alloc(dev); + if (obj == NULL) + return -ENOMEM; + + drm_gem_private_object_init(dev, &obj->base, args->user_size); + i915_gem_object_init(obj, &i915_gem_userptr_ops); + obj->cache_level = I915_CACHE_LLC; + obj->base.write_domain = I915_GEM_DOMAIN_CPU; + obj->base.read_domains = I915_GEM_DOMAIN_CPU; + + obj->userptr.ptr = args->user_ptr; + obj->userptr.read_only = !!(args->flags & I915_USERPTR_READ_ONLY); + + /* And keep a pointer to the current->mm for resolving the user pages + * at binding. This means that we need to hook into the mmu_notifier + * in order to detect if the mmu is destroyed. + */ + ret = -ENOMEM; + if ((obj->userptr.mm = get_task_mm(current))) + ret = i915_gem_userptr_init__mmu_notifier(obj, args->flags); + if (ret == 0) + ret = drm_gem_handle_create(file, &obj->base, &handle); + + /* drop reference from allocate - handle holds it now */ + drm_gem_object_unreference_unlocked(&obj->base); + if (ret) + return ret; + + args->handle = handle; + return 0; +} + +int +i915_gem_init_userptr(struct drm_device *dev) +{ +#if defined(CONFIG_MMU_NOTIFIER) + struct drm_i915_private *dev_priv = to_i915(dev); + hash_init(dev_priv->mmu_notifiers); +#endif + return 0; +} diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index ae8cf61b8ce3..cce9f559e3d7 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -201,6 +201,7 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m, err_puts(m, tiling_flag(err->tiling)); err_puts(m, dirty_flag(err->dirty)); err_puts(m, purgeable_flag(err->purgeable)); + err_puts(m, err->userptr ? " userptr" : ""); err_puts(m, err->ring != -1 ? " " : ""); err_puts(m, ring_str(err->ring)); err_puts(m, i915_cache_level_str(err->cache_level)); @@ -584,6 +585,7 @@ static void capture_bo(struct drm_i915_error_buffer *err, err->tiling = obj->tiling_mode; err->dirty = obj->dirty; err->purgeable = obj->madv != I915_MADV_WILLNEED; + err->userptr = obj->userptr.mm != 0; err->ring = obj->ring ? obj->ring->id : -1; err->cache_level = obj->cache_level; } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 37c8073a8246..6c145a0be250 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -224,6 +224,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_REG_READ 0x31 #define DRM_I915_GET_RESET_STATS 0x32 #define DRM_I915_GEM_CREATE2 0x33 +#define DRM_I915_GEM_USERPTR 0x34 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -275,6 +276,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy) #define DRM_IOCTL_I915_REG_READ DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read) #define DRM_IOCTL_I915_GET_RESET_STATS DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GET_RESET_STATS, struct drm_i915_reset_stats) +#define DRM_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1129,4 +1131,18 @@ struct drm_i915_reset_stats { __u32 pad; }; +struct drm_i915_gem_userptr { + __u64 user_ptr; + __u64 user_size; + __u32 flags; +#define I915_USERPTR_READ_ONLY 0x1 +#define I915_USERPTR_UNSYNCHRONIZED 0x80000000 + /** + * Returned handle for the object. + * + * Object handles are nonzero. + */ + __u32 handle; +}; + #endif /* _UAPI_I915_DRM_H_ */ -- 1.8.5.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl 2014-01-24 9:00 ` Chris Wilson @ 2014-01-27 17:56 ` Volkin, Bradley D 2014-01-27 18:09 ` Chris Wilson 0 siblings, 1 reply; 16+ messages in thread From: Volkin, Bradley D @ 2014-01-27 17:56 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx@lists.freedesktop.org, Goel, Akash Hi Chris, A few questions/comments throughout. I may be off the mark on some. Please bear with me as I try to get more familiar with the gem code. Thanks, Brad [ snip ] On Fri, Jan 24, 2014 at 01:00:19AM -0800, Chris Wilson wrote: > +static void > +__i915_mmu_notifier_destroy_worker(struct work_struct *work) > +{ > + struct i915_mmu_notifier *mmu = container_of(work, typeof(*mmu), work); > + mmu_notifier_unregister(&mmu->mn, mmu->mm); > + kfree(mmu); > +} > + > +static void > +__i915_mmu_notifier_destroy(struct i915_mmu_notifier *mmu) > +{ > + hash_del(&mmu->node); > + INIT_WORK(&mmu->work, __i915_mmu_notifier_destroy_worker); > + schedule_work(&mmu->work); The commit message mentions a potential lock inversion as the reason for using a wq. A comment with the details might be helpful. > +} > + > +static void __i915_mmu_notifier_update_serial(struct i915_mmu_notifier *mmu) > +{ > + if (++mmu->serial == 0) > + mmu->serial = 1; > +} > + > +static void > +i915_mmu_notifier_del(struct i915_mmu_notifier *mmu, > + struct i915_mmu_object *mn) > +{ > + bool destroy; > + > + spin_lock(&mmu->lock); > + interval_tree_remove(&mn->it, &mmu->objects); > + destroy = --mmu->count == 0; > + __i915_mmu_notifier_update_serial(mmu); > + spin_unlock(&mmu->lock); > + > + if (destroy) /* protected against _add() by struct_mutex */ > + __i915_mmu_notifier_destroy(mmu); I see that we should hold struct_mutex when this function is called, but I don't see that we try to get the mutex anywhere on the _add() path. Have I missed something? > +} > + > +static int > +i915_mmu_notifier_add(struct i915_mmu_notifier *mmu, > + struct i915_mmu_object *mn) > +{ > + int ret = -EINVAL; > + > + spin_lock(&mmu->lock); > + /* Disallow overlapping userptr objects */ > + if (!interval_tree_iter_first(&mmu->objects, > + mn->it.start, mn->it.last)) { > + interval_tree_insert(&mn->it, &mmu->objects); > + mmu->count++; > + __i915_mmu_notifier_update_serial(mmu); > + ret = 0; > + } > + spin_unlock(&mmu->lock); > + > + return ret; > +} > + > +static void > +i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj) > +{ > + struct i915_mmu_object *mn; > + > + mn = obj->userptr.mn; > + if (mn == NULL) > + return; > + > + i915_mmu_notifier_del(mn->mmu, mn); > + obj->userptr.mn = NULL; > +} > + > +static int > +i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, > + unsigned flags) > +{ > + struct i915_mmu_notifier *mmu; > + struct i915_mmu_object *mn; > + int ret; > + > + if (flags & I915_USERPTR_UNSYNCHRONIZED) > + return capable(CAP_SYS_ADMIN) ? 0 : -EPERM; > + > + mmu = i915_mmu_notifier_get(obj->base.dev, obj->userptr.mm); > + if (IS_ERR(mmu)) > + return PTR_ERR(mmu); > + > + mn = kzalloc(sizeof(*mn), GFP_KERNEL); > + if (mn == NULL) { > + ret = -ENOMEM; > + goto destroy_mmu; > + } > + > + mn->mmu = mmu; > + mn->it.start = obj->userptr.ptr; > + mn->it.last = mn->it.start + obj->base.size - 1; > + mn->obj = obj; > + > + ret = i915_mmu_notifier_add(mmu, mn); > + if (ret) > + goto free_mn; > + > + obj->userptr.mn = mn; > + return 0; > + > +free_mn: > + kfree(mn); > +destroy_mmu: > + if (mmu->count == 0) > + __i915_mmu_notifier_destroy(mmu); Other accesses to mmu->count are protected by mmu->lock. Again, I may have missed something but don't immediately see why that's not required. > + return ret; > +} > + > +#else > + > +static void > +i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj) > +{ > +} > + > +static int > +i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, > + unsigned flags) > +{ > + if ((flags & I915_USERPTR_UNSYNCHRONIZED) == 0) > + return -ENODEV; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + return 0; > +} > +#endif > + > +struct get_pages_work { > + struct work_struct work; > + struct drm_i915_gem_object *obj; > + struct task_struct *task; > +}; > + > +static void > +__i915_gem_userptr_get_pages_worker(struct work_struct *_work) > +{ > + struct get_pages_work *work = container_of(_work, typeof(*work), work); > + struct drm_i915_gem_object *obj = work->obj; > + struct drm_device *dev = obj->base.dev; > + const int num_pages = obj->base.size >> PAGE_SHIFT; > + struct page **pvec; > + int pinned, ret; > + > + ret = -ENOMEM; > + pinned = 0; > + > + pvec = kmalloc(num_pages*sizeof(struct page *), > + GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); > + if (pvec == NULL) > + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); > + if (pvec != NULL) { > + struct mm_struct *mm = obj->userptr.mm; > + > + use_mm(mm); > + down_read(&mm->mmap_sem); > + while (pinned < num_pages) { > + ret = get_user_pages(work->task, mm, > + obj->userptr.ptr + pinned * PAGE_SIZE, > + num_pages - pinned, > + !obj->userptr.read_only, 0, > + pvec + pinned, NULL); > + if (ret < 0) > + break; > + > + pinned += ret; > + } > + up_read(&mm->mmap_sem); > + unuse_mm(mm); > + } > + > + mutex_lock(&dev->struct_mutex); > + if (obj->userptr.work != &work->work) { > + ret = 0; > + } else if (pinned == num_pages) { > + struct sg_table *st; > + > + st = kmalloc(sizeof(*st), GFP_KERNEL); > + if (st == NULL || sg_alloc_table(st, num_pages, GFP_KERNEL)) { > + kfree(st); > + ret = -ENOMEM; > + } else { > + struct scatterlist *sg; > + int n; > + > + for_each_sg(st->sgl, sg, num_pages, n) > + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); > + > + obj->pages = st; > + list_add_tail(&obj->global_list, &to_i915(dev)->mm.unbound_list); > + pinned = 0; > + ret = 0; > + } > + } > + > + obj->userptr.work = ERR_PTR(ret); > + drm_gem_object_unreference(&obj->base); > + mutex_unlock(&dev->struct_mutex); > + > + release_pages(pvec, pinned, 0); > + drm_free_large(pvec); > + > + put_task_struct(work->task); > + kfree(work); > +} > + > +static int > +i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) > +{ > + const int num_pages = obj->base.size >> PAGE_SHIFT; > + struct page **pvec; > + int pinned, ret; > + > + /* If userspace should engineer that these pages are replaced in > + * the vma between us binding this page into the GTT and completion > + * of rendering... Their loss. If they change the mapping of their > + * pages they need to create a new bo to point to the new vma. > + * > + * However, that still leaves open the possibility of the vma > + * being copied upon fork. Which falls under the same userspace > + * synchronisation issue as a regular bo, except that this time > + * the process may not be expecting that a particular piece of > + * memory is tied to the GPU. > + * > + * Fortunately, we can hook into the mmu_notifier in order to > + * discard the page references prior to anything nasty happening > + * to the vma (discard or cloning) which should prevent the more > + * egregious cases from causing harm. > + */ > + > + pvec = NULL; > + pinned = 0; > + if (obj->userptr.mm == current->mm) { > + pvec = kmalloc(num_pages*sizeof(struct page *), > + GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY); > + if (pvec == NULL) { > + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); > + if (pvec == NULL) > + return -ENOMEM; > + } > + > + pinned = __get_user_pages_fast(obj->userptr.ptr, num_pages, > + !obj->userptr.read_only, pvec); > + } > + if (pinned < num_pages) { > + if (pinned < 0) { > + ret = pinned; > + pinned = 0; > + } else { > + /* Spawn a worker so that we can acquire the > + * user pages without holding our mutex. > + */ > + ret = -EAGAIN; > + if (obj->userptr.work == NULL) { > + struct get_pages_work *work; > + > + work = kmalloc(sizeof(*work), GFP_KERNEL); > + if (work != NULL) { > + obj->userptr.work = &work->work; > + > + work->obj = obj; > + drm_gem_object_reference(&obj->base); > + > + work->task = current; > + get_task_struct(work->task); > + > + INIT_WORK(&work->work, __i915_gem_userptr_get_pages_worker); > + schedule_work(&work->work); Any reason to use the system wq instead of the driver wq here? It doesn't look like it's the usual "takes modeset locks" justification. > + } else > + ret = -ENOMEM; > + } else { > + if (IS_ERR(obj->userptr.work)) { } else if (...) { ? > + ret = PTR_ERR(obj->userptr.work); > + obj->userptr.work = NULL; > + } > + } > + } > + } else { > + struct sg_table *st; > + > + st = kmalloc(sizeof(*st), GFP_KERNEL); > + if (st == NULL || sg_alloc_table(st, num_pages, GFP_KERNEL)) { > + kfree(st); > + ret = -ENOMEM; > + } else { > + struct scatterlist *sg; > + int n; > + > + for_each_sg(st->sgl, sg, num_pages, n) > + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); > + > + obj->pages = st; > + obj->userptr.work = NULL; > + > + pinned = 0; > + ret = 0; > + } This block is almost identical to code in the worker. Would it be worth extracting the common parts into a helper? > + } > + > + release_pages(pvec, pinned, 0); > + drm_free_large(pvec); > + return ret; > +} > + > +static void > +i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj) > +{ > + struct scatterlist *sg; > + int i; > + > + if (obj->madv != I915_MADV_WILLNEED) > + obj->dirty = 0; This is subtly different than similar code in the standard put_pages() in that it sets dirty=0 for both DONTNEED and PURGED instead of just DONTNEED (w/ BUG_ON(PURGED)). I don't think we will ever actually truncate userptr objects, so is there any reason for this to be different? > + > + for_each_sg(obj->pages->sgl, sg, obj->pages->nents, i) { > + struct page *page = sg_page(sg); > + > + if (obj->dirty) > + set_page_dirty(page); > + > + mark_page_accessed(page); > + page_cache_release(page); > + } > + obj->dirty = 0; > + > + sg_free_table(obj->pages); > + kfree(obj->pages); > + > + BUG_ON(obj->userptr.work != NULL); > +} > + > +static void > +i915_gem_userptr_release(struct drm_i915_gem_object *obj) > +{ > + i915_gem_userptr_release__mmu_notifier(obj); > + > + if (obj->userptr.mm) { > + mmput(obj->userptr.mm); > + obj->userptr.mm = NULL; > + } > +} > + > +static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = { > + .get_pages = i915_gem_userptr_get_pages, > + .put_pages = i915_gem_userptr_put_pages, > + .release = i915_gem_userptr_release, > +}; > + > +/** > + * Creates a new mm object that wraps some normal memory from the process > + * context - user memory. > + * > + * We impose several restrictions upon the memory being mapped > + * into the GPU. > + * 1. It must be page aligned (both start/end addresses, i.e ptr and size). > + * 2. We only allow a bo as large as we could in theory map into the GTT, > + * that is we limit the size to the total size of the GTT. > + * 3. The bo is marked as being snoopable. The backing pages are left > + * accessible directly by the CPU, but reads by the GPU may incur the cost > + * of a snoop (unless you have an LLC architecture). No overlapping ranges - Brad > + * > + * Synchronisation between multiple users and the GPU is left to userspace > + * through the normal set-domain-ioctl. The kernel will enforce that the > + * GPU relinquishes the VMA before it is returned back to the system > + * i.e. upon free(), munmap() or process termination. However, the userspace > + * malloc() library may not immediately relinquish the VMA after free() and > + * instead reuse it whilst the GPU is still reading and writing to the VMA. > + * Caveat emptor. > + * > + * Also note, that the object created here is not currently a "first class" > + * object, in that several ioctls are banned. These are the CPU access > + * ioctls: mmap(), pwrite and pread. In practice, you are expected to use > + * direct access via your pointer rather than use those ioctls. > + * > + * If you think this is a good interface to use to pass GPU memory between > + * drivers, please use dma-buf instead. In fact, wherever possible use > + * dma-buf instead. > + */ > +int > +i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_i915_gem_userptr *args = data; > + struct drm_i915_gem_object *obj; > + int ret; > + u32 handle; > + > + if (args->flags & ~(I915_USERPTR_READ_ONLY | > + I915_USERPTR_UNSYNCHRONIZED)) > + return -EINVAL; > + > + if (offset_in_page(args->user_ptr | args->user_size)) > + return -EINVAL; > + > + if (args->user_size > dev_priv->gtt.base.total) > + return -E2BIG; > + > + if (!access_ok(args->flags & I915_USERPTR_READ_ONLY ? VERIFY_READ : VERIFY_WRITE, > + (char __user *)(unsigned long)args->user_ptr, args->user_size)) > + return -EFAULT; > + > + /* Allocate the new object */ > + obj = i915_gem_object_alloc(dev); > + if (obj == NULL) > + return -ENOMEM; > + > + drm_gem_private_object_init(dev, &obj->base, args->user_size); > + i915_gem_object_init(obj, &i915_gem_userptr_ops); > + obj->cache_level = I915_CACHE_LLC; > + obj->base.write_domain = I915_GEM_DOMAIN_CPU; > + obj->base.read_domains = I915_GEM_DOMAIN_CPU; > + > + obj->userptr.ptr = args->user_ptr; > + obj->userptr.read_only = !!(args->flags & I915_USERPTR_READ_ONLY); > + > + /* And keep a pointer to the current->mm for resolving the user pages > + * at binding. This means that we need to hook into the mmu_notifier > + * in order to detect if the mmu is destroyed. > + */ > + ret = -ENOMEM; > + if ((obj->userptr.mm = get_task_mm(current))) > + ret = i915_gem_userptr_init__mmu_notifier(obj, args->flags); > + if (ret == 0) > + ret = drm_gem_handle_create(file, &obj->base, &handle); > + > + /* drop reference from allocate - handle holds it now */ > + drm_gem_object_unreference_unlocked(&obj->base); > + if (ret) > + return ret; > + > + args->handle = handle; > + return 0; > +} > + > +int > +i915_gem_init_userptr(struct drm_device *dev) > +{ > +#if defined(CONFIG_MMU_NOTIFIER) > + struct drm_i915_private *dev_priv = to_i915(dev); > + hash_init(dev_priv->mmu_notifiers); > +#endif > + return 0; > +} > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index ae8cf61b8ce3..cce9f559e3d7 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -201,6 +201,7 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m, > err_puts(m, tiling_flag(err->tiling)); > err_puts(m, dirty_flag(err->dirty)); > err_puts(m, purgeable_flag(err->purgeable)); > + err_puts(m, err->userptr ? " userptr" : ""); > err_puts(m, err->ring != -1 ? " " : ""); > err_puts(m, ring_str(err->ring)); > err_puts(m, i915_cache_level_str(err->cache_level)); > @@ -584,6 +585,7 @@ static void capture_bo(struct drm_i915_error_buffer *err, > err->tiling = obj->tiling_mode; > err->dirty = obj->dirty; > err->purgeable = obj->madv != I915_MADV_WILLNEED; > + err->userptr = obj->userptr.mm != 0; > err->ring = obj->ring ? obj->ring->id : -1; > err->cache_level = obj->cache_level; > } > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 37c8073a8246..6c145a0be250 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -224,6 +224,7 @@ typedef struct _drm_i915_sarea { > #define DRM_I915_REG_READ 0x31 > #define DRM_I915_GET_RESET_STATS 0x32 > #define DRM_I915_GEM_CREATE2 0x33 > +#define DRM_I915_GEM_USERPTR 0x34 > > #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) > #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) > @@ -275,6 +276,7 @@ typedef struct _drm_i915_sarea { > #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy) > #define DRM_IOCTL_I915_REG_READ DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read) > #define DRM_IOCTL_I915_GET_RESET_STATS DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GET_RESET_STATS, struct drm_i915_reset_stats) > +#define DRM_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr) > > /* Allow drivers to submit batchbuffers directly to hardware, relying > * on the security mechanisms provided by hardware. > @@ -1129,4 +1131,18 @@ struct drm_i915_reset_stats { > __u32 pad; > }; > > +struct drm_i915_gem_userptr { > + __u64 user_ptr; > + __u64 user_size; > + __u32 flags; > +#define I915_USERPTR_READ_ONLY 0x1 > +#define I915_USERPTR_UNSYNCHRONIZED 0x80000000 > + /** > + * Returned handle for the object. > + * > + * Object handles are nonzero. > + */ > + __u32 handle; > +}; > + > #endif /* _UAPI_I915_DRM_H_ */ > -- > 1.8.5.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl 2014-01-27 17:56 ` Volkin, Bradley D @ 2014-01-27 18:09 ` Chris Wilson 0 siblings, 0 replies; 16+ messages in thread From: Chris Wilson @ 2014-01-27 18:09 UTC (permalink / raw) To: Volkin, Bradley D; +Cc: intel-gfx@lists.freedesktop.org, Goel, Akash On Mon, Jan 27, 2014 at 09:56:12AM -0800, Volkin, Bradley D wrote: > > +static void > > +i915_mmu_notifier_del(struct i915_mmu_notifier *mmu, > > + struct i915_mmu_object *mn) > > +{ > > + bool destroy; > > + > > + spin_lock(&mmu->lock); > > + interval_tree_remove(&mn->it, &mmu->objects); > > + destroy = --mmu->count == 0; > > + __i915_mmu_notifier_update_serial(mmu); > > + spin_unlock(&mmu->lock); > > + > > + if (destroy) /* protected against _add() by struct_mutex */ > > + __i915_mmu_notifier_destroy(mmu); > > I see that we should hold struct_mutex when this function is called, > but I don't see that we try to get the mutex anywhere on the _add() path. > Have I missed something? No, it's fixed in a later patch. I assumed I had the lock taken in the outermost ioctl routine. > > +free_mn: > > + kfree(mn); > > +destroy_mmu: > > + if (mmu->count == 0) > > + __i915_mmu_notifier_destroy(mmu); > > Other accesses to mmu->count are protected by mmu->lock. Again, I may have missed > something but don't immediately see why that's not required. mmu->count is protected by struct_mutex. See above. > > + if (pinned < num_pages) { > > + if (pinned < 0) { > > + ret = pinned; > > + pinned = 0; > > + } else { > > + /* Spawn a worker so that we can acquire the > > + * user pages without holding our mutex. > > + */ > > + ret = -EAGAIN; > > + if (obj->userptr.work == NULL) { > > + struct get_pages_work *work; > > + > > + work = kmalloc(sizeof(*work), GFP_KERNEL); > > + if (work != NULL) { > > + obj->userptr.work = &work->work; > > + > > + work->obj = obj; > > + drm_gem_object_reference(&obj->base); > > + > > + work->task = current; > > + get_task_struct(work->task); > > + > > + INIT_WORK(&work->work, __i915_gem_userptr_get_pages_worker); > > + schedule_work(&work->work); > > Any reason to use the system wq instead of the driver wq here? > It doesn't look like it's the usual "takes modeset locks" justification. Performance. Using the driver workqueue would serialise the clients, but using the system workqueue we can do the pagefaulting in parallel. > > > + } else > > + ret = -ENOMEM; > > + } else { > > + if (IS_ERR(obj->userptr.work)) { > > } else if (...) { ? No, I think it is clearer as is. > > + ret = PTR_ERR(obj->userptr.work); > > + obj->userptr.work = NULL; > > + } > > + } > > + } > > + } else { > > + struct sg_table *st; > > + > > + st = kmalloc(sizeof(*st), GFP_KERNEL); > > + if (st == NULL || sg_alloc_table(st, num_pages, GFP_KERNEL)) { > > + kfree(st); > > + ret = -ENOMEM; > > + } else { > > + struct scatterlist *sg; > > + int n; > > + > > + for_each_sg(st->sgl, sg, num_pages, n) > > + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); > > + > > + obj->pages = st; > > + obj->userptr.work = NULL; > > + > > + pinned = 0; > > + ret = 0; > > + } > > This block is almost identical to code in the worker. Would it be worth extracting > the common parts into a helper? Almost, but subtly and importantly different. Only the loop was the same at which point I didn't feel like the saving was significant. It is now even less similar. > > + } > > + > > + release_pages(pvec, pinned, 0); > > + drm_free_large(pvec); > > + return ret; > > +} > > + > > +static void > > +i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj) > > +{ > > + struct scatterlist *sg; > > + int i; > > + > > + if (obj->madv != I915_MADV_WILLNEED) > > + obj->dirty = 0; > > This is subtly different than similar code in the standard put_pages() in that > it sets dirty=0 for both DONTNEED and PURGED instead of just DONTNEED (w/ BUG_ON(PURGED)). > I don't think we will ever actually truncate userptr objects, so is there any reason for > this to be different? No, there's no reason for the difference. Semantically it is the same, of course. > > +/** > > + * Creates a new mm object that wraps some normal memory from the process > > + * context - user memory. > > + * > > + * We impose several restrictions upon the memory being mapped > > + * into the GPU. > > + * 1. It must be page aligned (both start/end addresses, i.e ptr and size). > > + * 2. We only allow a bo as large as we could in theory map into the GTT, > > + * that is we limit the size to the total size of the GTT. > > + * 3. The bo is marked as being snoopable. The backing pages are left > > + * accessible directly by the CPU, but reads by the GPU may incur the cost > > + * of a snoop (unless you have an LLC architecture). > > No overlapping ranges Yes, that is an important addition. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] lib: Always build and export interval_tree 2014-01-21 15:07 [PATCH 1/3] lib: Always build and export interval_tree Chris Wilson 2014-01-21 15:07 ` [PATCH 2/3] drm/i915: Do not call retire_requests from wait_for_rendering Chris Wilson 2014-01-21 15:07 ` [PATCH 3/3] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl Chris Wilson @ 2014-01-25 4:42 ` Michel Lespinasse 2014-01-26 11:27 ` Chris Wilson 2014-01-26 12:24 ` [PATCH] lib: Export interval_tree Chris Wilson 3 siblings, 1 reply; 16+ messages in thread From: Michel Lespinasse @ 2014-01-25 4:42 UTC (permalink / raw) To: Chris Wilson Cc: Andrea Arcangeli, Rik van Riel, Peter Zijlstra, intel-gfx, Akash Goel, Andrew Morton Hi Chris, On Tue, Jan 21, 2014 at 7:07 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > lib/interval_tree.c provides a simple interface for an interval-tree > (an augmented red-black tree) but is only built when testing the generic > macros for building interval-trees. For drivers with modest needs, > export the simple interval-tree library as is. Thanks for suggesting this. I did plan for this use case, and thought it would show up earlier. My only concern is that I think we should keep the code under a config option (unless a use case shows up in core kernel). So I would suggest: in lib/Kconfig: config INTERVAL_TREE bool in lib/Kconfig.debug: make INTERVAL_TREE_TEST depend on m && DEBUG_KERNEL && INTERVAL_TREE in lib/Makefile: obj-$(CONFIG_INTERVAL_TREE) += interval_tree.o You would probably also need to add #include <linux/module.h> in lib/interval_tree.c to plan for that code being configured as a module. Hope this helps, -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] lib: Always build and export interval_tree 2014-01-25 4:42 ` [PATCH 1/3] lib: Always build and export interval_tree Michel Lespinasse @ 2014-01-26 11:27 ` Chris Wilson 2014-01-26 11:58 ` Michel Lespinasse 0 siblings, 1 reply; 16+ messages in thread From: Chris Wilson @ 2014-01-26 11:27 UTC (permalink / raw) To: Michel Lespinasse Cc: Andrea Arcangeli, Rik van Riel, Peter Zijlstra, intel-gfx, Akash Goel, Andrew Morton On Fri, Jan 24, 2014 at 08:42:18PM -0800, Michel Lespinasse wrote: > Hi Chris, > > On Tue, Jan 21, 2014 at 7:07 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > lib/interval_tree.c provides a simple interface for an interval-tree > > (an augmented red-black tree) but is only built when testing the generic > > macros for building interval-trees. For drivers with modest needs, > > export the simple interval-tree library as is. > > Thanks for suggesting this. I did plan for this use case, and thought > it would show up earlier. > > My only concern is that I think we should keep the code under a config > option (unless a use case shows up in core kernel). So I would > suggest: > > in lib/Kconfig: > config INTERVAL_TREE > bool +config INTERVAL_TREE + boolean + help + Simple, embeddable, interval-tree. Can find the start of an + overlapping range in log(n) time and then iterate over all + overlapping nodes. The algorithm is implemented as an + augmented rbtree. + + See: + + Documentation/rbtree.txt + + for more information. Though you probably have some better spiel. :) > in lib/Kconfig.debug: > make INTERVAL_TREE_TEST depend on m && DEBUG_KERNEL && INTERVAL_TREE Done. Are you sure you do not just want to select INTERVAL_TREE here to maintain the status quo? > in lib/Makefile: > obj-$(CONFIG_INTERVAL_TREE) += interval_tree.o > > You would probably also need to add #include <linux/module.h> in > lib/interval_tree.c to plan for that code being configured as a > module. > > Hope this helps, Thanks! -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] lib: Always build and export interval_tree 2014-01-26 11:27 ` Chris Wilson @ 2014-01-26 11:58 ` Michel Lespinasse 2014-01-26 12:25 ` Chris Wilson 0 siblings, 1 reply; 16+ messages in thread From: Michel Lespinasse @ 2014-01-26 11:58 UTC (permalink / raw) To: Chris Wilson, Michel Lespinasse, intel-gfx, Tvrtko Ursulin, Zhipeng Gong, Akash Goel, Rik van Riel, Peter Zijlstra, Andrea Arcangeli, David Woodhouse, Andrew Morton On Sun, Jan 26, 2014 at 3:27 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > +config INTERVAL_TREE > + boolean > + help > + Simple, embeddable, interval-tree. Can find the start of an > + overlapping range in log(n) time and then iterate over all > + overlapping nodes. The algorithm is implemented as an > + augmented rbtree. > + > + See: > + > + Documentation/rbtree.txt > + > + for more information. > > Though you probably have some better spiel. :) No, that's fine. I'm not good at writing these things either :) >> in lib/Kconfig.debug: >> make INTERVAL_TREE_TEST depend on m && DEBUG_KERNEL && INTERVAL_TREE > > Done. Are you sure you do not just want to select INTERVAL_TREE here to > maintain the status quo? You're right, select would actually be better. >> in lib/Makefile: >> obj-$(CONFIG_INTERVAL_TREE) += interval_tree.o One more thing I forget, lib/interval_tree_test_main.c should probably be renamed to interval_tree_test.c and in lib/Makefile the interval_tree_test-objs line forcing linking of interval_tree.o into the test module should be removed too. > Thanks! > -Chris I assume you're going to resend with these changes ? I'll be happy to aprove the patch then :) -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] lib: Always build and export interval_tree 2014-01-26 11:58 ` Michel Lespinasse @ 2014-01-26 12:25 ` Chris Wilson 0 siblings, 0 replies; 16+ messages in thread From: Chris Wilson @ 2014-01-26 12:25 UTC (permalink / raw) To: Michel Lespinasse Cc: Andrea Arcangeli, Rik van Riel, Peter Zijlstra, intel-gfx, Akash Goel, Andrew Morton On Sun, Jan 26, 2014 at 03:58:47AM -0800, Michel Lespinasse wrote: > On Sun, Jan 26, 2014 at 3:27 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > +config INTERVAL_TREE > > + boolean > > + help > > + Simple, embeddable, interval-tree. Can find the start of an > > + overlapping range in log(n) time and then iterate over all > > + overlapping nodes. The algorithm is implemented as an > > + augmented rbtree. > > + > > + See: > > + > > + Documentation/rbtree.txt > > + > > + for more information. > > > > Though you probably have some better spiel. :) > > No, that's fine. I'm not good at writing these things either :) > > >> in lib/Kconfig.debug: > >> make INTERVAL_TREE_TEST depend on m && DEBUG_KERNEL && INTERVAL_TREE > > > > Done. Are you sure you do not just want to select INTERVAL_TREE here to > > maintain the status quo? > > You're right, select would actually be better. > > >> in lib/Makefile: > >> obj-$(CONFIG_INTERVAL_TREE) += interval_tree.o > > One more thing I forget, lib/interval_tree_test_main.c should probably > be renamed to interval_tree_test.c and in lib/Makefile the > interval_tree_test-objs line forcing linking of interval_tree.o into > the test module should be removed too. > > > Thanks! > > -Chris > > I assume you're going to resend with these changes ? I'll be happy to > aprove the patch then :) Indeed. Resent with a wider audience (forgot to include lkml on the first pass). -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] lib: Export interval_tree 2014-01-21 15:07 [PATCH 1/3] lib: Always build and export interval_tree Chris Wilson ` (2 preceding siblings ...) 2014-01-25 4:42 ` [PATCH 1/3] lib: Always build and export interval_tree Michel Lespinasse @ 2014-01-26 12:24 ` Chris Wilson 2014-01-26 12:34 ` Michel Lespinasse ` (2 more replies) 3 siblings, 3 replies; 16+ messages in thread From: Chris Wilson @ 2014-01-26 12:24 UTC (permalink / raw) To: intel-gfx, linux-kernel Cc: Andrea Arcangeli, Rik van Riel, Peter Zijlstra, Andrew Morton, Michel Lespinasse lib/interval_tree.c provides a simple interface for an interval-tree (an augmented red-black tree) but is only built when testing the generic macros for building interval-trees. For drivers with modest needs, export the simple interval-tree library as is. v2: Lots of help from Michel Lespinasse to only compile the code as required: - make INTERVAL_TREE a config option - make INTERVAL_TREE_TEST select the library functions and sanitize the filenames & Makefile - prepare interval_tree for being built as a module if required Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michel Lespinasse <walken@google.com> Cc: Rik van Riel <riel@redhat.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Andrea Arcangeli <aarcange@redhat.com> Cc: David Woodhouse <dwmw2@infradead.org> Cc: Andrew Morton <akpm@linux-foundation.org> --- Note for maintainers, this is being proposed for use by i915.ko, so it may make the most sense to merge it via the drm/i915 tree in the next cycle. --- lib/Kconfig | 14 ++++++ lib/Kconfig.debug | 1 + lib/Makefile | 3 +- lib/interval_tree.c | 6 +++ lib/interval_tree_test.c | 106 ++++++++++++++++++++++++++++++++++++++++++ lib/interval_tree_test_main.c | 106 ------------------------------------------ 6 files changed, 128 insertions(+), 108 deletions(-) create mode 100644 lib/interval_tree_test.c delete mode 100644 lib/interval_tree_test_main.c diff --git a/lib/Kconfig b/lib/Kconfig index 991c98bc4a3f..04270aae4b60 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -322,6 +322,20 @@ config TEXTSEARCH_FSM config BTREE boolean +config INTERVAL_TREE + boolean + help + Simple, embeddable, interval-tree. Can find the start of an + overlapping range in log(n) time and then iterate over all + overlapping nodes. The algorithm is implemented as an + augmented rbtree. + + See: + + Documentation/rbtree.txt + + for more information. + config ASSOCIATIVE_ARRAY bool help diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index db25707aa41b..a29e9b84f102 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1478,6 +1478,7 @@ config RBTREE_TEST config INTERVAL_TREE_TEST tristate "Interval tree test" depends on m && DEBUG_KERNEL + select INTERVAL_TREE help A benchmark measuring the performance of the interval tree library diff --git a/lib/Makefile b/lib/Makefile index a459c31e8c6b..fc04948548ad 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -47,6 +47,7 @@ CFLAGS_hweight.o = $(subst $(quote),,$(CONFIG_ARCH_HWEIGHT_CFLAGS)) obj-$(CONFIG_GENERIC_HWEIGHT) += hweight.o obj-$(CONFIG_BTREE) += btree.o +obj-$(CONFIG_INTERVAL_TREE) += interval_tree.o obj-$(CONFIG_ASSOCIATIVE_ARRAY) += assoc_array.o obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o obj-$(CONFIG_DEBUG_LIST) += list_debug.o @@ -152,8 +153,6 @@ lib-$(CONFIG_LIBFDT) += $(libfdt_files) obj-$(CONFIG_RBTREE_TEST) += rbtree_test.o obj-$(CONFIG_INTERVAL_TREE_TEST) += interval_tree_test.o -interval_tree_test-objs := interval_tree_test_main.o interval_tree.o - obj-$(CONFIG_PERCPU_TEST) += percpu_test.o obj-$(CONFIG_ASN1) += asn1_decoder.o diff --git a/lib/interval_tree.c b/lib/interval_tree.c index e6eb406f2d65..e4109f624e51 100644 --- a/lib/interval_tree.c +++ b/lib/interval_tree.c @@ -1,6 +1,7 @@ #include <linux/init.h> #include <linux/interval_tree.h> #include <linux/interval_tree_generic.h> +#include <linux/module.h> #define START(node) ((node)->start) #define LAST(node) ((node)->last) @@ -8,3 +9,8 @@ INTERVAL_TREE_DEFINE(struct interval_tree_node, rb, unsigned long, __subtree_last, START, LAST,, interval_tree) + +EXPORT_SYMBOL(interval_tree_insert); +EXPORT_SYMBOL(interval_tree_remove); +EXPORT_SYMBOL(interval_tree_iter_first); +EXPORT_SYMBOL(interval_tree_iter_next); diff --git a/lib/interval_tree_test.c b/lib/interval_tree_test.c new file mode 100644 index 000000000000..245900b98c8e --- /dev/null +++ b/lib/interval_tree_test.c @@ -0,0 +1,106 @@ +#include <linux/module.h> +#include <linux/interval_tree.h> +#include <linux/random.h> +#include <asm/timex.h> + +#define NODES 100 +#define PERF_LOOPS 100000 +#define SEARCHES 100 +#define SEARCH_LOOPS 10000 + +static struct rb_root root = RB_ROOT; +static struct interval_tree_node nodes[NODES]; +static u32 queries[SEARCHES]; + +static struct rnd_state rnd; + +static inline unsigned long +search(unsigned long query, struct rb_root *root) +{ + struct interval_tree_node *node; + unsigned long results = 0; + + for (node = interval_tree_iter_first(root, query, query); node; + node = interval_tree_iter_next(node, query, query)) + results++; + return results; +} + +static void init(void) +{ + int i; + for (i = 0; i < NODES; i++) { + u32 a = prandom_u32_state(&rnd); + u32 b = prandom_u32_state(&rnd); + if (a <= b) { + nodes[i].start = a; + nodes[i].last = b; + } else { + nodes[i].start = b; + nodes[i].last = a; + } + } + for (i = 0; i < SEARCHES; i++) + queries[i] = prandom_u32_state(&rnd); +} + +static int interval_tree_test_init(void) +{ + int i, j; + unsigned long results; + cycles_t time1, time2, time; + + printk(KERN_ALERT "interval tree insert/remove"); + + prandom_seed_state(&rnd, 3141592653589793238ULL); + init(); + + time1 = get_cycles(); + + for (i = 0; i < PERF_LOOPS; i++) { + for (j = 0; j < NODES; j++) + interval_tree_insert(nodes + j, &root); + for (j = 0; j < NODES; j++) + interval_tree_remove(nodes + j, &root); + } + + time2 = get_cycles(); + time = time2 - time1; + + time = div_u64(time, PERF_LOOPS); + printk(" -> %llu cycles\n", (unsigned long long)time); + + printk(KERN_ALERT "interval tree search"); + + for (j = 0; j < NODES; j++) + interval_tree_insert(nodes + j, &root); + + time1 = get_cycles(); + + results = 0; + for (i = 0; i < SEARCH_LOOPS; i++) + for (j = 0; j < SEARCHES; j++) + results += search(queries[j], &root); + + time2 = get_cycles(); + time = time2 - time1; + + time = div_u64(time, SEARCH_LOOPS); + results = div_u64(results, SEARCH_LOOPS); + printk(" -> %llu cycles (%lu results)\n", + (unsigned long long)time, results); + + return -EAGAIN; /* Fail will directly unload the module */ +} + +static void interval_tree_test_exit(void) +{ + printk(KERN_ALERT "test exit\n"); +} + +module_init(interval_tree_test_init) +module_exit(interval_tree_test_exit) + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Michel Lespinasse"); +MODULE_DESCRIPTION("Interval Tree test"); diff --git a/lib/interval_tree_test_main.c b/lib/interval_tree_test_main.c deleted file mode 100644 index 245900b98c8e..000000000000 --- a/lib/interval_tree_test_main.c +++ /dev/null @@ -1,106 +0,0 @@ -#include <linux/module.h> -#include <linux/interval_tree.h> -#include <linux/random.h> -#include <asm/timex.h> - -#define NODES 100 -#define PERF_LOOPS 100000 -#define SEARCHES 100 -#define SEARCH_LOOPS 10000 - -static struct rb_root root = RB_ROOT; -static struct interval_tree_node nodes[NODES]; -static u32 queries[SEARCHES]; - -static struct rnd_state rnd; - -static inline unsigned long -search(unsigned long query, struct rb_root *root) -{ - struct interval_tree_node *node; - unsigned long results = 0; - - for (node = interval_tree_iter_first(root, query, query); node; - node = interval_tree_iter_next(node, query, query)) - results++; - return results; -} - -static void init(void) -{ - int i; - for (i = 0; i < NODES; i++) { - u32 a = prandom_u32_state(&rnd); - u32 b = prandom_u32_state(&rnd); - if (a <= b) { - nodes[i].start = a; - nodes[i].last = b; - } else { - nodes[i].start = b; - nodes[i].last = a; - } - } - for (i = 0; i < SEARCHES; i++) - queries[i] = prandom_u32_state(&rnd); -} - -static int interval_tree_test_init(void) -{ - int i, j; - unsigned long results; - cycles_t time1, time2, time; - - printk(KERN_ALERT "interval tree insert/remove"); - - prandom_seed_state(&rnd, 3141592653589793238ULL); - init(); - - time1 = get_cycles(); - - for (i = 0; i < PERF_LOOPS; i++) { - for (j = 0; j < NODES; j++) - interval_tree_insert(nodes + j, &root); - for (j = 0; j < NODES; j++) - interval_tree_remove(nodes + j, &root); - } - - time2 = get_cycles(); - time = time2 - time1; - - time = div_u64(time, PERF_LOOPS); - printk(" -> %llu cycles\n", (unsigned long long)time); - - printk(KERN_ALERT "interval tree search"); - - for (j = 0; j < NODES; j++) - interval_tree_insert(nodes + j, &root); - - time1 = get_cycles(); - - results = 0; - for (i = 0; i < SEARCH_LOOPS; i++) - for (j = 0; j < SEARCHES; j++) - results += search(queries[j], &root); - - time2 = get_cycles(); - time = time2 - time1; - - time = div_u64(time, SEARCH_LOOPS); - results = div_u64(results, SEARCH_LOOPS); - printk(" -> %llu cycles (%lu results)\n", - (unsigned long long)time, results); - - return -EAGAIN; /* Fail will directly unload the module */ -} - -static void interval_tree_test_exit(void) -{ - printk(KERN_ALERT "test exit\n"); -} - -module_init(interval_tree_test_init) -module_exit(interval_tree_test_exit) - -MODULE_LICENSE("GPL"); -MODULE_AUTHOR("Michel Lespinasse"); -MODULE_DESCRIPTION("Interval Tree test"); -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] lib: Export interval_tree 2014-01-26 12:24 ` [PATCH] lib: Export interval_tree Chris Wilson @ 2014-01-26 12:34 ` Michel Lespinasse 2014-01-26 13:54 ` Daniel Vetter 2014-01-27 23:13 ` Andrew Morton 2 siblings, 0 replies; 16+ messages in thread From: Michel Lespinasse @ 2014-01-26 12:34 UTC (permalink / raw) To: Chris Wilson Cc: intel-gfx, linux-kernel, Rik van Riel, Peter Zijlstra, Andrea Arcangeli, David Woodhouse, Andrew Morton On Sun, Jan 26, 2014 at 4:24 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > lib/interval_tree.c provides a simple interface for an interval-tree > (an augmented red-black tree) but is only built when testing the generic > macros for building interval-trees. For drivers with modest needs, > export the simple interval-tree library as is. > > v2: Lots of help from Michel Lespinasse to only compile the code > as required: > - make INTERVAL_TREE a config option > - make INTERVAL_TREE_TEST select the library functions > and sanitize the filenames & Makefile > - prepare interval_tree for being built as a module if required > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michel Lespinasse <walken@google.com> > Cc: Rik van Riel <riel@redhat.com> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: David Woodhouse <dwmw2@infradead.org> > Cc: Andrew Morton <akpm@linux-foundation.org> Reviewed-by: Michel Lespinasse <walken@google.com> Note for other reviewers, the lib/interval_tree_test.c is a straight rename of the prior lib/interval_tree_test_main.c file. > Note for maintainers, this is being proposed for use by i915.ko, so it > may make the most sense to merge it via the drm/i915 tree in the next > cycle. Sounds fine to me, unless we get any other use cases before then. -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lib: Export interval_tree 2014-01-26 12:24 ` [PATCH] lib: Export interval_tree Chris Wilson 2014-01-26 12:34 ` Michel Lespinasse @ 2014-01-26 13:54 ` Daniel Vetter 2014-01-26 14:03 ` [Intel-gfx] " Michel Lespinasse 2014-01-27 23:13 ` Andrew Morton 2 siblings, 1 reply; 16+ messages in thread From: Daniel Vetter @ 2014-01-26 13:54 UTC (permalink / raw) To: Chris Wilson Cc: Andrea Arcangeli, Rik van Riel, Peter Zijlstra, intel-gfx, linux-kernel, Andrew Morton, Michel Lespinasse On Sun, Jan 26, 2014 at 12:24:33PM +0000, Chris Wilson wrote: > lib/interval_tree.c provides a simple interface for an interval-tree > (an augmented red-black tree) but is only built when testing the generic > macros for building interval-trees. For drivers with modest needs, > export the simple interval-tree library as is. > > v2: Lots of help from Michel Lespinasse to only compile the code > as required: > - make INTERVAL_TREE a config option > - make INTERVAL_TREE_TEST select the library functions > and sanitize the filenames & Makefile > - prepare interval_tree for being built as a module if required > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michel Lespinasse <walken@google.com> > Cc: Rik van Riel <riel@redhat.com> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: David Woodhouse <dwmw2@infradead.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > --- [snip] > diff --git a/lib/interval_tree.c b/lib/interval_tree.c > index e6eb406f2d65..e4109f624e51 100644 > --- a/lib/interval_tree.c > +++ b/lib/interval_tree.c > @@ -1,6 +1,7 @@ > #include <linux/init.h> > #include <linux/interval_tree.h> > #include <linux/interval_tree_generic.h> > +#include <linux/module.h> > > #define START(node) ((node)->start) > #define LAST(node) ((node)->last) > @@ -8,3 +9,8 @@ > INTERVAL_TREE_DEFINE(struct interval_tree_node, rb, > unsigned long, __subtree_last, > START, LAST,, interval_tree) > + > +EXPORT_SYMBOL(interval_tree_insert); > +EXPORT_SYMBOL(interval_tree_remove); > +EXPORT_SYMBOL(interval_tree_iter_first); > +EXPORT_SYMBOL(interval_tree_iter_next); Hm, I've thought kernel coding style nowadays is to put the EXPORT_SYMBOL right below the definition of the function? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] lib: Export interval_tree 2014-01-26 13:54 ` Daniel Vetter @ 2014-01-26 14:03 ` Michel Lespinasse 0 siblings, 0 replies; 16+ messages in thread From: Michel Lespinasse @ 2014-01-26 14:03 UTC (permalink / raw) To: Chris Wilson, intel-gfx, linux-kernel, Andrea Arcangeli, Rik van Riel, Peter Zijlstra, Andrew Morton, Michel Lespinasse On Sun, Jan 26, 2014 at 5:54 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Sun, Jan 26, 2014 at 12:24:33PM +0000, Chris Wilson wrote: >> INTERVAL_TREE_DEFINE(struct interval_tree_node, rb, >> unsigned long, __subtree_last, >> START, LAST,, interval_tree) >> + >> +EXPORT_SYMBOL(interval_tree_insert); >> +EXPORT_SYMBOL(interval_tree_remove); >> +EXPORT_SYMBOL(interval_tree_iter_first); >> +EXPORT_SYMBOL(interval_tree_iter_next); > > Hm, I've thought kernel coding style nowadays is to put the EXPORT_SYMBOL > right below the definition of the function? INTERVAL_TREE_DEFINE generates the definitions of these functions. -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] lib: Export interval_tree 2014-01-26 12:24 ` [PATCH] lib: Export interval_tree Chris Wilson 2014-01-26 12:34 ` Michel Lespinasse 2014-01-26 13:54 ` Daniel Vetter @ 2014-01-27 23:13 ` Andrew Morton 2 siblings, 0 replies; 16+ messages in thread From: Andrew Morton @ 2014-01-27 23:13 UTC (permalink / raw) To: Chris Wilson Cc: Andrea Arcangeli, Rik van Riel, Peter Zijlstra, intel-gfx, linux-kernel, Michel Lespinasse On Sun, 26 Jan 2014 12:24:33 +0000 Chris Wilson <chris@chris-wilson.co.uk> wrote: > Note for maintainers, this is being proposed for use by i915.ko, so it > may make the most sense to merge it via the drm/i915 tree in the next > cycle. Yes, please proceed that way. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-01-27 23:13 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-21 15:07 [PATCH 1/3] lib: Always build and export interval_tree Chris Wilson 2014-01-21 15:07 ` [PATCH 2/3] drm/i915: Do not call retire_requests from wait_for_rendering Chris Wilson 2014-01-21 15:07 ` [PATCH 3/3] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl Chris Wilson 2014-01-22 9:46 ` [PATCH] " Chris Wilson 2014-01-24 9:00 ` Chris Wilson 2014-01-27 17:56 ` Volkin, Bradley D 2014-01-27 18:09 ` Chris Wilson 2014-01-25 4:42 ` [PATCH 1/3] lib: Always build and export interval_tree Michel Lespinasse 2014-01-26 11:27 ` Chris Wilson 2014-01-26 11:58 ` Michel Lespinasse 2014-01-26 12:25 ` Chris Wilson 2014-01-26 12:24 ` [PATCH] lib: Export interval_tree Chris Wilson 2014-01-26 12:34 ` Michel Lespinasse 2014-01-26 13:54 ` Daniel Vetter 2014-01-26 14:03 ` [Intel-gfx] " Michel Lespinasse 2014-01-27 23:13 ` Andrew Morton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox