Re: [PATCH 1/2] drm/i915: Allow overlapping userptr objects

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 07/10/2014 10:21 AM, Chris Wilson wrote:
> Whilst I strongly advise against doing so for the implicit coherency
> issues between the multiple buffer objects accessing the same backing
> store, it nevertheless is a valid use case, akin to mmaping the same
> file multiple times.
>
> The reason why we forbade it earlier was that our use of the interval
> tree for fast invalidation upon vma changes excluded overlapping
> objects. So in the case where the user wishes to create such pairs of
> overlapping objects, we degrade the range invalidation to walkin the
> linear list of objects associated with the mm.
>
> v2: Compile for mmu-notifiers after tweaking

[snip]

> +static void invalidate_range__linear(struct i915_mmu_notifier *mn,
> +				     struct mm_struct *mm,
> +				     unsigned long start,
> +				     unsigned long end)
> +{
> +	struct i915_mmu_object *mmu;
> +	unsigned long serial;
> +
> +restart:
> +	serial = mn->serial;
> +	list_for_each_entry(mmu, &mn->linear, link) {
> +		struct drm_i915_gem_object *obj;
> +
> +		if (mmu->it.last < start || mmu->it.start > end)
> +			continue;
> +
> +		obj = mmu->obj;
> +		drm_gem_object_reference(&obj->base);
> +		spin_unlock(&mn->lock);
> +
> +		cancel_userptr(obj);
> +
> +		spin_lock(&mn->lock);
> +		if (serial != mn->serial)
> +			goto restart;
> +	}
> +
> +	spin_unlock(&mn->lock);
> +}
> +
>   static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
>   						       struct mm_struct *mm,
>   						       unsigned long start,
> @@ -60,16 +128,19 @@ static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
>   {
>   	struct i915_mmu_notifier *mn = container_of(_mn, struct i915_mmu_notifier, mn);
>   	struct interval_tree_node *it = NULL;
> +	unsigned long next = start;
>   	unsigned long serial = 0;
>
>   	end--; /* interval ranges are inclusive, but invalidate range is exclusive */
> -	while (start < end) {
> +	while (next < end) {
>   		struct drm_i915_gem_object *obj;
>
>   		obj = NULL;
>   		spin_lock(&mn->lock);
> +		if (mn->is_linear)
> +			return invalidate_range__linear(mn, mm, start, end);

Too bad that on first overlapping object the whole process goes into 
"slow mode". I wonder what would benchmarking say to that.

Perhaps we could still use interval tree but add another layer of 
indirection where ranges would be merged for overlapping objects and 
contain a linear list of them only there?

Tvrtko