Re: [PATCH 2/8] drm/i915: Adds graphic address space ballooning logic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Daniel & Chris,

Thank you very much for your comments, And sorry for my late reply.:) I was focusing on other tasks previously.
  See my questions below:

On 9/23/2014 7:25 PM, Daniel Vetter wrote:
On Tue, Sep 23, 2014 at 10:19:02AM +0100, Chris Wilson wrote:
On Tue, Sep 23, 2014 at 10:26:26AM +0200, Daniel Vetter wrote:
On Fri, Sep 19, 2014 at 09:00:00PM +0100, Chris Wilson wrote:
On Fri, Sep 19, 2014 at 06:21:46PM +0000, Tian, Kevin wrote:
From: Chris Wilson
The implementation also looks backwards. To work correctly with the GTT
allocator, you need to preallocate the reserved space such that it can
only allocate from the allowed ranges. Similarly, it should evict any
conflicting nodes when deballooning.

Could you elaborate a bit for above suggestion?

My expectation was that the dev_priv->gtt.base.vm would contain exactly
two holes after setup (in the mappable and non-mappable range). To do
that you would explicitly reserve everything barred from this client
using a set of drm_mm_reserve_node()

Essentially a reserve_node implements what you open-code with
insert_node_range right now.

Heh, there is a big difference. One inserts exactly where you ask and
fails if it conflicts, the other inserts where it feels like within that
range.

Do you mean drm_mm_search_free_in_range_generic() may not get reserve the exact range we are expecting to? Is this why you'd prefer the drm_mm_reserve_node()?

Besides, the ggtt_vm->mm is just initialized right before the ballooning code in routine i915_gem_setup_global_gtt(), so is there any chance the range to be partitioned out is already reserved by someone else?


Well if the the requested size matches the range exactly then it will be
the same. Which iirc is what's going on here I think.

One issue aside with both this and with the PDE reservations for gen7 is
that there are now other thins in the ggtt drm_mm allocator than just gem
objects. Which means our debugfs files are now less useful.

It might be useful to augment that dumper with one that dumps everything.
We could add a few bits of driver-private tags in drm_mm_node (there's
space) to figure out what kind of object it is. Would be a great follow-up
task.

I think moving the other way and making them all objects so that we can
tie them into evection and the shrinker, use more interesting allocation
strategies, improve integration with debugging etc.

Hm, not sure yet since it will be a lot of work at least. But I guess we
could untangle the meaning of obj->pin a bit and add an unbind vfunc which
adds some magic. But there's a lot of stuff attached to a gem bo that just
doesn't make a lot of sense really, so maybe a better option would be to
subclass a struct i915_ggtt_vma with special magic. Dunno really.

Sorry, not sure what these comments are about. :) I'll need time to read the code. Could you please elaborate a bit? Thanks!


P.S. about the guard page: for now, the current logic reserves a guard page between different guests and at the very last entry of the whole physical GTT. the previous comments says: "The CS prefetcher happens everywhere and so can read from the end of one range into the beginning of another clients". So I guess the guard page in current patch is necessary, right?

-Daniel


Thanks
Yu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux