From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH 05/13] drm/ttm: ttm_fault callback to allow driver to handle bo placement V5 Date: Fri, 09 Apr 2010 14:39:53 +0200 Message-ID: <4BBF2019.7040100@shipmail.org> References: <1270635691-2933-1-git-send-email-jglisse@redhat.com> <1270635691-2933-2-git-send-email-jglisse@redhat.com> <1270635691-2933-3-git-send-email-jglisse@redhat.com> <1270635691-2933-4-git-send-email-jglisse@redhat.com> <1270635691-2933-5-git-send-email-jglisse@redhat.com> <1270635691-2933-6-git-send-email-jglisse@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.sourceforge.net To: Dave Airlie Cc: Jerome Glisse , dri-devel@lists.sf.net List-Id: dri-devel@lists.freedesktop.org Dave Airlie wrote: > On Wed, Apr 7, 2010 at 8:21 PM, Jerome Glisse wrote: > >> On fault the driver is given the opportunity to perform any operation >> it sees fit in order to place the buffer into a CPU visible area of >> memory. This patch doesn't break TTM users, nouveau, vmwgfx and radeon >> should keep working properly. Future patch will take advantage of this >> infrastructure and remove the old path from TTM once driver are >> converted. >> >> V2 return VM_FAULT_NOPAGE if callback return -EBUSY or -ERESTARTSYS >> V3 balance io_mem_reserve and io_mem_free call, fault_reserve_notify >> is responsible to perform any necessary task for mapping to succeed >> V4 minor cleanup, atomic_t -> bool as member is protected by reserve >> mecanism from concurent access >> V5 the callback is now responsible for iomapping the bo and providing >> a virtual address this simplify TTM and will allow to get rid of >> TTM_MEMTYPE_FLAG_NEEDS_IOREMAP >> > > Okay I've applied all these to drm-next and it fails to run X for longer than > 5 mins on my 32-bit machine. > > The whole IO reserve thing looks like a bad idea for anything but kmap, > since it ioremap's the pages, but we have limited ioremap space on 32-bit, > and you hit that and vmallocs all start failing soon after. > > >> - ret = ttm_bo_pci_offset(bdev, &bo->mem, &bus_base, &bus_offset, >> - &bus_size); >> - if (unlikely(ret != 0)) { >> + ret = ttm_mem_io_reserve(bdev, &bo->mem); >> + if (ret) { >> retval = VM_FAULT_SIGBUS; >> goto out_unlock; >> } >> > > This is the start of the insanity, since ever bo that takes a > userspace pagefault > ends up using ioremap space for *no* reason whatsoever at all. You only need > to use ioremap space if the *kernel* is accessing the bo not if userspace is, > so really only in the kmap space. > > I've dropped it all from drm-next until you can resolve this. > > Dave. > Indeed, ttm_mem_io_reserve() shouldn't do anything like that. It should place the bo data in a mappable region. If there is a copy involving ioremap() to do so, it should be iounmapped() before the operation is finished. Thomas ------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev --