From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhang, Yu" Subject: Re: [PATCH 2/8] drm/i915: Adds graphic address space ballooning logic Date: Fri, 26 Sep 2014 16:26:20 +0800 Message-ID: <5425232C.3030107@linux.intel.com> References: <1411152428-7226-1-git-send-email-jike.song@intel.com> <1411152428-7226-3-git-send-email-jike.song@intel.com> <20140919080557.GC21738@nuc-i3427.alporthouse.com> <20140919200000.GH21738@nuc-i3427.alporthouse.com> <20140923082626.GV15734@phenom.ffwll.local> <20140923091902.GG8727@nuc-i3427.alporthouse.com> <20140923112503.GD15734@phenom.ffwll.local> <5422BAA6.8090105@linux.intel.com> <20140924132118.GA30278@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 890AB6E19A for ; Fri, 26 Sep 2014 01:32:11 -0700 (PDT) In-Reply-To: <20140924132118.GA30278@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Daniel Vetter , "Tian, Kevin" , "Song, Jike" , "Vetter, Daniel" , "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org Hi Chris & Daniel, Thanks for your comments. Following are my understandings about the changes needed for this patch: 1> We do not need the guard page anymore between different VMs. For the very last physical GTT entry, let's keep it pointing to a guard page. 2> To reserve the GMs in our balloon code, drm_mm_reserve_node() is preferred than the current drm_mm_insert_node_in_range_generic(). Am I correct? Please correct me if anything wrong. Thanks! :) Yu On 9/24/2014 9:21 PM, Chris Wilson wrote: > On Wed, Sep 24, 2014 at 08:35:50PM +0800, Zhang, Yu wrote: >> 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? > > No. It is just that drm_mm_reserve_node() was added to have the precise > semantics expected here with the strict checks. And should work better > with dynamic ballooning... > -Chris >