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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/23/2016 1:28 PM, Chris Wilson wrote:
On Wed, Mar 23, 2016 at 11:39:44AM +0530, akash.goel@xxxxxxxxx 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.

fine, will rename the function to gem_shrink_migratepage, move it inside the gem_shrinker.c file, and use the existing constructs.

> Any reason for dropping the early
> if (!page_private(obj)) skip?
>

Would this sequence be fine ?

	if (!page_private(page))
		goto migrate; /*skip */

	Loop for locking mutex

	obj = (struct drm_i915_gem_object *)page_private(page);

	if (!PageSwapCache(page) && obj) {


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.

Tidying, like split that one single if condition into multiple if, else if blocks ?

Best regards
Akash
-Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux