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 -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
