Re: [PATCH 2/2] drm/i915: Make pages of GFX allocations movable

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, Mar 23, 2016 at 11:39:44AM +0530, akash.goel@intel.com wrote:
> +#ifdef CONFIG_MIGRATION
> +static int i915_migratepage(struct address_space *mapping,
> +			    struct page *newpage, struct page *page,
> +			    enum migrate_mode mode, void *dev_priv_data)

If we move this to i915_gem_shrink_migratepage (i.e. i915_gem_shrink),
we can

> +	/*
> +	 * Use trylock here, with a timeout, for struct_mutex as
> +	 * otherwise there is a possibility of deadlock due to lock
> +	 * inversion. This path, which tries to migrate a particular
> +	 * page after locking that page, can race with a path which
> +	 * truncate/purge pages of the corresponding object (after
> +	 * acquiring struct_mutex). Since page truncation will also
> +	 * try to lock the page, a scenario of deadlock can arise.
> +	 */
> +	while (!mutex_trylock(&dev->struct_mutex) && --timeout)
> +		schedule_timeout_killable(1);

replace this with i915_gem_shrinker_lock() and like constructs with the
other shrinkers. Any reason for dropping the early
if (!page_private(obj)) skip?

Similarly there are other patterns here that would benefit from
integration with existing shrinker logic. However, things like tidying
up the pin_display, unbinding, rpm lock inversion are still only on
list.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>