Re: Unmappable VRAM patchset V3

[Thomas Hellstrom <thellstrom@vmware.com>]

Jerome Glisse wrote:
> On Tue, Feb 23, 2010 at 02:05:50PM +0100, Thomas Hellstrom wrote:
>   
>> Jerome Glisse wrote:
>>     
>>> On Mon, Feb 22, 2010 at 08:58:28PM +0100, Thomas Hellstrom wrote:
>>>       
>>>> Jerome Glisse wrote:
>>>>         
>>>>> On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote:
>>>>>           
>>>>>> Jerome Glisse wrote:
>>>>>>             
>>>>>>> Thomas i think i addressed your concern here, the ttm_bo_validate
>>>>>>> didn't needed a new argument or i did not understand what was
>>>>>>> necessary beside no_wait. In this patchset we check the value
>>>>>>> of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
>>>>>>> we return VM_FAULT_NOPAGE.
>>>>>>>               
>>>>>> Well, if we from the fault callback call any function that might
>>>>>> call ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure
>>>>>> that we never wait, but return -EBUSY all the way back to the
>>>>>> fault function. Such a case may be ttm_bo_validate that calls
>>>>>> ttm_bo_evict_first, or something causing a swapout...
>>>>>> ttm_bo_validate currently doesn't have that functionality,
>>>>>> because @no_wait just means don't wait for GPU.
>>>>>>             
>>>>> What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't
>>>>> use mutex or kernel code path that might sleep ?
>>>>>           
>>>> I mean waiting while reserving a bo. (If another thread has the bo
>>>> reserved).
>>>>
>>>>         
>>>>> After a new review i don't think we ever wait for the GPU with the current
>>>>> patch and as far as i can tell we will return EBUSY or ERESTART all the
>>>>> way up.
>>>>>
>>>>> Cheers,
>>>>> Jerome
>>>>>           
>>>> If there is *no* code path trying to reserve a bo or create a
>>>> user-space visible object from within the fault handler, it should
>>>> be ok.
>>>>
>>>> /Thomas
>>>>
>>>>
>>>>         
>>> Did a new review again here is the call chain :
>>> ttm_bo_move_buffer
>>>  ttm_bo_mem_space
>>>    ttm_bo_mem_force_space
>>>      ttm_mem_evict_first
>>>        ttm_bo_reserve_locked (no_wait = true)
>>>       
>> Here ttm_mem_evict_fist may wait for unreserve IIRC (the -EBUSY
>> return from ttm_bo_reserve_locked) is not propagated back.
>>     
>
> The code is not straightforward but if no_wait is true the
> -EBUSY of ttm_bo_reserve_locked will be propagated back.
>   

The point is that we don't want to set no_wait to true, because it's OK 
to wait for GPU. We want to add an extra argument no_wait_reserve.

/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
--