[3/6] drm: add unified vma offset manager

Commit Message

David Herrmann July 1, 2013, 6:33 p.m. UTC

If we want to map GPU memory into user-space, we need to linearize the
addresses to not confuse mm-core. Currently, GEM and TTM both implement
their own offset-managers to assign a pgoff to each object for user-space
CPU access. GEM uses a hash-table, TTM uses an rbtree.

This patch provides a unified implementation that can be used to replace
both. TTM allows partial mmaps with a given offset, so we cannot use
hashtables as the start address may not be known at mmap time. Hence, we
use the rbtree-implementation of TTM.

We could easily update drm_mm to use an rbtree instead of a linked list
for it's object list and thus drop the rbtree from the vma-manager.
However, this would slow down drm_mm object allocation for all other
use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
Hence, use the separate tree but allow for later migration.

This is a rewrite of the 2012-proposal by David Airlie <[e-mail]>

Signed-off-by: David Herrmann <[e-mail]>
---
 drivers/gpu/drm/Makefile          |   2 +-
 drivers/gpu/drm/drm_vma_manager.c | 224 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_vma_manager.h     | 107 ++++++++++++++++++
 3 files changed, 332 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_vma_manager.c
 create mode 100644 include/drm/drm_vma_manager.h

Comments

Daniel Vetter July 1, 2013, 7:42 p.m. UTC | #1

On Mon, Jul 01, 2013 at 08:33:00PM +0200, David Herrmann wrote:
> If we want to map GPU memory into user-space, we need to linearize the
> addresses to not confuse mm-core. Currently, GEM and TTM both implement
> their own offset-managers to assign a pgoff to each object for user-space
> CPU access. GEM uses a hash-table, TTM uses an rbtree.
> 
> This patch provides a unified implementation that can be used to replace
> both. TTM allows partial mmaps with a given offset, so we cannot use
> hashtables as the start address may not be known at mmap time. Hence, we
> use the rbtree-implementation of TTM.
> 
> We could easily update drm_mm to use an rbtree instead of a linked list
> for it's object list and thus drop the rbtree from the vma-manager.
> However, this would slow down drm_mm object allocation for all other
> use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
> Hence, use the separate tree but allow for later migration.
> 
> This is a rewrite of the 2012-proposal by David Airlie <[e-mail]>
> 
> Signed-off-by: David Herrmann <[e-mail]>

This seems to sprinkle a lot of likely/unlikely. Imo those are only
justified in two cases:
- When we have real performance data proving that they're worth it.
- Sometimes they're also useful as self-documenting code for the truly
  unlikely. But early do-nothing returns and error handling are
  established code patterns, so imo don't qualify.

I think none of the likely/unlikely below qualify, so imo better to remove
them.

Second high-level review request: Can you please convert the very nice
documentation you already added into proper kerneldoc and link it up at an
appropriate section in the drm DocBook?

A few more comments below. I'll postpone reviewing the gem/ttm conversion
patches until this is settled a bit.

