On Wed, Jul 01, 2015 at 01:26:59PM +0100, Tvrtko Ursulin wrote: > > On 07/01/2015 12:09 PM, Chris Wilson wrote: > >On Wed, Jul 01, 2015 at 11:58:46AM +0100, Tvrtko Ursulin wrote: > >>On 07/01/2015 10:59 AM, Chris Wilson wrote: > >>>On Wed, Jul 01, 2015 at 10:48:59AM +0100, Tvrtko Ursulin wrote: > >>>>Previously the canceled worker would allow another worker to be > >>>>created in case it failed (obj->userptr.work != &work->work; ret = > >>>>0;) and now it still does since obj->userptr.work remains at NULL > >>>>from cancellation. > >>>> > >>>>Both seem wrong, am I missing the change? > >>> > >>>No, the obj->userptr.work must remain NULL until a new get_pages() > >>>because we don't actually know if this worker's gup was before or after > >>>the cancellation - mmap_sem vs struct_mutex ordering. > >> > >>No one is not wrong, or no I was not missing the change? > > > >The only change is that we don't change the value of userptr.work if it > >is set to something else. The only time it should be different was if it > >had been cancelled and so NULL. The patch just makes it so that a coding > >error is less damaging - and I think easier to read because of that. > > > >>I am thinking more and more that we should just mark it canceled > >>forever and not allow get_pages to succeed ever since. > > > >Yes, I toyed with that yesterday in response to you being able to alias > >a GTT mmap address with the userptr after munmap(userptr.ptr). The > >problem is that cancel_userptr() is caller for any change in the CPU > >PTE's, including mprotect() or cow after forking. Both of those are > >valid situations where we want to keep the userptr around, but with a > >new gup. > > Why do we want that? I would be surprised if someone is using it > like that. How would it be defined on the GEM handle level even? I would be surprised as well, but it is a race condition we can handle correctly and succinctly. The race is just bo = userptr(ptr, size); set-to-domain(bo); mremap(ptr, newptr, size); set-to-domain(bo); // or exec(bo); -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
