Re: Unmappable VRAM patchset V3

[Jerome Glisse <glisse@freedesktop.org>]

On Wed, Feb 24, 2010 at 06:04:57PM +0100, Thomas Hellstrom wrote:
> Jerome Glisse wrote:
> >On Wed, Feb 24, 2010 at 01:37:42PM +0100, Thomas Hellstrom wrote:
> >>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
> >>
> >
> >My patchset doesn't change the code there, no_wait will be true
> >only if it's true as argument of ttm_bo_validate from the ttm
> >fault callback in the driver.
> >
> 
> But you did call ttm_bo_move_buffer from the fault callback?
> Given the above, it's illegal to call it with no_wait set to false,
> and undesirable to call it with no_wait set to true.
> 
> /Thomas
> 

I only call ttm_bo_validate from the fault callback, to validate
the bo into visible vram. I don't see why it would be undesirable
to call it with no_wait = true ?

Correct me if i am wrong but what we want is to try to move bo
without waiting at all on referencing other bo to avoid deadlock,
thus calling ttm_bo_validate with no_wait=true will return to
us with -EBUSY or -ERESTARTSYS if a bo we need to reserve to
move it away is already reserved, in this case we return to
the ttm fault handler which will return VM_FAULT_NOPAGE

The no_wait argument of ttm_bo_validate is pretty much only
use as an argument for reserving other bo in case we need
to evict or as an argument to ttm_bo_wait if we have to
wait for the bo. I can understand that we would like to
permit waiting on the bo we are faulting on, this is the
only place where i can understand having a different value
for validating.

Cheers,
Jerome

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