> ---
>  drivers/gpu/drm/Makefile          |   2 +-
>  drivers/gpu/drm/drm_vma_manager.c | 224 ++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_vma_manager.h     | 107 ++++++++++++++++++
>  3 files changed, 332 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_vma_manager.c
>  create mode 100644 include/drm/drm_vma_manager.h
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1c9f2439..f8851cc 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -12,7 +12,7 @@ drm-y       :=	drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
>  		drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
>  		drm_crtc.o drm_modes.o drm_edid.o \
>  		drm_info.o drm_debugfs.o drm_encoder_slave.o \
> -		drm_trace_points.o drm_global.o drm_prime.o
> +		drm_trace_points.o drm_global.o drm_prime.o drm_vma_manager.o
>  
>  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
> new file mode 100644
> index 0000000..c28639f
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_vma_manager.c
> @@ -0,0 +1,224 @@
> +/*
> + * Copyright (c) 2012 David Airlie <[e-mail]>
> + * Copyright (c) 2013 David Herrmann <[e-mail]>
> + *
> + * 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 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 <drm/drmP.h>
> +#include <drm/drm_mm.h>
> +#include <drm/drm_vma_manager.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/rbtree.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +/** @file drm_vma_manager.c
> + *
> + * The vma-manager is responsible to map arbitrary driver-dependent memory
> + * regions into the linear user address-space. It provides offsets to the
> + * caller which can then be used on the address_space of the drm-device. It
> + * takes care to not overlap regions, size them appropriately and to not
> + * confuse mm-core by inconsistent fake vm_pgoff fields.
> + *
> + * We use drm_mm as backend to manage object allocations. But it is highly
> + * optimized for alloc/free calls, not lookups. Hence, we use an rb-tree to
> + * speed up offset lookups.
> + */
> +
> +/** drm_vma_offset_manager_init()
> + *
> + * Initialize a new offset-manager. The offset and area size available for the
> + * manager are given as @page_offset and @size. Both are interpreted as
> + * page-numbers, not bytes.
> + *
> + * Adding/removing nodes from the manager is locked internally and protected
> + * against concurrent access. However, node allocation and destruction is left
> + * for the caller. While calling into the vma-manager, a given node must
> + * always be guaranteed to be referenced.
> + */
> +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
> +				 unsigned long page_offset, unsigned long size)
> +{
> +	rwlock_init(&mgr->vm_lock);
> +	mgr->vm_addr_space_rb = RB_ROOT;
> +	drm_mm_init(&mgr->vm_addr_space_mm, page_offset, size);
> +}
> +EXPORT_SYMBOL(drm_vma_offset_manager_init);
> +
> +/** drm_vma_offset_manager_destroy()
> + *
> + * Destroy an object manager which was previously created via
> + * drm_vma_offset_manager_init(). The caller must remove all allocated nodes
> + * before destroying the manager. Otherwise, drm_mm will refuse to free the
> + * requested resources.
> + *
> + * The manager must not be accessed after this function is called.
> + */
> +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr)
> +{
> +	BUG_ON(!drm_mm_clean(&mgr->vm_addr_space_mm));

Please convert this to a WARN_ON - drm_mm has code to cobble over buggy
drivers, and killing the driver with a BUG_ON if we could keep on going
with just a WARN_ON is a real pain for development.

> +
> +	/* take the lock to protect against buggy drivers */
> +	write_lock(&mgr->vm_lock);
> +	drm_mm_takedown(&mgr->vm_addr_space_mm);
> +	write_unlock(&mgr->vm_lock);
> +}
> +EXPORT_SYMBOL(drm_vma_offset_manager_destroy);
> +
> +/** drm_vma_offset_lookup()
> + *
> + * Find a node given a start address and object size. This returns the _best_
> + * match for the given node. That is, @start may point somewhere into a valid
> + * region and the given node will be returned, as long as the node spans the
> + * whole requested area (given the size in number of pages as @pages).
> + *
> + * Returns NULL if no suitable node can be found. Otherwise, the best match
> + * is returned. It's the caller's responsibility to make sure the node doesn't
> + * get destroyed before the caller can access it.
> + */
> +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
> +						  unsigned long start,
> +						  unsigned long pages)
> +{
> +	struct drm_vma_offset_node *node, *best;
> +	struct rb_node *iter;
> +	unsigned long offset;
> +
> +	read_lock(&mgr->vm_lock);
> +
> +	iter = mgr->vm_addr_space_rb.rb_node;
> +	best = NULL;
> +
> +	while (likely(iter)) {
> +		node = rb_entry(iter, struct drm_vma_offset_node, vm_rb);
> +		offset = node->vm_node.start;
> +		if (start >= offset) {
> +			iter = iter->rb_right;
> +			best = node;
> +			if (start == offset)
> +				break;
> +		} else {
> +			iter = iter->rb_left;
> +		}
> +	}
> +
> +	/* verify that the node spans the requested area */
> +	if (likely(best)) {
> +		offset = best->vm_node.start + best->vm_pages;
> +		if (offset > start + pages)
> +			best = NULL;
> +	}
> +
> +	read_unlock(&mgr->vm_lock);
> +
> +	return best;
> +}
> +EXPORT_SYMBOL(drm_vma_offset_lookup);
> +
> +/* internal helper to link @node into the rb-tree */
> +static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr,
> +				   struct drm_vma_offset_node *node)
> +{
> +	struct rb_node **iter = &mgr->vm_addr_space_rb.rb_node;
> +	struct rb_node *parent = NULL;
> +	struct drm_vma_offset_node *iter_node;
> +
> +	while (likely(*iter)) {
> +		parent = *iter;
> +		iter_node = rb_entry(*iter, struct drm_vma_offset_node, vm_rb);
> +
> +		if (node->vm_node.start < iter_node->vm_node.start)
> +			iter = &(*iter)->rb_left;
> +		else if (node->vm_node.start > iter_node->vm_node.start)
> +			iter = &(*iter)->rb_right;
> +		else
> +			BUG();
> +	}
> +
> +	rb_link_node(&node->vm_rb, parent, iter);
> +	rb_insert_color(&node->vm_rb, &mgr->vm_addr_space_rb);
> +}
> +
> +/** drm_vma_offset_add()
> + *
> + * Add a node to the offset-manager. If the node was already added, this does
> + * nothing and return 0. @pages is the size of the object given in number of
> + * pages.
> + * After this call succeeds, you can access the offset of the node until it
> + * is removed again.
> + *
> + * If this call fails, it is safe to retry the operation or call
> + * drm_vma_offset_remove(), anyway. However, no cleanup is required in that
> + * case.
> + */
> +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
> +		       struct drm_vma_offset_node *node, unsigned long pages)
> +{
> +	int ret;
> +
> +	write_lock(&mgr->vm_lock);
> +
> +	if (unlikely(drm_mm_node_linked(&node->vm_node))) {
> +		ret = 0;
> +		goto out_unlock;
> +	}
> +
> +	ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
> +					 &node->vm_node, pages, 0, 0);

This allocates memory, so potentially blocks. This doesn't mesh well with
the spinning rwlock (lockdep should seriously complain about this btw).
Please use an rwsemaphore instead.

> +	if (unlikely(ret))
> +		goto out_reset;
> +
> +	node->vm_pages = pages;
> +	_drm_vma_offset_add_rb(mgr, node);
> +	goto out_unlock;
> +
> +out_reset:
> +	/* we don't know what happened to @node->vm_node, so clear it */
> +	drm_vma_node_reset(node);

If you switch over to drm_mm_node_allocated then this reset could
shouldn't be required - it'd be a pretty serious bug in drm_mm.c

