Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1506031

Re: [Intel-gfx] [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store

From Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Newsgroups linux.kernel
Subject Re: [Intel-gfx] [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store
Date 2016-10-21 17:00 +0200
Message-ID <suJsC-73K-15@gated-at.bofh.it> (permalink)
References <suIPU-6HU-11@gated-at.bofh.it> <suIPU-6HU-33@gated-at.bofh.it> <suIZF-6Lk-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 21/10/2016 15:27, Chris Wilson wrote:
> On Fri, Oct 21, 2016 at 03:11:22PM +0100, Tvrtko Ursulin wrote:
>> @@ -2236,18 +2233,16 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
>>  	BUG_ON(obj->base.read_domains & I915_GEM_GPU_DOMAINS);
>>  	BUG_ON(obj->base.write_domain & I915_GEM_GPU_DOMAINS);
>>
>> -	max_segment = swiotlb_max_size();
>> -	if (!max_segment)
>> -		max_segment = rounddown(UINT_MAX, PAGE_SIZE);
>> -
>> -	st = kmalloc(sizeof(*st), GFP_KERNEL);
>> -	if (st == NULL)
>> -		return -ENOMEM;
>> -
>>  	page_count = obj->base.size / PAGE_SIZE;
>> -	if (sg_alloc_table(st, page_count, GFP_KERNEL)) {
>> -		kfree(st);
>> +	pages = drm_malloc_gfp(page_count, sizeof(struct page *),
>> +			       GFP_TEMPORARY | __GFP_ZERO);
>> +	if (!pages)
>>  		return -ENOMEM;
>
> Full circle! The whole reason this exists was to avoid that vmalloc. I
> don't really want it back...

Yes, it is not ideal.

However all objects under 4 MiB should fall under the kmalloc fast path 
(8 KiB of struct page pointers, which should always be available), and 
possibly bigger ones as well if there is room.

It only fallbacks to vmalloc for objects larger than 4 MiB, when it also 
fails to get the page pointer array from the SLAB (GFP_TEMPORARY).

So perhaps SLAB would most of the time have some nice chunks for us to 
pretty much limit vmalloc apart for the huge objects? And then, is 
creation time for those so performance critical?

I came up with this because I started to dislike my previous 
sg_trim_table approach as too ugly. It had an advantage of simplicity 
after fixing the theoretical chunk overflow in sg_alloc_table_from_pages.

If we choose none of the two, only third option I can think of is to 
allocate the sg table as we add entries to it. I don't think it would be 
hard to do that.

Regards,

Tvrtko

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/5] Avoid pessimistic scatter-gather allocation Tvrtko Ursulin <tursulin@ursulin.net> - 2016-10-21 16:20 +0200
  [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for allocating object backing store Tvrtko Ursulin <tursulin@ursulin.net> - 2016-10-21 16:20 +0200
    Re: [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages for  allocating object backing store Chris Wilson <chris@chris-wilson.co.uk> - 2016-10-21 16:30 +0200
      Re: [Intel-gfx] [PATCH 4/5] drm/i915: Use __sg_alloc_table_from_pages  for allocating object backing store Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> - 2016-10-21 17:00 +0200
  [PATCH 3/5] lib/scatterlist: Introduce and export __sg_alloc_table_from_pages Tvrtko Ursulin <tursulin@ursulin.net> - 2016-10-21 16:20 +0200
  [PATCH 1/5] lib/scatterlist: Fix offset type in sg_alloc_table_from_pages Tvrtko Ursulin <tursulin@ursulin.net> - 2016-10-21 16:20 +0200
    Re: [PATCH 1/5] lib/scatterlist: Fix offset type in  sg_alloc_table_from_pages Marek Szyprowski <m.szyprowski@samsung.com> - 2016-10-24 09:30 +0200
  [PATCH 2/5] lib/scatterlist: Avoid potential scatterlist entry overflow Tvrtko Ursulin <tursulin@ursulin.net> - 2016-10-21 16:20 +0200

csiph-web