[PATCH 2/2] drm/i915/gvt: Refine the combined intel_vgpu_oos_page struct to save memory

[Zhao, Yakui]

>-----Original Message-----
>From: Zhenyu Wang [mailto:zhenyuw at linux.intel.com]
>Sent: Monday, February 18, 2019 3:58 PM
>To: Zhao, Yakui <yakui.zhao at intel.com>
>Cc: intel-gvt-dev at lists.freedesktop.org
>Subject: Re: [PATCH 2/2] drm/i915/gvt: Refine the combined
>intel_vgpu_oos_page struct to save memory
>
>On 2019.02.14 17:09:17 +0800, Zhao Yakui wrote:
>> The intel_vgpu_oos_page uses the combined structure, which embdeds the
>> tracked page. As it is allocated by kmalloc, the size(4140) is aligned
>> to 8192. The 8192 oos_pages will waste about 32M memory.
>> So the tracked page is split from the intel_vgpu_oos_page. And this
>> will help to assure that the access of tracked page is cache aligned.
>>
>> Another minor change is that it doesn't need to be cleared to zero as
>> it is writen firstly when one page is added to oos_page list.
>>
>> Signed-off-by: Zhao Yakui <yakui.zhao at intel.com>
>> ---
>>  drivers/gpu/drm/i915/gvt/gtt.c | 7 +++++++
>> drivers/gpu/drm/i915/gvt/gtt.h | 2 +-
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c
>> b/drivers/gpu/drm/i915/gvt/gtt.c index c7103dd..9ba1ed8 100644
>> --- a/drivers/gpu/drm/i915/gvt/gtt.c
>> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
>> @@ -2489,6 +2489,7 @@ static void clean_spt_oos(struct intel_gvt *gvt)
>>  	list_for_each_safe(pos, n, &gtt->oos_page_free_list_head) {
>>  		oos_page = container_of(pos, struct intel_vgpu_oos_page,
>list);
>>  		list_del(&oos_page->list);
>> +		free_page((unsigned long)oos_page->mem);
>>  		kfree(oos_page);
>>  	}
>>  }
>> @@ -2509,6 +2510,12 @@ static int setup_spt_oos(struct intel_gvt *gvt)
>>  			ret = -ENOMEM;
>>  			goto fail;
>>  		}
>> +		oos_page->mem = (void *)__get_free_pages(GFP_KERNEL, 0);
>> +		if (!oos_page->mem) {
>> +			ret = -ENOMEM;
>> +			kfree(oos_page);
>> +			goto fail;
>> +		}
>>
>>  		INIT_LIST_HEAD(&oos_page->list);
>>  		INIT_LIST_HEAD(&oos_page->vm_list);
>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h
>> b/drivers/gpu/drm/i915/gvt/gtt.h index d8cb04c..e9f72a6 100644
>> --- a/drivers/gpu/drm/i915/gvt/gtt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
>> @@ -221,7 +221,7 @@ struct intel_vgpu_oos_page {
>>  	struct list_head list;
>>  	struct list_head vm_list;
>>  	int id;
>> -	unsigned char mem[I915_GTT_PAGE_SIZE];
>> +	void *mem;
>>  };
>>
>>  #define GTT_ENTRY_NUM_IN_ONE_PAGE 512
>> --
>
>Looks good to me.
>
>Reviewed-by: Zhenyu Wang <zhenyuw at linux.intel.com>
>
Thanks for the review.

>btw, last time check I think we could skip oos page setup if out of sync is not
>enabled at all, would you add that change too?

Now the setup_oos_page is controlled by the "enable_out_of_sync". When the "out_of_sync" is not disabled, it will skip the oos_page.
So it doesn't need to change it.



>
>thanks
>
>--
>Open Source Technology Center, Intel ltd.
>
>$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827