[PATCH v6 1/3] drm/i915/gvt: Use gtt_lock to protect gtt list

Colin Xu Colin.Xu at intel.com
Fri Apr 20 08:01:46 UTC 2018 

On 04/19/2018 03:40 PM, Du, Changbin wrote:
> 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.

OK I'll remake the patch set to keep only vgpu_lock and sched_lock.
gtt_lock can be added when it's necessary.

> OK
>> 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(&gtt->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(&gtt->oos_page_free_list_head);
>>   	INIT_LIST_HEAD(&gtt->oos_page_use_list_head);
>>   
>> @@ -2142,6 +2147,8 @@ static int setup_spt_oos(struct intel_gvt *gvt)
>>   		list_add_tail(&oos_page->list, &gtt->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

-- 
Best Regards,
Colin Xu

More information about the intel-gvt-dev mailing list