[PATCH v6 1/3] drm/i915/gvt: Use gtt_lock to protect gtt list
Du, Changbin
changbin.du at intel.com
Thu Apr 19 07:40:48 UTC 2018
On Fri, Apr 20, 2018 at 03:26:25PM +0800, colin.xu at intel.com wrote:
> From: Pei Zhang <pei.zhang at intel.com>
>
> The patch set splits out 3 small locks from the original big gvt lock:
> - gtt_lock is used to protect the gvt global gtt oos lists.
> - vgpu_lock protects per-vGPU data and logic, especially the vGPU
> trap emulation path.
> - sched_lock protects gvt scheudler structure, context schedule logic
> and vGPU's schedule data.
>
> GVT gtt contains 2 oos list, and one global mm list. Use a special
> gvt->gtt_lock to protect those 3 lists.
>
> v6:
> - Rebase to latest gvt-staging.
> v5:
> - Rebase to latest gvt-staging.
> - gtt access is already protected by vgpu_lock, so gtt_lock is
> unnecessary currently. Will keep for init/clean for further usage.
Maybe we should delay this fully overlayed gtt_lock to when really needed.
Who knows what the code will be changed to in future.
> v4:
> - Rebase to latest gvt-staging.
> - Add missing protection when access list in clean_spt_oos().
> - Fix locked recursive call of detach_oos_page when cleanup. (zhenyu)
> v3: Update to latest code base.
> v2: use gtt_lock to cover correct scope in ppgtt_allocate_oos_page().
>
> Performance comparison on Kabylake platform.
> - Configuration:
> Host: Ubuntu 16.04.
> Guest 1 & 2: Ubuntu 16.04.
>
> glmark2 score comparison:
> - Configuration:
> Host: glxgears.
> Guests: glmark2.
> +--------------------------------+-----------------+
> | Setup | glmark2 score |
> +--------------------------------+-----------------+
> | unified lock, iommu=on | 58~62 (avg. 60) |
> +--------------------------------+-----------------+
> | unified lock, iommu=igfx_off | 57~61 (avg. 59) |
> +--------------------------------+-----------------+
> | per-logic lock, iommu=on | 60~68 (avg. 64) |
> +--------------------------------+-----------------+
> | per-logic lock, iommu=igfx_off | 61~67 (avg. 64) |
> +--------------------------------+-----------------+
>
> lock_stat comparison:
> - Configuration:
> Stop lock stat immediately after boot up.
> Boot 2 VM Guests.
> Run glmark2 in guests.
> Start perf lock_stat for 20 seconds and stop again.
> - Legend: c - contentions; w - waittime-avg
> +------------+-----------------+-----------+---------------+------------+
> | | gvt_lock |sched_lock | vgpu_lock | gtt_lock |
> + lock type; +-----------------+-----------+---------------+------------+
> | iommu set | c | w | c | w | c | w | c | w |
> +------------+-------+---------+----+------+------+--------+-----+------+
> | unified; | 20697 | 839 |N/A | N/A | N/A | N/A | N/A | N/A |
> | on | | | | | | | | |
> +------------+-------+---------+----+------+------+--------+-----+------+
> | unified; | 21838 | 658.15 |N/A | N/A | N/A | N/A | N/A | N/A |
> | igfx_off | | | | | | | | |
> +------------+-------+---------+----+------+------+--------+-----+------+
> | per-logic; | 1553 | 1599.96 |9458|429.97| 5846 | 274.33 | 0 | 0.00 |
> | on | | | | | | | | |
> +------------+-------+---------+----+------+------+--------+-----+------+
> | per-logic; | 1911 | 1678.32 |8335|445.16| 5451 | 244.80 | 0 | 0.00 |
> | igfx_off | | | | | | | | |
> +------------+-------+---------+----+------+------+--------+-----+------+
>
> Signed-off-by: Pei Zhang <pei.zhang at intel.com>
> Signed-off-by: Colin Xu <colin.xu at intel.com>
> ---
> drivers/gpu/drm/i915/gvt/gtt.c | 7 +++++++
> drivers/gpu/drm/i915/gvt/gvt.c | 1 +
> drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
> 3 files changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 78e55aafc8bc..9ad3b8105e73 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -2109,6 +2109,8 @@ static void clean_spt_oos(struct intel_gvt *gvt)
> struct list_head *pos, *n;
> struct intel_vgpu_oos_page *oos_page;
>
> + mutex_lock(&gvt->gtt_lock);
> +
> WARN(!list_empty(>t->oos_page_use_list_head),
> "someone is still using oos page\n");
>
> @@ -2117,6 +2119,7 @@ static void clean_spt_oos(struct intel_gvt *gvt)
> list_del(&oos_page->list);
> kfree(oos_page);
> }
> + mutex_unlock(&gvt->gtt_lock);
> }
>
> static int setup_spt_oos(struct intel_gvt *gvt)
> @@ -2126,6 +2129,8 @@ static int setup_spt_oos(struct intel_gvt *gvt)
> int i;
> int ret;
>
> + mutex_lock(&gvt->gtt_lock);
> +
> INIT_LIST_HEAD(>t->oos_page_free_list_head);
> INIT_LIST_HEAD(>t->oos_page_use_list_head);
>
> @@ -2142,6 +2147,8 @@ static int setup_spt_oos(struct intel_gvt *gvt)
> list_add_tail(&oos_page->list, >t->oos_page_free_list_head);
> }
>
> + mutex_unlock(&gvt->gtt_lock);
> +
> gvt_dbg_mm("%d oos pages preallocated\n", i);
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 61bd14fcb649..91c37f115780 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -379,6 +379,7 @@ int intel_gvt_init_device(struct drm_i915_private *dev_priv)
> idr_init(&gvt->vgpu_idr);
> spin_lock_init(&gvt->scheduler.mmio_context_lock);
> mutex_init(&gvt->lock);
> + mutex_init(&gvt->gtt_lock);
> gvt->dev_priv = dev_priv;
>
> init_device_info(gvt);
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 6ec888822a0f..8860343e850f 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -296,6 +296,8 @@ struct intel_vgpu_type {
>
> struct intel_gvt {
> struct mutex lock;
> + struct mutex gtt_lock;
> +
> struct drm_i915_private *dev_priv;
> struct idr vgpu_idr; /* vGPU IDR pool */
>
> --
> 2.17.0
>
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
--
Thanks,
Changbin Du
More information about the intel-gvt-dev
mailing list