> +out_unlock:
> +	write_unlock(&mgr->vm_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_vma_offset_add);
> +
> +/** drm_vma_offset_remove()
> + *
> + * Remove a node from the offset manager. If the node wasn't added before, this
> + * does nothing. After this call returns, the offset of the node must be not
> + * accessed, anymore.
> + * It is safe to add the node via drm_vma_offset_add() again.
> + */
> +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
> +			   struct drm_vma_offset_node *node)
> +{
> +	if (unlikely(!drm_mm_node_linked(&node->vm_node)))
> +		return;
> +
> +	write_lock(&mgr->vm_lock);
> +
> +	rb_erase(&node->vm_rb, &mgr->vm_addr_space_rb);
> +	drm_mm_remove_node(&node->vm_node);
> +	drm_vma_node_reset(node);
> +
> +	write_unlock(&mgr->vm_lock);
> +}
> +EXPORT_SYMBOL(drm_vma_offset_remove);
> diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
> new file mode 100644
> index 0000000..8a1975c
> --- /dev/null
> +++ b/include/drm/drm_vma_manager.h
> @@ -0,0 +1,107 @@
> +#ifndef __DRM_VMA_MANAGER_H__
> +#define __DRM_VMA_MANAGER_H__
> +
> +/*
> + * Copyright (c) 2013 David Herrmann <[e-mail]>
> + *
> + * 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 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 <drm/drmP.h>
> +#include <drm/drm_mm.h>
> +#include <linux/module.h>
> +#include <linux/rbtree.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +struct drm_vma_offset_node {
> +	struct drm_mm_node vm_node;
> +	struct rb_node vm_rb;
> +	unsigned long vm_pages;
> +};
> +
> +struct drm_vma_offset_manager {
> +	rwlock_t vm_lock;
> +	struct rb_root vm_addr_space_rb;
> +	struct drm_mm vm_addr_space_mm;
> +};
> +
> +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
> +				 unsigned long page_offset, unsigned long size);
> +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr);
> +
> +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
> +						  unsigned long start,
> +						  unsigned long pages);
> +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
> +		       struct drm_vma_offset_node *node, unsigned long pages);
> +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
> +			   struct drm_vma_offset_node *node);
> +
> +/** drm_vma_offset_exact_lookup()
> + *
> + * Same as drm_vma_offset_lookup() but does not allow any offset but only
> + * returns the exact object with the given start address.
> + */
> +static inline struct drm_vma_offset_node *
> +drm_vma_offset_exact_lookup(struct drm_vma_offset_manager *mgr,
> +			    unsigned long start)
> +{
> +	return drm_vma_offset_lookup(mgr, start, 1);
> +}
> +
> +/** drm_vma_node_reset()
> + *
> + * Reset a node to its initial state.
> + */
> +static inline void drm_vma_node_reset(struct drm_vma_offset_node *node)
> +{
> +	memset(&node->vm_node, 0, sizeof(node->vm_node));
> +}
> +
> +/** drm_vma_node_start()
> + *
> + * Return the start address of the given node as pfn.
> + */
> +static inline unsigned long drm_vma_node_start(struct drm_vma_offset_node *node)
> +{
> +	return node->vm_node.start;
> +}
> +
> +/** drm_vma_node_has_offset()
> + *
> + * Return true iff the node was previously allocated an offset and added to
> + * an vma offset manager.
> + */
> +static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node)
> +{
> +	return drm_mm_node_linked(&node->vm_node);
> +}
> +
> +/** drm_vma_node_offset_addr()
> + *
> + * Same as drm_vma_node_start() but returns the address as a valid userspace
> + * address (instead of a pfn).
> + */

Userspace address and pfn are a bit misleading imo in this context. I'd go
with "byte offset for consumption by userspace/mmap" and "page-based
addressing" or something similar.

> +static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
> +{
> +	return ((__u64)node->vm_node.start) << PAGE_SHIFT;
> +}
> +
> +#endif /* __DRM_VMA_MANAGER_H__ */
> -- 
> 1.8.3.2
> 
> _______________________________________________
> dri-devel mailing list
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

David Herrmann July 1, 2013, 7:54 p.m. UTC | #2

Hi

On Mon, Jul 1, 2013 at 9:42 PM, Daniel Vetter <[e-mail]> wrote:
> On Mon, Jul 01, 2013 at 08:33:00PM +0200, David Herrmann wrote:
>> If we want to map GPU memory into user-space, we need to linearize the
>> addresses to not confuse mm-core. Currently, GEM and TTM both implement
>> their own offset-managers to assign a pgoff to each object for user-space
>> CPU access. GEM uses a hash-table, TTM uses an rbtree.
>>
>> This patch provides a unified implementation that can be used to replace
>> both. TTM allows partial mmaps with a given offset, so we cannot use
>> hashtables as the start address may not be known at mmap time. Hence, we
>> use the rbtree-implementation of TTM.
>>
>> We could easily update drm_mm to use an rbtree instead of a linked list
>> for it's object list and thus drop the rbtree from the vma-manager.
>> However, this would slow down drm_mm object allocation for all other
>> use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
>> Hence, use the separate tree but allow for later migration.
>>
>> This is a rewrite of the 2012-proposal by David Airlie <[e-mail]>
>>
>> Signed-off-by: David Herrmann <[e-mail]>
>
> This seems to sprinkle a lot of likely/unlikely. Imo those are only
> justified in two cases:
> - When we have real performance data proving that they're worth it.
> - Sometimes they're also useful as self-documenting code for the truly
>   unlikely. But early do-nothing returns and error handling are
>   established code patterns, so imo don't qualify.
>
> I think none of the likely/unlikely below qualify, so imo better to remove
> them.

They are copied mostly from ttm_bo.c. However, I agree that they are
probably unneeded. Furthermore, these aren't code-paths I'd expect to
be performance-critical. I will dig into the git-history of TTM to see
whether they have been in-place since the beginning or whether there
is some real reason for them.

> Second high-level review request: Can you please convert the very nice
> documentation you already added into proper kerneldoc and link it up at an
> appropriate section in the drm DocBook?

Sure.

