Re: [PATCH 05/13] drm/ttm: ttm_fault callback to allow driver to handle bo placement V5

[Thomas Hellstrom <thomas@shipmail.org>]

Dave Airlie wrote:
> On Wed, Apr 7, 2010 at 8:21 PM, Jerome Glisse <jglisse@redhat.com> 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&#174; 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
--