diff mbox series

drm/i915: Force ww lock for i915_gem_object_ggtt_pin_ww, v2.

Message ID 20211130092055.679740-1-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Force ww lock for i915_gem_object_ggtt_pin_ww, v2. | expand

Commit Message

Maarten Lankhorst Nov. 30, 2021, 9:20 a.m. UTC
We will need the lock to unbind the vma, and wait for bind to complete.
Remove the special casing for the !ww path, and force ww locking for all.

Changes since v1:
- Pass err to for_i915_gem_ww handling for -EDEADLK handling.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  7 ++-----
 drivers/gpu/drm/i915/i915_gem.c | 30 ++++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 9 deletions(-)

Comments

Matthew Auld Dec. 1, 2021, 3:07 p.m. UTC | #1
On Tue, 30 Nov 2021 at 09:21, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
>
> We will need the lock to unbind the vma, and wait for bind to complete.
> Remove the special casing for the !ww path, and force ww locking for all.
>
> Changes since v1:
> - Pass err to for_i915_gem_ww handling for -EDEADLK handling.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  7 ++-----
>  drivers/gpu/drm/i915/i915_gem.c | 30 ++++++++++++++++++++++++++----
>  2 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1bfadd9127fc..8322abe194da 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1842,13 +1842,10 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
>                             const struct i915_ggtt_view *view,
>                             u64 size, u64 alignment, u64 flags);
>
> -static inline struct i915_vma * __must_check
> +struct i915_vma * __must_check
>  i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
>                          const struct i915_ggtt_view *view,
> -                        u64 size, u64 alignment, u64 flags)
> -{
> -       return i915_gem_object_ggtt_pin_ww(obj, NULL, view, size, alignment, flags);
> -}
> +                        u64 size, u64 alignment, u64 flags);
>
>  int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
>                            unsigned long flags);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 527228d4da7e..6002045bd194 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -877,6 +877,8 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
>         struct i915_vma *vma;
>         int ret;
>
> +       GEM_WARN_ON(!ww);

Return something here, or GEM_BUG_ON()? I assume it's going to crash
and burn anyway?

> +
>         if (flags & PIN_MAPPABLE &&
>             (!view || view->type == I915_GGTT_VIEW_NORMAL)) {
>                 /*
> @@ -936,10 +938,7 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
>                         return ERR_PTR(ret);
>         }
>
> -       if (ww)
> -               ret = i915_vma_pin_ww(vma, ww, size, alignment, flags | PIN_GLOBAL);
> -       else
> -               ret = i915_vma_pin(vma, size, alignment, flags | PIN_GLOBAL);
> +       ret = i915_vma_pin_ww(vma, ww, size, alignment, flags | PIN_GLOBAL);
>
>         if (ret)
>                 return ERR_PTR(ret);
> @@ -959,6 +958,29 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
>         return vma;
>  }
>
> +struct i915_vma * __must_check
> +i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
> +                        const struct i915_ggtt_view *view,
> +                        u64 size, u64 alignment, u64 flags)
> +{
> +       struct i915_gem_ww_ctx ww;
> +       struct i915_vma *ret;
> +       int err;
> +
> +       for_i915_gem_ww(&ww, err, true) {
> +               err = i915_gem_object_lock(obj, &ww);
> +               if (err)
> +                       continue;
> +
> +               ret = i915_gem_object_ggtt_pin_ww(obj, &ww, view, size,
> +                                                 alignment, flags);
> +               if (IS_ERR(ret))
> +                       err = PTR_ERR(ret);

Maybe s/ret/vma/ ? Seeing ret makes me think it's an integer.

Anyway,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

> +       }
> +
> +       return err ? ERR_PTR(err) : ret;
> +}
> +
>  int
>  i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
>                        struct drm_file *file_priv)
> --
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1bfadd9127fc..8322abe194da 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1842,13 +1842,10 @@  i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
 			    const struct i915_ggtt_view *view,
 			    u64 size, u64 alignment, u64 flags);
 
-static inline struct i915_vma * __must_check
+struct i915_vma * __must_check
 i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
 			 const struct i915_ggtt_view *view,
-			 u64 size, u64 alignment, u64 flags)
-{
-	return i915_gem_object_ggtt_pin_ww(obj, NULL, view, size, alignment, flags);
-}
+			 u64 size, u64 alignment, u64 flags);
 
 int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			   unsigned long flags);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 527228d4da7e..6002045bd194 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -877,6 +877,8 @@  i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
 	struct i915_vma *vma;
 	int ret;
 
+	GEM_WARN_ON(!ww);
+
 	if (flags & PIN_MAPPABLE &&
 	    (!view || view->type == I915_GGTT_VIEW_NORMAL)) {
 		/*
@@ -936,10 +938,7 @@  i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
 			return ERR_PTR(ret);
 	}
 
-	if (ww)
-		ret = i915_vma_pin_ww(vma, ww, size, alignment, flags | PIN_GLOBAL);
-	else
-		ret = i915_vma_pin(vma, size, alignment, flags | PIN_GLOBAL);
+	ret = i915_vma_pin_ww(vma, ww, size, alignment, flags | PIN_GLOBAL);
 
 	if (ret)
 		return ERR_PTR(ret);
@@ -959,6 +958,29 @@  i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
 	return vma;
 }
 
+struct i915_vma * __must_check
+i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
+			 const struct i915_ggtt_view *view,
+			 u64 size, u64 alignment, u64 flags)
+{
+	struct i915_gem_ww_ctx ww;
+	struct i915_vma *ret;
+	int err;
+
+	for_i915_gem_ww(&ww, err, true) {
+		err = i915_gem_object_lock(obj, &ww);
+		if (err)
+			continue;
+
+		ret = i915_gem_object_ggtt_pin_ww(obj, &ww, view, size,
+						  alignment, flags);
+		if (IS_ERR(ret))
+			err = PTR_ERR(ret);
+	}
+
+	return err ? ERR_PTR(err) : ret;
+}
+
 int
 i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 		       struct drm_file *file_priv)