> A few more comments below. I'll postpone reviewing the gem/ttm conversion
> patches until this is settled a bit.
>
>> ---
>>  drivers/gpu/drm/Makefile          |   2 +-
>>  drivers/gpu/drm/drm_vma_manager.c | 224 ++++++++++++++++++++++++++++++++++++++
>>  include/drm/drm_vma_manager.h     | 107 ++++++++++++++++++
>>  3 files changed, 332 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/drm_vma_manager.c
>>  create mode 100644 include/drm/drm_vma_manager.h
>>
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 1c9f2439..f8851cc 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -12,7 +12,7 @@ drm-y       :=      drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
>>               drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
>>               drm_crtc.o drm_modes.o drm_edid.o \
>>               drm_info.o drm_debugfs.o drm_encoder_slave.o \
>> -             drm_trace_points.o drm_global.o drm_prime.o
>> +             drm_trace_points.o drm_global.o drm_prime.o drm_vma_manager.o
>>
>>  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>>  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
>> diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
>> new file mode 100644
>> index 0000000..c28639f
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_vma_manager.c
>> @@ -0,0 +1,224 @@
>> +/*
>> + * Copyright (c) 2012 David Airlie <[e-mail]>
>> + * Copyright (c) 2013 David Herrmann <[e-mail]>
>> + *
>> + * 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 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 <drm/drmP.h>
>> +#include <drm/drm_mm.h>
>> +#include <drm/drm_vma_manager.h>
>> +#include <linux/mm.h>
>> +#include <linux/module.h>
>> +#include <linux/rbtree.h>
>> +#include <linux/slab.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/types.h>
>> +
>> +/** @file drm_vma_manager.c
>> + *
>> + * The vma-manager is responsible to map arbitrary driver-dependent memory
>> + * regions into the linear user address-space. It provides offsets to the
>> + * caller which can then be used on the address_space of the drm-device. It
>> + * takes care to not overlap regions, size them appropriately and to not
>> + * confuse mm-core by inconsistent fake vm_pgoff fields.
>> + *
>> + * We use drm_mm as backend to manage object allocations. But it is highly
>> + * optimized for alloc/free calls, not lookups. Hence, we use an rb-tree to
>> + * speed up offset lookups.
>> + */
>> +
>> +/** drm_vma_offset_manager_init()
>> + *
>> + * Initialize a new offset-manager. The offset and area size available for the
>> + * manager are given as @page_offset and @size. Both are interpreted as
>> + * page-numbers, not bytes.
>> + *
>> + * Adding/removing nodes from the manager is locked internally and protected
>> + * against concurrent access. However, node allocation and destruction is left
>> + * for the caller. While calling into the vma-manager, a given node must
>> + * always be guaranteed to be referenced.
>> + */
>> +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
>> +                              unsigned long page_offset, unsigned long size)
>> +{
>> +     rwlock_init(&mgr->vm_lock);
>> +     mgr->vm_addr_space_rb = RB_ROOT;
>> +     drm_mm_init(&mgr->vm_addr_space_mm, page_offset, size);
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_manager_init);
>> +
>> +/** drm_vma_offset_manager_destroy()
>> + *
>> + * Destroy an object manager which was previously created via
>> + * drm_vma_offset_manager_init(). The caller must remove all allocated nodes
>> + * before destroying the manager. Otherwise, drm_mm will refuse to free the
>> + * requested resources.
>> + *
>> + * The manager must not be accessed after this function is called.
>> + */
>> +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr)
>> +{
>> +     BUG_ON(!drm_mm_clean(&mgr->vm_addr_space_mm));
>
> Please convert this to a WARN_ON - drm_mm has code to cobble over buggy
> drivers, and killing the driver with a BUG_ON if we could keep on going
> with just a WARN_ON is a real pain for development.

Ok.

>> +
>> +     /* take the lock to protect against buggy drivers */
>> +     write_lock(&mgr->vm_lock);
>> +     drm_mm_takedown(&mgr->vm_addr_space_mm);
>> +     write_unlock(&mgr->vm_lock);
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_manager_destroy);
>> +
>> +/** drm_vma_offset_lookup()
>> + *
>> + * Find a node given a start address and object size. This returns the _best_
>> + * match for the given node. That is, @start may point somewhere into a valid
>> + * region and the given node will be returned, as long as the node spans the
>> + * whole requested area (given the size in number of pages as @pages).
>> + *
>> + * Returns NULL if no suitable node can be found. Otherwise, the best match
>> + * is returned. It's the caller's responsibility to make sure the node doesn't
>> + * get destroyed before the caller can access it.
>> + */
>> +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
>> +                                               unsigned long start,
>> +                                               unsigned long pages)
>> +{
>> +     struct drm_vma_offset_node *node, *best;
>> +     struct rb_node *iter;
>> +     unsigned long offset;
>> +
>> +     read_lock(&mgr->vm_lock);
>> +
>> +     iter = mgr->vm_addr_space_rb.rb_node;
>> +     best = NULL;
>> +
>> +     while (likely(iter)) {
>> +             node = rb_entry(iter, struct drm_vma_offset_node, vm_rb);
>> +             offset = node->vm_node.start;
>> +             if (start >= offset) {
>> +                     iter = iter->rb_right;
>> +                     best = node;
>> +                     if (start == offset)
>> +                             break;
>> +             } else {
>> +                     iter = iter->rb_left;
>> +             }
>> +     }
>> +
>> +     /* verify that the node spans the requested area */
>> +     if (likely(best)) {
>> +             offset = best->vm_node.start + best->vm_pages;
>> +             if (offset > start + pages)
>> +                     best = NULL;
>> +     }
>> +
>> +     read_unlock(&mgr->vm_lock);
>> +
>> +     return best;
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_lookup);
>> +
>> +/* internal helper to link @node into the rb-tree */
>> +static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr,
>> +                                struct drm_vma_offset_node *node)
>> +{
>> +     struct rb_node **iter = &mgr->vm_addr_space_rb.rb_node;
>> +     struct rb_node *parent = NULL;
>> +     struct drm_vma_offset_node *iter_node;
>> +
>> +     while (likely(*iter)) {
>> +             parent = *iter;
>> +             iter_node = rb_entry(*iter, struct drm_vma_offset_node, vm_rb);
>> +
>> +             if (node->vm_node.start < iter_node->vm_node.start)
>> +                     iter = &(*iter)->rb_left;
>> +             else if (node->vm_node.start > iter_node->vm_node.start)
>> +                     iter = &(*iter)->rb_right;
>> +             else
>> +                     BUG();
>> +     }
>> +
>> +     rb_link_node(&node->vm_rb, parent, iter);
>> +     rb_insert_color(&node->vm_rb, &mgr->vm_addr_space_rb);
>> +}
>> +
>> +/** drm_vma_offset_add()
>> + *
>> + * Add a node to the offset-manager. If the node was already added, this does
>> + * nothing and return 0. @pages is the size of the object given in number of
>> + * pages.
>> + * After this call succeeds, you can access the offset of the node until it
>> + * is removed again.
>> + *
>> + * If this call fails, it is safe to retry the operation or call
>> + * drm_vma_offset_remove(), anyway. However, no cleanup is required in that
>> + * case.
>> + */
>> +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
>> +                    struct drm_vma_offset_node *node, unsigned long pages)
>> +{
>> +     int ret;
>> +
>> +     write_lock(&mgr->vm_lock);
>> +
>> +     if (unlikely(drm_mm_node_linked(&node->vm_node))) {
>> +             ret = 0;
>> +             goto out_unlock;
>> +     }
>> +
>> +     ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
>> +                                      &node->vm_node, pages, 0, 0);
>
> This allocates memory, so potentially blocks. This doesn't mesh well with
> the spinning rwlock (lockdep should seriously complain about this btw).
> Please use an rwsemaphore instead.

Ugh? This shouldn't allocate any memory. I use pre-allocated nodes and
looking at drm_mm.c this just links the node at the correct position.
As long as the color_adjust() callbacks are atomic, this should be
fine here. Did you mix this up with get_block()?

>> +     if (unlikely(ret))
>> +             goto out_reset;
>> +
>> +     node->vm_pages = pages;
>> +     _drm_vma_offset_add_rb(mgr, node);
>> +     goto out_unlock;
>> +
>> +out_reset:
>> +     /* we don't know what happened to @node->vm_node, so clear it */
>> +     drm_vma_node_reset(node);
>
> If you switch over to drm_mm_node_allocated then this reset could
> shouldn't be required - it'd be a pretty serious bug in drm_mm.c

I thought this flag was used for kfree() tracking. Turns out, it's
not. I will fix it up and remove the unnecessary resets.

>> +out_unlock:
>> +     write_unlock(&mgr->vm_lock);
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_add);
>> +
>> +/** drm_vma_offset_remove()
>> + *
>> + * Remove a node from the offset manager. If the node wasn't added before, this
>> + * does nothing. After this call returns, the offset of the node must be not
>> + * accessed, anymore.
>> + * It is safe to add the node via drm_vma_offset_add() again.
>> + */
>> +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
>> +                        struct drm_vma_offset_node *node)
>> +{
>> +     if (unlikely(!drm_mm_node_linked(&node->vm_node)))
>> +             return;
>> +
>> +     write_lock(&mgr->vm_lock);
>> +
>> +     rb_erase(&node->vm_rb, &mgr->vm_addr_space_rb);
>> +     drm_mm_remove_node(&node->vm_node);
>> +     drm_vma_node_reset(node);
>> +
>> +     write_unlock(&mgr->vm_lock);
>> +}
>> +EXPORT_SYMBOL(drm_vma_offset_remove);
>> diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
>> new file mode 100644
>> index 0000000..8a1975c
>> --- /dev/null
>> +++ b/include/drm/drm_vma_manager.h
>> @@ -0,0 +1,107 @@
>> +#ifndef __DRM_VMA_MANAGER_H__
>> +#define __DRM_VMA_MANAGER_H__
>> +
>> +/*
>> + * Copyright (c) 2013 David Herrmann <[e-mail]>
>> + *
>> + * 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 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 <drm/drmP.h>
>> +#include <drm/drm_mm.h>
>> +#include <linux/module.h>
>> +#include <linux/rbtree.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/types.h>
>> +
>> +struct drm_vma_offset_node {
>> +     struct drm_mm_node vm_node;
>> +     struct rb_node vm_rb;
>> +     unsigned long vm_pages;
>> +};
>> +
>> +struct drm_vma_offset_manager {
>> +     rwlock_t vm_lock;
>> +     struct rb_root vm_addr_space_rb;
>> +     struct drm_mm vm_addr_space_mm;
>> +};
>> +
>> +void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
>> +                              unsigned long page_offset, unsigned long size);
>> +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr);
>> +
>> +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
>> +                                               unsigned long start,
>> +                                               unsigned long pages);
>> +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
>> +                    struct drm_vma_offset_node *node, unsigned long pages);
>> +void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
>> +                        struct drm_vma_offset_node *node);
>> +
>> +/** drm_vma_offset_exact_lookup()
>> + *
>> + * Same as drm_vma_offset_lookup() but does not allow any offset but only
>> + * returns the exact object with the given start address.
>> + */
>> +static inline struct drm_vma_offset_node *
>> +drm_vma_offset_exact_lookup(struct drm_vma_offset_manager *mgr,
>> +                         unsigned long start)
>> +{
>> +     return drm_vma_offset_lookup(mgr, start, 1);
>> +}
>> +
>> +/** drm_vma_node_reset()
>> + *
>> + * Reset a node to its initial state.
>> + */
>> +static inline void drm_vma_node_reset(struct drm_vma_offset_node *node)
>> +{
>> +     memset(&node->vm_node, 0, sizeof(node->vm_node));
>> +}
>> +
>> +/** drm_vma_node_start()
>> + *
>> + * Return the start address of the given node as pfn.
>> + */
>> +static inline unsigned long drm_vma_node_start(struct drm_vma_offset_node *node)
>> +{
>> +     return node->vm_node.start;
>> +}
>> +
>> +/** drm_vma_node_has_offset()
>> + *
>> + * Return true iff the node was previously allocated an offset and added to
>> + * an vma offset manager.
>> + */
>> +static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node)
>> +{
>> +     return drm_mm_node_linked(&node->vm_node);
>> +}
>> +
>> +/** drm_vma_node_offset_addr()
>> + *
>> + * Same as drm_vma_node_start() but returns the address as a valid userspace
>> + * address (instead of a pfn).
>> + */
>
> Userspace address and pfn are a bit misleading imo in this context. I'd go
> with "byte offset for consumption by userspace/mmap" and "page-based
> addressing" or something similar.

