On Thu, Aug 22, 2019 at 10:16 AM Jason Gunthorpe <jgg@xxxxxxxx> wrote: > > On Wed, Aug 21, 2019 at 05:41:51PM +0200, Daniel Vetter wrote: > > > > Hm, I thought the page table locks we're holding there already prevent any > > > sleeping, so would be redundant? But reading through code I think that's > > > not guaranteed, so yeah makes sense to add it for invalidate_range_end > > > too. I'll respin once I have the ack/nack from scheduler people. > > > > So I started to look into this, and I'm a bit confused. There's no > > _nonblock version of this, so does this means blocking is never allowed, > > or always allowed? > > RDMA has a mutex: > > ib_umem_notifier_invalidate_range_end > rbt_ib_umem_for_each_in_range > invalidate_range_start_trampoline > ib_umem_notifier_end_account > mutex_lock(&umem_odp->umem_mutex); > > I'm working to delete this path though! > > nonblocking or not follows the start, the same flag gets placed into > the mmu_notifier_range struct passed to end. Ok, makes sense. I guess that also means the might_sleep (I started on that) in invalidate_range_end also needs to be conditional? Or not bother with a might_sleep in invalidate_range_end since you're working on removing the last sleep in there? > > From a quick look through implementations I've only seen spinlocks, and > > one up_read. So I guess I should wrape this callback in some unconditional > > non_block_start/end, but I'm not sure. > > For now, we should keep it the same as start, conditionally blocking. > > Hopefully before LPC I can send a RFC series that eliminates most > invalidate_range_end users in favor of common locking.. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
