From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Skeggs Subject: Re: [PATCH 0/1][RFC] drm/ttm Improved io_mem_reserve / io_mem_free_calling Date: Fri, 12 Nov 2010 08:46:44 +1000 Message-ID: <1289515604.9605.17.camel@nisroch> References: <1289464917-25085-1-git-send-email-thellstrom@vmware.com> <4CDC1ED4.4070903@vmware.com> Reply-To: skeggsb@gmail.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-vw0-f49.google.com (mail-vw0-f49.google.com [209.85.212.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 74B0C9EEEB for ; Thu, 11 Nov 2010 14:47:08 -0800 (PST) Received: by vws1 with SMTP id 1so566514vws.36 for ; Thu, 11 Nov 2010 14:47:07 -0800 (PST) In-Reply-To: <4CDC1ED4.4070903@vmware.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Thomas Hellstrom Cc: "airlied@redhat.com" , "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org On Thu, 2010-11-11 at 17:50 +0100, Thomas Hellstrom wrote: > On 11/11/2010 04:27 PM, Jerome Glisse wrote: > > On Thu, Nov 11, 2010 at 3:41 AM, Thomas Hellstrom wrote: > > > >> The following patch is really intended for the next merge window. > >> > >> RFC: > >> 1) Are there any implementations of driver::io_mem_reserve today that can't use > >> the fastpath? > >> 2) Can we put an atomic requirement on driver::io_mem_reserve / > >> driver::io_mem_free? > >> > >> The patch improves on the io_mem_reserve / io_mem_free calling sequences by > >> introducing a fastpath and an optional eviction mechanism. > >> > >> The fastpath is enabled by default and is switched off by the driver setting > >> struct ttm_mem_type_manager::io_reserve_fastpath to false on mem type init. > >> With the fastpath no locking occurs, and io_mem_free is never called. > >> I'm not sure if there are any implementations today that could not use the > >> fastpath. > >> > >> As mentioned in the patch, if the fastpath is disabled, calls to > >> io_mem_reserve and io_mem_free are exactly balanced, and refcounted within > >> struct ttm_mem_reg so that io_mem_reserve should never be called recursively > >> for the same struct ttm_mem_reg. > >> Locking is required to make sure that ptes are never present on when the > >> underlying memory region is not reserved. Currently I'm using > >> man::io_reserve_mutex for this. Can we use a spinlock? That would require > >> io_mem_reserve and io_mem_free to be atomic. > >> > >> Optionally, there is an eviction mechanism that is activated by setting > >> struct ttm_mem_type_manager::use_io_reserve_lru to true when initialized. > >> If the eviction mechanism is activated, and io_mem_reserve returns -EAGAIN, > >> it will attempt to kill user-space mappings to free up reserved regions. > >> Kernel mappings (ttm_bo_kmap) are not affected. > >> > >> > > Radeon can use fast path, i think nouveau can too. I am not sure we > > can consider io_mem_reserve as atomic. Use case i fear is GPU with > > remappable apperture i don't know what kind of code we would need for > > that and it might sleep. Thought my first guess is that it likely can > > be done atomicly. > > > > In that case, I think I will change it to a spinlock, with a code > comment that it can be changed to a mutex later if it turns out to be > very hard / impossible to implement atomic operations. Another possible > concern is the execution of umap_mapping_range() that may in some cases > be long. Perhaps too long to use a spinlock. I'd rather keep the mutex personally, the code I have in development uses mutexes itself beyond the io_mem_reserve/io_mem_free calls. An earlier revision used spinlocks, but it wasn't very nice. Ben. > > > Quick review of the patch looks good, i will try to take a closer look latter. > > > > Cheers, > > Jerome Glisse > > > > Great. Thanks, > Thomas >