Ok.

Thanks!

>> +static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
>> +{
>> +     return ((__u64)node->vm_node.start) << PAGE_SHIFT;
>> +}
>> +
>> +#endif /* __DRM_VMA_MANAGER_H__ */
>> --
>> 1.8.3.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> [e-mail]
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> [phone] - http://blog.ffwll.ch

Daniel Vetter July 1, 2013, 8:17 p.m. UTC | #3

On Mon, Jul 01, 2013 at 09:54:17PM +0200, David Herrmann wrote:
> On Mon, Jul 1, 2013 at 9:42 PM, Daniel Vetter <[e-mail]> wrote:
> > On Mon, Jul 01, 2013 at 08:33:00PM +0200, David Herrmann wrote:
> >> +/** drm_vma_offset_manager_destroy()
> >> + *
> >> + * Destroy an object manager which was previously created via
> >> + * drm_vma_offset_manager_init(). The caller must remove all allocated nodes
> >> + * before destroying the manager. Otherwise, drm_mm will refuse to free the
> >> + * requested resources.
> >> + *
> >> + * The manager must not be accessed after this function is called.
> >> + */
> >> +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr)
> >> +{
> >> +     BUG_ON(!drm_mm_clean(&mgr->vm_addr_space_mm));
> >
> > Please convert this to a WARN_ON - drm_mm has code to cobble over buggy
> > drivers, and killing the driver with a BUG_ON if we could keep on going
> > with just a WARN_ON is a real pain for development.
> 
> Ok.

Actually I think the check in the takedown function should be good enough
already. I've just dumped a patch onto dri-devel which will convert that
to a WARN, so I think adding a second one here is redundant.

> 
> >> +
> >> +     /* take the lock to protect against buggy drivers */
> >> +     write_lock(&mgr->vm_lock);
> >> +     drm_mm_takedown(&mgr->vm_addr_space_mm);
> >> +     write_unlock(&mgr->vm_lock);
> >> +}
> >> +EXPORT_SYMBOL(drm_vma_offset_manager_destroy);
> >> +
> >> +/** drm_vma_offset_lookup()
> >> + *
> >> + * Find a node given a start address and object size. This returns the _best_
> >> + * match for the given node. That is, @start may point somewhere into a valid
> >> + * region and the given node will be returned, as long as the node spans the
> >> + * whole requested area (given the size in number of pages as @pages).
> >> + *
> >> + * Returns NULL if no suitable node can be found. Otherwise, the best match
> >> + * is returned. It's the caller's responsibility to make sure the node doesn't
> >> + * get destroyed before the caller can access it.
> >> + */
> >> +struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
> >> +                                               unsigned long start,
> >> +                                               unsigned long pages)
> >> +{
> >> +     struct drm_vma_offset_node *node, *best;
> >> +     struct rb_node *iter;
> >> +     unsigned long offset;
> >> +
> >> +     read_lock(&mgr->vm_lock);
> >> +
> >> +     iter = mgr->vm_addr_space_rb.rb_node;
> >> +     best = NULL;
> >> +
> >> +     while (likely(iter)) {
> >> +             node = rb_entry(iter, struct drm_vma_offset_node, vm_rb);
> >> +             offset = node->vm_node.start;
> >> +             if (start >= offset) {
> >> +                     iter = iter->rb_right;
> >> +                     best = node;
> >> +                     if (start == offset)
> >> +                             break;
> >> +             } else {
> >> +                     iter = iter->rb_left;
> >> +             }
> >> +     }
> >> +
> >> +     /* verify that the node spans the requested area */
> >> +     if (likely(best)) {
> >> +             offset = best->vm_node.start + best->vm_pages;
> >> +             if (offset > start + pages)
> >> +                     best = NULL;
> >> +     }
> >> +
> >> +     read_unlock(&mgr->vm_lock);
> >> +
> >> +     return best;
> >> +}
> >> +EXPORT_SYMBOL(drm_vma_offset_lookup);
> >> +
> >> +/* internal helper to link @node into the rb-tree */
> >> +static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr,
> >> +                                struct drm_vma_offset_node *node)
> >> +{
> >> +     struct rb_node **iter = &mgr->vm_addr_space_rb.rb_node;
> >> +     struct rb_node *parent = NULL;
> >> +     struct drm_vma_offset_node *iter_node;
> >> +
> >> +     while (likely(*iter)) {
> >> +             parent = *iter;
> >> +             iter_node = rb_entry(*iter, struct drm_vma_offset_node, vm_rb);
> >> +
> >> +             if (node->vm_node.start < iter_node->vm_node.start)
> >> +                     iter = &(*iter)->rb_left;
> >> +             else if (node->vm_node.start > iter_node->vm_node.start)
> >> +                     iter = &(*iter)->rb_right;
> >> +             else
> >> +                     BUG();
> >> +     }
> >> +
> >> +     rb_link_node(&node->vm_rb, parent, iter);
> >> +     rb_insert_color(&node->vm_rb, &mgr->vm_addr_space_rb);
> >> +}
> >> +
> >> +/** drm_vma_offset_add()
> >> + *
> >> + * Add a node to the offset-manager. If the node was already added, this does
> >> + * nothing and return 0. @pages is the size of the object given in number of
> >> + * pages.
> >> + * After this call succeeds, you can access the offset of the node until it
> >> + * is removed again.
> >> + *
> >> + * If this call fails, it is safe to retry the operation or call
> >> + * drm_vma_offset_remove(), anyway. However, no cleanup is required in that
> >> + * case.
> >> + */
> >> +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
> >> +                    struct drm_vma_offset_node *node, unsigned long pages)
> >> +{
> >> +     int ret;
> >> +
> >> +     write_lock(&mgr->vm_lock);
> >> +
> >> +     if (unlikely(drm_mm_node_linked(&node->vm_node))) {
> >> +             ret = 0;
> >> +             goto out_unlock;
> >> +     }
> >> +
> >> +     ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
> >> +                                      &node->vm_node, pages, 0, 0);
> >
> > This allocates memory, so potentially blocks. This doesn't mesh well with
> > the spinning rwlock (lockdep should seriously complain about this btw).
> > Please use an rwsemaphore instead.
> 
> Ugh? This shouldn't allocate any memory. I use pre-allocated nodes and
> looking at drm_mm.c this just links the node at the correct position.
> As long as the color_adjust() callbacks are atomic, this should be
> fine here. Did you mix this up with get_block()?

Oops, indeed I've mixed stuff up ;-)

I'd still vote for an rwsem though since these codepaths shouldn't be
performance critical and in pathalogical corner cases the O(n) list
walking drm_mm does could result in quite awful long spinlock holding
times.
-Daniel

Patch

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1c9f2439..f8851cc 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -12,7 +12,7 @@  drm-y       :=	drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
 		drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
 		drm_crtc.o drm_modes.o drm_edid.o \
 		drm_info.o drm_debugfs.o drm_encoder_slave.o \
-		drm_trace_points.o drm_global.o drm_prime.o
+		drm_trace_points.o drm_global.o drm_prime.o drm_vma_manager.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
new file mode 100644
index 0000000..c28639f
--- /dev/null
+++ b/drivers/gpu/drm/drm_vma_manager.c
@@ -0,0 +1,224 @@ 
+/*
+ * Copyright (c) 2012 David Airlie <[e-mail]>
+ * Copyright (c) 2013 David Herrmann <[e-mail]>
+ *
+ * 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 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 <drm/drmP.h>
+#include <drm/drm_mm.h>
+#include <drm/drm_vma_manager.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/rbtree.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+/** @file drm_vma_manager.c
+ *
+ * The vma-manager is responsible to map arbitrary driver-dependent memory
+ * regions into the linear user address-space. It provides offsets to the
+ * caller which can then be used on the address_space of the drm-device. It
+ * takes care to not overlap regions, size them appropriately and to not
+ * confuse mm-core by inconsistent fake vm_pgoff fields.
+ *
+ * We use drm_mm as backend to manage object allocations. But it is highly
+ * optimized for alloc/free calls, not lookups. Hence, we use an rb-tree to
+ * speed up offset lookups.
+ */
+
+/** drm_vma_offset_manager_init()
+ *
+ * Initialize a new offset-manager. The offset and area size available for the
+ * manager are given as @page_offset and @size. Both are interpreted as
+ * page-numbers, not bytes.
+ *
+ * Adding/removing nodes from the manager is locked internally and protected
+ * against concurrent access. However, node allocation and destruction is left
+ * for the caller. While calling into the vma-manager, a given node must
+ * always be guaranteed to be referenced.
+ */
+void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
+				 unsigned long page_offset, unsigned long size)
+{
+	rwlock_init(&mgr->vm_lock);
+	mgr->vm_addr_space_rb = RB_ROOT;
+	drm_mm_init(&mgr->vm_addr_space_mm, page_offset, size);
+}
+EXPORT_SYMBOL(drm_vma_offset_manager_init);
+
+/** drm_vma_offset_manager_destroy()
+ *
+ * Destroy an object manager which was previously created via
+ * drm_vma_offset_manager_init(). The caller must remove all allocated nodes
+ * before destroying the manager. Otherwise, drm_mm will refuse to free the
+ * requested resources.
+ *
+ * The manager must not be accessed after this function is called.
+ */
+void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr)
+{
+	BUG_ON(!drm_mm_clean(&mgr->vm_addr_space_mm));
+
+	/* take the lock to protect against buggy drivers */
+	write_lock(&mgr->vm_lock);
+	drm_mm_takedown(&mgr->vm_addr_space_mm);
+	write_unlock(&mgr->vm_lock);
+}
+EXPORT_SYMBOL(drm_vma_offset_manager_destroy);
+
+/** drm_vma_offset_lookup()
+ *
+ * Find a node given a start address and object size. This returns the _best_
+ * match for the given node. That is, @start may point somewhere into a valid
+ * region and the given node will be returned, as long as the node spans the
+ * whole requested area (given the size in number of pages as @pages).
+ *
+ * Returns NULL if no suitable node can be found. Otherwise, the best match
+ * is returned. It's the caller's responsibility to make sure the node doesn't
+ * get destroyed before the caller can access it.
+ */
+struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
+						  unsigned long start,
+						  unsigned long pages)
+{
+	struct drm_vma_offset_node *node, *best;
+	struct rb_node *iter;
+	unsigned long offset;
+
+	read_lock(&mgr->vm_lock);
+
+	iter = mgr->vm_addr_space_rb.rb_node;
+	best = NULL;
+
+	while (likely(iter)) {
+		node = rb_entry(iter, struct drm_vma_offset_node, vm_rb);
+		offset = node->vm_node.start;
+		if (start >= offset) {
+			iter = iter->rb_right;
+			best = node;
+			if (start == offset)
+				break;
+		} else {
+			iter = iter->rb_left;
+		}
+	}
+
+	/* verify that the node spans the requested area */
+	if (likely(best)) {
+		offset = best->vm_node.start + best->vm_pages;
+		if (offset > start + pages)
+			best = NULL;
+	}
+
+	read_unlock(&mgr->vm_lock);
+
+	return best;
+}
+EXPORT_SYMBOL(drm_vma_offset_lookup);
+
+/* internal helper to link @node into the rb-tree */
+static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr,
+				   struct drm_vma_offset_node *node)
+{
+	struct rb_node **iter = &mgr->vm_addr_space_rb.rb_node;
+	struct rb_node *parent = NULL;
+	struct drm_vma_offset_node *iter_node;
+
+	while (likely(*iter)) {
+		parent = *iter;
+		iter_node = rb_entry(*iter, struct drm_vma_offset_node, vm_rb);
+
+		if (node->vm_node.start < iter_node->vm_node.start)
+			iter = &(*iter)->rb_left;
+		else if (node->vm_node.start > iter_node->vm_node.start)
+			iter = &(*iter)->rb_right;
+		else
+			BUG();
+	}
+
+	rb_link_node(&node->vm_rb, parent, iter);
+	rb_insert_color(&node->vm_rb, &mgr->vm_addr_space_rb);
+}
+
+/** drm_vma_offset_add()
+ *
+ * Add a node to the offset-manager. If the node was already added, this does
+ * nothing and return 0. @pages is the size of the object given in number of
+ * pages.
+ * After this call succeeds, you can access the offset of the node until it
+ * is removed again.
+ *
+ * If this call fails, it is safe to retry the operation or call
+ * drm_vma_offset_remove(), anyway. However, no cleanup is required in that
+ * case.
+ */
+int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
+		       struct drm_vma_offset_node *node, unsigned long pages)
+{
+	int ret;
+
+	write_lock(&mgr->vm_lock);
+
+	if (unlikely(drm_mm_node_linked(&node->vm_node))) {
+		ret = 0;
+		goto out_unlock;
+	}
+
+	ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
+					 &node->vm_node, pages, 0, 0);
+	if (unlikely(ret))
+		goto out_reset;
+
+	node->vm_pages = pages;
+	_drm_vma_offset_add_rb(mgr, node);
+	goto out_unlock;
+
+out_reset:
+	/* we don't know what happened to @node->vm_node, so clear it */
+	drm_vma_node_reset(node);
+out_unlock:
+	write_unlock(&mgr->vm_lock);
+	return ret;
+}
+EXPORT_SYMBOL(drm_vma_offset_add);
+
+/** drm_vma_offset_remove()
+ *
+ * Remove a node from the offset manager. If the node wasn't added before, this
+ * does nothing. After this call returns, the offset of the node must be not
+ * accessed, anymore.
+ * It is safe to add the node via drm_vma_offset_add() again.
+ */
+void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
+			   struct drm_vma_offset_node *node)
+{
+	if (unlikely(!drm_mm_node_linked(&node->vm_node)))
+		return;
+
+	write_lock(&mgr->vm_lock);
+
+	rb_erase(&node->vm_rb, &mgr->vm_addr_space_rb);
+	drm_mm_remove_node(&node->vm_node);
+	drm_vma_node_reset(node);
+
+	write_unlock(&mgr->vm_lock);
+}
+EXPORT_SYMBOL(drm_vma_offset_remove);
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
new file mode 100644
index 0000000..8a1975c
--- /dev/null
+++ b/include/drm/drm_vma_manager.h
@@ -0,0 +1,107 @@ 
+#ifndef __DRM_VMA_MANAGER_H__
+#define __DRM_VMA_MANAGER_H__
+
+/*
+ * Copyright (c) 2013 David Herrmann <[e-mail]>
+ *
+ * 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 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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 <drm/drmP.h>
+#include <drm/drm_mm.h>
+#include <linux/module.h>
+#include <linux/rbtree.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+struct drm_vma_offset_node {
+	struct drm_mm_node vm_node;
+	struct rb_node vm_rb;
+	unsigned long vm_pages;
+};
+
+struct drm_vma_offset_manager {
+	rwlock_t vm_lock;
+	struct rb_root vm_addr_space_rb;
+	struct drm_mm vm_addr_space_mm;
+};
+
+void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
+				 unsigned long page_offset, unsigned long size);
+void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr);
+
+struct drm_vma_offset_node *drm_vma_offset_lookup(struct drm_vma_offset_manager *mgr,
+						  unsigned long start,
+						  unsigned long pages);
+int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
+		       struct drm_vma_offset_node *node, unsigned long pages);
+void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
+			   struct drm_vma_offset_node *node);
+
+/** drm_vma_offset_exact_lookup()
+ *
+ * Same as drm_vma_offset_lookup() but does not allow any offset but only
+ * returns the exact object with the given start address.
+ */
+static inline struct drm_vma_offset_node *
+drm_vma_offset_exact_lookup(struct drm_vma_offset_manager *mgr,
+			    unsigned long start)
+{
+	return drm_vma_offset_lookup(mgr, start, 1);
+}
+
+/** drm_vma_node_reset()
+ *
+ * Reset a node to its initial state.
+ */
+static inline void drm_vma_node_reset(struct drm_vma_offset_node *node)
+{
+	memset(&node->vm_node, 0, sizeof(node->vm_node));
+}
+
+/** drm_vma_node_start()
+ *
+ * Return the start address of the given node as pfn.
+ */
+static inline unsigned long drm_vma_node_start(struct drm_vma_offset_node *node)
+{
+	return node->vm_node.start;
+}
+
+/** drm_vma_node_has_offset()
+ *
+ * Return true iff the node was previously allocated an offset and added to
+ * an vma offset manager.
+ */
+static inline bool drm_vma_node_has_offset(struct drm_vma_offset_node *node)
+{
+	return drm_mm_node_linked(&node->vm_node);
+}
+
+/** drm_vma_node_offset_addr()
+ *
+ * Same as drm_vma_node_start() but returns the address as a valid userspace
+ * address (instead of a pfn).
+ */
+static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
+{
+	return ((__u64)node->vm_node.start) << PAGE_SHIFT;
+}
+
+#endif /* __DRM_VMA_MANAGER_H__ */