Re: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

Re: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Jerome Glisse]

> * Add an interval tree implementation for ODP umems. Create an interval tree
>   for each ucontext (including a count of the number of ODP MRs in this
>   context, mutex, etc.), and register ODP umems in the interval tree.
> * Add MMU notifiers handling functions, using the interval tree to notify only
>   the relevant umems and underlying MRs.
> * Register to receive MMU notifier events from the MM subsystem upon ODP MR
>   registration (and unregister accordingly).
> * Add a completion object to synchronize the destruction of ODP umems.
> * Add mechanism to abort page faults when there's a concurrent invalidation.
> 
> The way we synchronize between concurrent invalidations and page faults is by
> keeping a counter of currently running invalidations, and a sequence number
> that is incremented whenever an invalidation is caught. The page fault code
> checks the counter and also verifies that the sequence number hasn't
> progressed before it updates the umem's page tables. This is similar to what
> the kvm module does.
> 
> There's currently a rare race in the code when registering a umem in the
> middle of an ongoing notifier. The proper fix is to either serialize the
> insertion to our umem tree with mm_lock_all or use a ucontext wide running
> notifiers count for retries decision. Either is ugly and can lead to some sort
> of starvation. The current workaround is ugly as well - now the user can end
> up with mapped addresses that are not in the user's address space (although it
> is highly unlikely).

I have been trying to wrap my head around this comment. I am totaly unfamiliar
with RDMA code, but from quick look at it when registering umem you take the
mmap_sem in read mode so any munmap from userspace would be serialize. Really
the worst that can happen is that a umem pointing to a mmaped file that is
concurently truncated but even then the address is still valid, but it should
result in a SIGBUS which here is obviously harder to report (again dunno how
RDMA works).

So am i missing something ?

> 
> Signed-off-by: Sagi Grimberg <sagig@xxxxxxxxxxxx>
> Signed-off-by: Shachar Raindel <raindel@xxxxxxxxxxxx>
> Signed-off-by: Haggai Eran <haggaie@xxxxxxxxxxxx>
> Signed-off-by: Yuval Dagan <yuvalda@xxxxxxxxxxxx>
> ---
>  drivers/infiniband/Kconfig            |   1 +
>  drivers/infiniband/core/Makefile      |   2 +-
>  drivers/infiniband/core/umem.c        |   2 +-
>  drivers/infiniband/core/umem_odp.c    | 337 +++++++++++++++++++++++++++++++++-
>  drivers/infiniband/core/umem_rbtree.c |  94 ++++++++++
>  drivers/infiniband/core/uverbs_cmd.c  |  16 ++
>  include/rdma/ib_umem_odp.h            |  56 ++++++
>  include/rdma/ib_verbs.h               |  16 ++
>  8 files changed, 512 insertions(+), 12 deletions(-)
>  create mode 100644 drivers/infiniband/core/umem_rbtree.c
> 
> diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
> index 089a2c2..b899531 100644
> --- a/drivers/infiniband/Kconfig
> +++ b/drivers/infiniband/Kconfig
> @@ -41,6 +41,7 @@ config INFINIBAND_USER_MEM
>  config INFINIBAND_ON_DEMAND_PAGING
>  	bool "InfiniBand on-demand paging support"
>  	depends on INFINIBAND_USER_MEM
> +	select MMU_NOTIFIER
>  	default y
>  	---help---
>  	  On demand paging support for the InfiniBand subsystem.
> diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile
> index c58f791..acf7367 100644
> --- a/drivers/infiniband/core/Makefile
> +++ b/drivers/infiniband/core/Makefile
> @@ -11,7 +11,7 @@ obj-$(CONFIG_INFINIBAND_USER_ACCESS) +=	ib_uverbs.o ib_ucm.o \
>  ib_core-y :=			packer.o ud_header.o verbs.o sysfs.o \
>  				device.o fmr_pool.o cache.o netlink.o
>  ib_core-$(CONFIG_INFINIBAND_USER_MEM) += umem.o
> -ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o
> +ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o umem_rbtree.o
>  
>  ib_mad-y :=			mad.o smi.o agent.o mad_rmpp.o
>  
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index e9798e0..014977f 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -72,7 +72,7 @@ static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int d
>   * ib_umem_get - Pin and DMA map userspace memory.
>   *
>   * If access flags indicate ODP memory, avoid pinning. Instead, stores
> - * the mm for future page fault handling.
> + * the mm for future page fault handling in conjuction with MMU notifiers.
>   *
>   * @context: userspace context to pin memory for
>   * @addr: userspace virtual address to start at
> diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
> index 0c90ce50..c048269 100644
> --- a/drivers/infiniband/core/umem_odp.c
> +++ b/drivers/infiniband/core/umem_odp.c
> @@ -41,26 +41,204 @@
>  #include <rdma/ib_umem.h>
>  #include <rdma/ib_umem_odp.h>
>  
> +void ib_umem_notifier_start_account(struct ib_umem *item)
> +{
> +	int notifiers_count;
> +	mutex_lock(&item->odp_data->umem_mutex);
> +	/*
> +	 * Avoid performing another locked operation, as we are
> +	 * already protected by the wrapping mutex.
> +	 */
> +	notifiers_count = atomic_read(&item->odp_data->notifiers_count) + 1;
> +	if (notifiers_count == 1)
> +		reinit_completion(&item->odp_data->notifier_completion);
> +	atomic_set(&item->odp_data->notifiers_count,
> +		   notifiers_count);
> +	mutex_unlock(&item->odp_data->umem_mutex);
> +}
> +EXPORT_SYMBOL(ib_umem_notifier_start_account);
> +
> +void ib_umem_notifier_end_account(struct ib_umem *item)
> +{
> +	int notifiers_count, notifiers_seq;
> +	mutex_lock(&item->odp_data->umem_mutex);
> +	/*
> +	 * This sequence increase will notify the QP page fault that
> +	 * the page that is going to be mapped in the spte could have
> +	 * been freed.
> +	 */
> +	notifiers_seq = atomic_read(&item->odp_data->notifiers_seq) + 1;
> +	atomic_set(&item->odp_data->notifiers_seq,
> +		   notifiers_seq);
> +	/*
> +	 * The above sequence increase must be visible before the
> +	 * below count decrease, which is ensured by the smp_wmb below
> +	 * in conjunction with the smp_rmb in mmu_notifier_retry().
> +	 */
> +	smp_wmb();
> +
> +	notifiers_count = atomic_read(&item->odp_data->notifiers_count);
> +	/*
> +	 * This is a workaround for the unlikely case where we register a umem
> +	 * in the middle of an ongoing notifier.
> +	 */
> +	if (notifiers_count > 0)
> +		notifiers_count -= 1;
> +	else
> +		pr_warn("Got notifier end call without a previous start call");
> +	atomic_set(&item->odp_data->notifiers_count,
> +		   notifiers_count);
> +	if (notifiers_count == 0)
> +		complete_all(&item->odp_data->notifier_completion);
> +	mutex_unlock(&item->odp_data->umem_mutex);
> +}
> +
> +
> +static int ib_umem_notifier_release_trampoline(struct ib_umem *item, u64 start,
> +					       u64 end, void *cookie) {
> +	/*
> +	 * Increase the number of notifiers running, to
> +	 * prevent any further fault handling on this MR.
> +	 */
> +	ib_umem_notifier_start_account(item);
> +	item->odp_data->dying = 1;
> +	/* Make sure that the fact the umem is dying is out before we release
> +	 * all pending page faults. */
> +	smp_wmb();
> +	complete_all(&item->odp_data->notifier_completion);
> +	item->context->invalidate_range(item, ib_umem_start(item),
> +					ib_umem_end(item));
> +	return 0;
> +}
> +
> +static void ib_umem_notifier_release(struct mmu_notifier *mn,
> +				     struct mm_struct *mm)
> +{
> +	struct ib_ucontext *context = container_of(mn, struct ib_ucontext, mn);
> +
> +	if (!context->invalidate_range)
> +		return;
> +
> +	down_read(&context->umem_mutex);
> +
> +	rbt_ib_umem_for_each_in_range(&context->umem_tree, 0,
> +				      ULLONG_MAX,
> +				      ib_umem_notifier_release_trampoline,
> +				      NULL);
> +
> +	up_read(&context->umem_mutex);
> +}
> +
> +static int invalidate_page_trampoline(struct ib_umem *item, u64 start,
> +				      u64 end, void *cookie)
> +{
> +	ib_umem_notifier_start_account(item);
> +	item->context->invalidate_range(item, start, start + PAGE_SIZE);
> +	ib_umem_notifier_end_account(item);
> +	return 0;
> +}
> +
> +static void ib_umem_notifier_invalidate_page(struct mmu_notifier *mn,
> +					     struct mm_struct *mm,
> +					     unsigned long address)
> +{
> +	struct ib_ucontext *context = container_of(mn, struct ib_ucontext, mn);
> +
> +	if (!context->invalidate_range)
> +		return;
> +
> +	down_read(&context->umem_mutex);
> +	rbt_ib_umem_for_each_in_range(&context->umem_tree, address,
> +				      address + PAGE_SIZE,
> +				      invalidate_page_trampoline, NULL);
> +	up_read(&context->umem_mutex);
> +}
> +
> +static int invalidate_range_start_trampoline(struct ib_umem *item, u64 start,
> +					     u64 end, void *cookie)
> +{
> +	ib_umem_notifier_start_account(item);
> +	item->context->invalidate_range(item, start, end);
> +	return 0;
> +}
> +
> +static void ib_umem_notifier_invalidate_range_start(struct mmu_notifier *mn,
> +						    struct mm_struct *mm,
> +						    unsigned long start,
> +						    unsigned long end)
> +{
> +	struct ib_ucontext *context = container_of(mn, struct ib_ucontext, mn);
> +
> +	if (!context->invalidate_range)
> +		return;
> +
> +	down_read(&context->umem_mutex);
> +	rbt_ib_umem_for_each_in_range(&context->umem_tree, start,
> +				      end,
> +				      invalidate_range_start_trampoline, NULL);
> +	up_read(&context->umem_mutex);
> +}
> +
> +static int invalidate_range_end_trampoline(struct ib_umem *item, u64 start,
> +					   u64 end, void *cookie)
> +{
> +	ib_umem_notifier_end_account(item);
> +	return 0;
> +}
> +
> +static void ib_umem_notifier_invalidate_range_end(struct mmu_notifier *mn,
> +						  struct mm_struct *mm,
> +						  unsigned long start,
> +						  unsigned long end)
> +{
> +	struct ib_ucontext *context = container_of(mn, struct ib_ucontext, mn);
> +
> +	if (!context->invalidate_range)
> +		return;
> +
> +	down_read(&context->umem_mutex);
> +	rbt_ib_umem_for_each_in_range(&context->umem_tree, start,
> +				      end,
> +				      invalidate_range_end_trampoline, NULL);
> +	up_read(&context->umem_mutex);
> +}
> +
> +static struct mmu_notifier_ops ib_umem_notifiers = {
> +	.release                    = ib_umem_notifier_release,
> +	.invalidate_page            = ib_umem_notifier_invalidate_page,
> +	.invalidate_range_start     = ib_umem_notifier_invalidate_range_start,
> +	.invalidate_range_end       = ib_umem_notifier_invalidate_range_end,
> +};
> +
>  int ib_umem_odp_get(struct ib_ucontext *context, struct ib_umem *umem)
>  {
>  	int ret_val;
>  	struct pid *our_pid;
> +	struct mm_struct *mm = get_task_mm(current);
> +	BUG_ON(!mm);
>  
>  	/* Prevent creating ODP MRs in child processes */
>  	rcu_read_lock();
>  	our_pid = get_task_pid(current->group_leader, PIDTYPE_PID);
>  	rcu_read_unlock();
>  	put_pid(our_pid);
> -	if (context->tgid != our_pid)
> -		return -EINVAL;
> +	if (context->tgid != our_pid) {
> +		ret_val = -EINVAL;
> +		goto out_mm;
> +	}
>  
>  	umem->hugetlb = 0;
>  	umem->odp_data = kzalloc(sizeof(*umem->odp_data), GFP_KERNEL);
> -	if (!umem->odp_data)
> -		return -ENOMEM;
> +	if (!umem->odp_data) {
> +		ret_val = -ENOMEM;
> +		goto out_mm;
> +	}
> +	umem->odp_data->umem = umem;
>  
>  	mutex_init(&umem->odp_data->umem_mutex);
>  
> +	init_completion(&umem->odp_data->notifier_completion);
> +
>  	umem->odp_data->page_list = vzalloc(ib_umem_num_pages(umem) *
>  					    sizeof(*umem->odp_data->page_list));
>  	if (!umem->odp_data->page_list) {
> @@ -75,17 +253,66 @@ int ib_umem_odp_get(struct ib_ucontext *context, struct ib_umem *umem)
>  		goto out_page_list;
>  	}
>  
> +	/*
> +	 * When using MMU notifiers, we will get a
> +	 * notification before the "current" task (and MM) is
> +	 * destroyed. We use the umem_mutex lock to synchronize.
> +	 */
> +	down_write(&context->umem_mutex);
> +	context->odp_mrs_count++;
> +	if (likely(ib_umem_start(umem) != ib_umem_end(umem)))
> +		rbt_ib_umem_insert(&umem->odp_data->interval_tree,
> +				   &context->umem_tree);
> +	downgrade_write(&context->umem_mutex);
> +
> +	if (context->odp_mrs_count == 1) {
> +		/*
> +		 * Note that at this point, no MMU notifier is running
> +		 * for this context!
> +		 */
> +		INIT_HLIST_NODE(&context->mn.hlist);
> +		context->mn.ops = &ib_umem_notifiers;
> +		/*
> +		 * Lock-dep detects a false positive for mmap_sem vs.
> +		 * umem_mutex, due to not grasping downgrade_write correctly.
> +		 */
> +		lockdep_off();
> +		ret_val = mmu_notifier_register(&context->mn, mm);
> +		lockdep_on();
> +		if (ret_val) {
> +			pr_err("Failed to register mmu_notifier %d\n", ret_val);
> +			ret_val = -EBUSY;
> +			goto out_mutex;
> +		}
> +	}
> +
> +	up_read(&context->umem_mutex);
> +
> +	/*
> +	 * Note that doing an mmput can cause a notifier for the relevant mm.
> +	 * If the notifier is called while we hold the umem_mutex, this will
> +	 * cause a deadlock. Therefore, we release the reference only after we
> +	 * released the mutex.
> +	 */
> +	mmput(mm);
>  	return 0;
>  
> +out_mutex:
> +	up_read(&context->umem_mutex);
> +	vfree(umem->odp_data->dma_list);
>  out_page_list:
>  	vfree(umem->odp_data->page_list);
>  out_odp_data:
>  	kfree(umem->odp_data);
> +out_mm:
> +	mmput(mm);
>  	return ret_val;
>  }
>  
>  void ib_umem_odp_release(struct ib_umem *umem)
>  {
> +	struct ib_ucontext *context = umem->context;
> +
>  	/*
>  	 * Ensure that no more pages are mapped in the umem.
>  	 *
> @@ -95,6 +322,49 @@ void ib_umem_odp_release(struct ib_umem *umem)
>  	ib_umem_odp_unmap_dma_pages(umem, ib_umem_start(umem),
>  				    ib_umem_end(umem));
>  
> +	down_write(&context->umem_mutex);
> +	if (likely(ib_umem_start(umem) != ib_umem_end(umem)))
> +		rbt_ib_umem_remove(&umem->odp_data->interval_tree,
> +				   &context->umem_tree);
> +	context->odp_mrs_count--;
> +
> +	/*
> +	 * Downgrade the lock to a read lock. This ensures that the notifiers
> +	 * (who lock the mutex for reading) will be able to finish, and we
> +	 * will be able to enventually obtain the mmu notifiers SRCU. Note
> +	 * that since we are doing it atomically, no other user could register
> +	 * and unregister while we do the check.
> +	 */
> +	downgrade_write(&context->umem_mutex);
> +	if (!context->odp_mrs_count) {
> +		struct task_struct *owning_process = NULL;
> +		struct mm_struct *owning_mm        = NULL;
> +		owning_process = get_pid_task(context->tgid,
> +					      PIDTYPE_PID);
> +		if (owning_process == NULL)
> +			/*
> +			 * The process is already dead, notifier were removed
> +			 * already.
> +			 */
> +			goto out;
> +
> +		owning_mm = get_task_mm(owning_process);
> +		if (owning_mm == NULL)
> +			/*
> +			 * The process' mm is already dead, notifier were
> +			 * removed already.
> +			 */
> +			goto out_put_task;
> +		mmu_notifier_unregister(&context->mn, owning_mm);
> +
> +		mmput(owning_mm);
> +
> +out_put_task:
> +		put_task_struct(owning_process);
> +	}
> +out:
> +	up_read(&context->umem_mutex);
> +
>  	vfree(umem->odp_data->dma_list);
>  	vfree(umem->odp_data->page_list);
>  	kfree(umem);
> @@ -111,7 +381,8 @@ void ib_umem_odp_release(struct ib_umem *umem)
>   *               the sequence number is taken from
>   *               umem->odp_data->notifiers_seq.
>   *
> - * The function returns -EFAULT if the DMA mapping operation fails.
> + * The function returns -EFAULT if the DMA mapping operation fails. It returns
> + * -EAGAIN if a concurrent invalidation prevents us from updating the page. It
>   *
>   * The page is released via put_page even if the operation failed. For
>   * on-demand pinning, the page is released whenever it isn't stored in the
> @@ -120,6 +391,7 @@ void ib_umem_odp_release(struct ib_umem *umem)
>  static int ib_umem_odp_map_dma_single_page(
>  		struct ib_umem *umem,
>  		int page_index,
> +		u64 base_virt_addr,
>  		struct page *page,
>  		u64 access_mask,
>  		unsigned long current_seq)
> @@ -127,8 +399,18 @@ static int ib_umem_odp_map_dma_single_page(
>  	struct ib_device *dev = umem->context->device;
>  	dma_addr_t dma_addr;
>  	int stored_page = 0;
> +	int remove_existing_mapping = 0;
>  	int ret = 0;
>  	mutex_lock(&umem->odp_data->umem_mutex);
> +	/*
> +	 * Note: we avoid writing if seq is different from the initial seq, to
> +	 * handle case of a racing notifier. This check also allows us to bail
> +	 * early if we have a notifier running in parallel with us.
> +	 */
> +	if (ib_umem_mmu_notifier_retry(umem, current_seq)) {
> +		ret = -EAGAIN;
> +		goto out;
> +	}
>  	if (!(umem->odp_data->dma_list[page_index])) {
>  		dma_addr = ib_dma_map_page(dev,
>  					   page,
> @@ -146,14 +428,27 @@ static int ib_umem_odp_map_dma_single_page(
>  	} else {
>  		pr_err("error: got different pages in IB device and from get_user_pages. IB device page: %p, gup page: %p\n",
>  		       umem->odp_data->page_list[page_index], page);
> +		/* Better remove the mapping now, to prevent any further
> +		 * damage. */
> +		remove_existing_mapping = 1;
>  	}
>  
>  out:
>  	mutex_unlock(&umem->odp_data->umem_mutex);
>  
> -	if (!stored_page)
> +	/* On Demand Paging - avoid pinning the page */
> +	if (umem->context->invalidate_range || !stored_page)
>  		put_page(page);
>  
> +	if (remove_existing_mapping && umem->context->invalidate_range) {
> +		invalidate_page_trampoline(
> +			umem,
> +			base_virt_addr + (page_index * PAGE_SIZE),
> +			base_virt_addr + ((page_index+1)*PAGE_SIZE),
> +			NULL);
> +		ret = -EAGAIN;
> +	}
> +
>  	return ret;
>  }
>  
> @@ -166,6 +461,8 @@ out:
>   *
>   * Returns the number of pages mapped in success, negative error code
>   * for failure.
> + * An -EAGAIN error code is returned when a concurrent mmu notifier prevents
> + * the function from completing its task.
>   *
>   * @umem: the umem to map and pin
>   * @user_virt: the address from which we need to map.
> @@ -187,6 +484,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
>  	struct page       **local_page_list = NULL;
>  	u64 off;
>  	int j, k, ret = 0, start_idx, npages = 0;
> +	u64 base_virt_addr;
>  
>  	if (access_mask == 0)
>  		return -EINVAL;
> @@ -201,6 +499,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
>  
>  	off = user_virt & (~PAGE_MASK);
>  	user_virt = user_virt & PAGE_MASK;
> +	base_virt_addr = user_virt;
>  	bcnt += off; /* Charge for the first page offset as well. */
>  
>  	start_idx = (user_virt - ib_umem_start(umem)) >> PAGE_SHIFT;
> @@ -242,8 +541,8 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
>  		user_virt += npages << PAGE_SHIFT;
>  		for (j = 0; j < npages; ++j) {
>  			ret = ib_umem_odp_map_dma_single_page(
> -				umem, k, local_page_list[j], access_mask,
> -				current_seq);
> +				umem, k, base_virt_addr, local_page_list[j],
> +				access_mask, current_seq);
>  			if (ret < 0)
>  				break;
>  			k++;
> @@ -280,6 +579,11 @@ void ib_umem_odp_unmap_dma_pages(struct ib_umem *umem, u64 virt,
>  	struct ib_device *dev = umem->context->device;
>  	virt  = max_t(u64, virt,  ib_umem_start(umem));
>  	bound = min_t(u64, bound, ib_umem_end(umem));
> +	/* Note that during the run of this function, the
> +	 * notifiers_count of the MR is > 0, preventing any racing
> +	 * faults from completion. We might be racing with other
> +	 * invalidations, so we must make sure we free each page only
> +	 * once. */
>  	for (addr = virt; addr < bound; addr += (u64)umem->page_size) {
>  		idx = (addr - ib_umem_start(umem)) / PAGE_SIZE;
>  		mutex_lock(&umem->odp_data->umem_mutex);
> @@ -294,8 +598,21 @@ void ib_umem_odp_unmap_dma_pages(struct ib_umem *umem, u64 virt,
>  			ib_dma_unmap_page(dev, dma_addr, PAGE_SIZE,
>  					  DMA_BIDIRECTIONAL);
>  			if (umem->writable)
> -				set_page_dirty_lock(head_page);
> -			put_page(page);
> +				/*
> +				 * set_page_dirty prefers being called with
> +				 * the page lock. However, MMU notifiers are
> +				 * called sometimes with and sometimes without
> +				 * the lock. We rely on the umem_mutex instead
> +				 * to prevent other mmu notifiers from
> +				 * continuing and allowing the page mapping to
> +				 * be removed.
> +				 */
> +				set_page_dirty(head_page);
> +			/* on demand pinning support */
> +			if (!umem->context->invalidate_range)
> +				put_page(page);
> +			umem->odp_data->page_list[idx] = NULL;
> +			umem->odp_data->dma_list[idx] = 0;
>  		}
>  		mutex_unlock(&umem->odp_data->umem_mutex);
>  	}
> diff --git a/drivers/infiniband/core/umem_rbtree.c b/drivers/infiniband/core/umem_rbtree.c
> new file mode 100644
> index 0000000..727d788
> --- /dev/null
> +++ b/drivers/infiniband/core/umem_rbtree.c
> @@ -0,0 +1,94 @@
> +/*
> + * Copyright (c) 2014 Mellanox Technologies. All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses.  You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + *     Redistribution and use in source and binary forms, with or
> + *     without modification, are permitted provided that the following
> + *     conditions are met:
> + *
> + *      - Redistributions of source code must retain the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer.
> + *
> + *      - Redistributions in binary form must reproduce the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer in the documentation and/or other materials
> + *        provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interval_tree_generic.h>
> +#include <linux/sched.h>
> +#include <linux/gfp.h>
> +#include <rdma/ib_umem_odp.h>
> +
> +/*
> + * The ib_umem list keeps track of memory regions for which the HW
> + * device request to receive notification when the related memory
> + * mapping is changed.
> + *
> + * ib_umem_lock protects the list.
> + */
> +
> +static inline u64 node_start(struct umem_odp_node *n)
> +{
> +	struct ib_umem_odp *umem_odp =
> +			container_of(n, struct ib_umem_odp, interval_tree);
> +
> +	return ib_umem_start(umem_odp->umem);
> +}
> +
> +/* Note that the representation of the intervals in the interval tree
> + * considers the ending point as contained in the interval, while the
> + * function ib_umem_end returns the first address which is not contained
> + * in the umem.
> + */
> +static inline u64 node_last(struct umem_odp_node *n)
> +{
> +	struct ib_umem_odp *umem_odp =
> +			container_of(n, struct ib_umem_odp, interval_tree);
> +
> +	return ib_umem_end(umem_odp->umem) - 1;
> +}
> +
> +INTERVAL_TREE_DEFINE(struct umem_odp_node, rb, u64, __subtree_last,
> +		     node_start, node_last, , rbt_ib_umem)
> +
> +/* @last is not a part of the interval. See comment for function
> + * node_last.
> + */
> +int rbt_ib_umem_for_each_in_range(struct rb_root *root,
> +				  u64 start, u64 last,
> +				  umem_call_back cb,
> +				  void *cookie)
> +{
> +	int ret_val = 0;
> +	struct umem_odp_node *node;
> +	struct ib_umem_odp *umem;
> +
> +	if (unlikely(start == last))
> +		return ret_val;
> +
> +	for (node = rbt_ib_umem_iter_first(root, start, last - 1); node;
> +			node = rbt_ib_umem_iter_next(node, start, last - 1)) {
> +		umem = container_of(node, struct ib_umem_odp, interval_tree);
> +		ret_val = cb(umem->umem, start, last, cookie) || ret_val;
> +	}
> +
> +	return ret_val;
> +}
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index fe709ca..a81d0c7 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -289,6 +289,9 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
>  	struct ib_uverbs_get_context_resp resp;
>  	struct ib_udata                   udata;
>  	struct ib_device                 *ibdev = file->device->ib_dev;
> +#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> +	struct ib_device_attr		  dev_attr;
> +#endif
>  	struct ib_ucontext		 *ucontext;
>  	struct file			 *filp;
>  	int ret;
> @@ -331,6 +334,19 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
>  	rcu_read_unlock();
>  	ucontext->closing = 0;
>  
> +#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> +	ucontext->umem_tree = RB_ROOT;
> +	init_rwsem(&ucontext->umem_mutex);
> +	ucontext->odp_mrs_count = 0;
> +
> +	ret = ib_query_device(ibdev, &dev_attr);
> +	if (ret)
> +		goto err_free;
> +	if (!(dev_attr.device_cap_flags & IB_DEVICE_ON_DEMAND_PAGING))
> +		ucontext->invalidate_range = NULL;
> +
> +#endif
> +
>  	resp.num_comp_vectors = file->device->num_comp_vectors;
>  
>  	ret = get_unused_fd_flags(O_CLOEXEC);
> diff --git a/include/rdma/ib_umem_odp.h b/include/rdma/ib_umem_odp.h
> index 375ce28..9b93206 100644
> --- a/include/rdma/ib_umem_odp.h
> +++ b/include/rdma/ib_umem_odp.h
> @@ -34,6 +34,12 @@
>  #define IB_UMEM_ODP_H
>  
>  #include <rdma/ib_umem.h>
> +#include <linux/interval_tree.h>
> +
> +struct umem_odp_node {
> +	u64 __subtree_last;
> +	struct rb_node rb;
> +};
>  
>  struct ib_umem_odp {
>  	/*
> @@ -58,6 +64,14 @@ struct ib_umem_odp {
>  
>  	atomic_t		notifiers_seq;
>  	atomic_t		notifiers_count;
> +
> +	struct ib_umem		*umem;
> +
> +	/* Tree tracking */
> +	struct umem_odp_node	interval_tree;
> +
> +	struct completion	notifier_completion;
> +	int			dying;
>  };
>  
>  #ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> @@ -85,6 +99,48 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 start_offset, u64 bcnt,
>  void ib_umem_odp_unmap_dma_pages(struct ib_umem *umem, u64 start_offset,
>  				 u64 bound);
>  
> +void rbt_ib_umem_insert(struct umem_odp_node *node, struct rb_root *root);
> +void rbt_ib_umem_remove(struct umem_odp_node *node, struct rb_root *root);
> +typedef int (*umem_call_back)(struct ib_umem *item, u64 start, u64 end,
> +			      void *cookie);
> +/*
> + * Call the callback on each ib_umem in the range. Returns the logical or of
> + * the return values of the functions called.
> + */
> +int rbt_ib_umem_for_each_in_range(struct rb_root *root, u64 start, u64 end,
> +				  umem_call_back cb, void *cookie);
> +
> +struct umem_odp_node *rbt_ib_umem_iter_first(struct rb_root *root,
> +					     u64 start, u64 last);
> +struct umem_odp_node *rbt_ib_umem_iter_next(struct umem_odp_node *node,
> +					    u64 start, u64 last);
> +
> +static inline int ib_umem_mmu_notifier_retry(struct ib_umem *item,
> +					     unsigned long mmu_seq)
> +{
> +	/*
> +	 * This code is strongly based on the KVM code from
> +	 * mmu_notifier_retry. Should be called with
> +	 * item->odp_data->umem_mutex locked.
> +	 */
> +	if (unlikely(atomic_read(&item->odp_data->notifiers_count)))
> +		return 1;
> +	/*
> +	 * Ensure the read of mmu_notifier_count happens before the read
> +	 * of mmu_notifier_seq.  This interacts with the smp_wmb() in
> +	 * mmu_notifier_invalidate_range_end to make sure that the caller
> +	 * either sees the old (non-zero) value of mmu_notifier_count or
> +	 * the new (incremented) value of mmu_notifier_seq.
> +	 */
> +	smp_rmb();
> +	if (atomic_read(&item->odp_data->notifiers_seq) != mmu_seq)
> +		return 1;
> +	return 0;
> +}
> +
> +void ib_umem_notifier_start_account(struct ib_umem *item);
> +void ib_umem_notifier_end_account(struct ib_umem *item);
> +
>  #else /* CONFIG_INFINIBAND_ON_DEMAND_PAGING */
>  
>  static inline int ib_umem_odp_get(struct ib_ucontext *context,
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 6d7f75e..54df9a8 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -51,6 +51,7 @@
>  #include <uapi/linux/if_ether.h>
>  
>  #include <linux/atomic.h>
> +#include <linux/mmu_notifier.h>
>  #include <asm/uaccess.h>
>  
>  extern struct workqueue_struct *ib_wq;
> @@ -1142,6 +1143,8 @@ struct ib_fmr_attr {
>  	u8	page_shift;
>  };
>  
> +struct ib_umem;
> +
>  struct ib_ucontext {
>  	struct ib_device       *device;
>  	struct list_head	pd_list;
> @@ -1157,6 +1160,19 @@ struct ib_ucontext {
>  
>  	/* For ODP support: */
>  	struct pid             *tgid;
> +#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> +	struct rb_root      umem_tree;
> +	/*
> +	 * Protects .umem_rbroot and tree, as well as odp_mrs_count and
> +	 * mmu notifiers registration.
> +	 */
> +	struct rw_semaphore	umem_mutex;
> +	void (*invalidate_range)(struct ib_umem *umem,
> +				 unsigned long start, unsigned long end);
> +
> +	struct mmu_notifier	mn;
> +	int                     odp_mrs_count;
> +#endif
>  };
>  
>  struct ib_uobject {
> -- 
> 1.7.11.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Shachar Raindel]

Hi,

> -----Original Message-----
> From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> Sent: Thursday, September 04, 2014 11:25 PM
> To: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Shachar Raindel; Sagi Grimberg
> Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support for
> MMU notifiers regarding on demand paging regions
> 
> > * Add an interval tree implementation for ODP umems. Create an
> interval tree
> >   for each ucontext (including a count of the number of ODP MRs in
> this
> >   context, mutex, etc.), and register ODP umems in the interval tree.
> > * Add MMU notifiers handling functions, using the interval tree to
> notify only
> >   the relevant umems and underlying MRs.
> > * Register to receive MMU notifier events from the MM subsystem upon
> ODP MR
> >   registration (and unregister accordingly).
> > * Add a completion object to synchronize the destruction of ODP umems.
> > * Add mechanism to abort page faults when there's a concurrent
> invalidation.
> >
> > The way we synchronize between concurrent invalidations and page
> faults is by
> > keeping a counter of currently running invalidations, and a sequence
> number
> > that is incremented whenever an invalidation is caught. The page fault
> code
> > checks the counter and also verifies that the sequence number hasn't
> > progressed before it updates the umem's page tables. This is similar
> to what
> > the kvm module does.
> >
> > There's currently a rare race in the code when registering a umem in
> the
> > middle of an ongoing notifier. The proper fix is to either serialize
> the
> > insertion to our umem tree with mm_lock_all or use a ucontext wide
> running
> > notifiers count for retries decision. Either is ugly and can lead to
> some sort
> > of starvation. The current workaround is ugly as well - now the user
> can end
> > up with mapped addresses that are not in the user's address space
> (although it
> > is highly unlikely).
> 
> I have been trying to wrap my head around this comment. I am totaly
> unfamiliar
> with RDMA code, but from quick look at it when registering umem you take
> the
> mmap_sem in read mode so any munmap from userspace would be serialize.
> Really
> the worst that can happen is that a umem pointing to a mmaped file that
> is
> concurently truncated but even then the address is still valid, but it
> should
> result in a SIGBUS which here is obviously harder to report (again dunno
> how
> RDMA works).
> 
> So am i missing something ?
> 

Sadly, taking mmap_sem in read-only mode does not prevent all possible invalidations from happening.
For example, a call to madvise requesting MADVISE_DONTNEED will lock the mmap_sem for reading only, allowing a notifier to run in parallel to the MR registration As a result, the following sequence of events could happen:

Thread 1:                       |   Thread 2
--------------------------------+-------------------------
madvise                         |
down_read(mmap_sem)             |
notifier_start                  |
                                |   down_read(mmap_sem)
                                |   register_mr
notifier_end                    |
reduce_mr_notifiers_count       |

The end result of this sequence is an mr with running notifiers count of -1, which is bad.
The current workaround is to avoid decreasing the notifiers count if it is zero, which can cause other issues.
The proper fix would be to prevent notifiers from running in parallel to registration. For this, taking mmap_sem in write mode might be sufficient, but we are not sure about this.
We will be happy to hear additional input on this subject, to make sure we got it covered properly.

Thanks,
--Shachar
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Jerome Glisse]

On Sun, Sep 07, 2014 at 02:35:59PM +0000, Shachar Raindel wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > Sent: Thursday, September 04, 2014 11:25 PM
> > To: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: Shachar Raindel; Sagi Grimberg
> > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support for
> > MMU notifiers regarding on demand paging regions
> > 
> > > * Add an interval tree implementation for ODP umems. Create an
> > interval tree
> > >   for each ucontext (including a count of the number of ODP MRs in
> > this
> > >   context, mutex, etc.), and register ODP umems in the interval tree.
> > > * Add MMU notifiers handling functions, using the interval tree to
> > notify only
> > >   the relevant umems and underlying MRs.
> > > * Register to receive MMU notifier events from the MM subsystem upon
> > ODP MR
> > >   registration (and unregister accordingly).
> > > * Add a completion object to synchronize the destruction of ODP umems.
> > > * Add mechanism to abort page faults when there's a concurrent
> > invalidation.
> > >
> > > The way we synchronize between concurrent invalidations and page
> > faults is by
> > > keeping a counter of currently running invalidations, and a sequence
> > number
> > > that is incremented whenever an invalidation is caught. The page fault
> > code
> > > checks the counter and also verifies that the sequence number hasn't
> > > progressed before it updates the umem's page tables. This is similar
> > to what
> > > the kvm module does.
> > >
> > > There's currently a rare race in the code when registering a umem in
> > the
> > > middle of an ongoing notifier. The proper fix is to either serialize
> > the
> > > insertion to our umem tree with mm_lock_all or use a ucontext wide
> > running
> > > notifiers count for retries decision. Either is ugly and can lead to
> > some sort
> > > of starvation. The current workaround is ugly as well - now the user
> > can end
> > > up with mapped addresses that are not in the user's address space
> > (although it
> > > is highly unlikely).
> > 
> > I have been trying to wrap my head around this comment. I am totaly
> > unfamiliar
> > with RDMA code, but from quick look at it when registering umem you take
> > the
> > mmap_sem in read mode so any munmap from userspace would be serialize.
> > Really
> > the worst that can happen is that a umem pointing to a mmaped file that
> > is
> > concurently truncated but even then the address is still valid, but it
> > should
> > result in a SIGBUS which here is obviously harder to report (again dunno
> > how
> > RDMA works).
> > 
> > So am i missing something ?
> > 
> 
> Sadly, taking mmap_sem in read-only mode does not prevent all possible invalidations from happening.
> For example, a call to madvise requesting MADVISE_DONTNEED will lock the mmap_sem for reading only, allowing a notifier to run in parallel to the MR registration As a result, the following sequence of events could happen:
> 
> Thread 1:                       |   Thread 2
> --------------------------------+-------------------------
> madvise                         |
> down_read(mmap_sem)             |
> notifier_start                  |
>                                 |   down_read(mmap_sem)
>                                 |   register_mr
> notifier_end                    |
> reduce_mr_notifiers_count       |
> 
> The end result of this sequence is an mr with running notifiers count of -1, which is bad.
> The current workaround is to avoid decreasing the notifiers count if it is zero, which can cause other issues.
> The proper fix would be to prevent notifiers from running in parallel to registration. For this, taking mmap_sem in write mode might be sufficient, but we are not sure about this.
> We will be happy to hear additional input on this subject, to make sure we got it covered properly.

So in HMM i solve this by having a struct allocated in the start range callback
and the end range callback just ignore things when it can not find the matching
struct.

That being said when registering the mmu_notifier you need 2 things, first you
need a pin on the mm (either mm is current ie current->mm or you took a reference
on it). Second you need to that the mmap smemaphore in write mode so that
no concurrent mmap/munmap/madvise can happen. By doing that you protect yourself
from concurrent range_start/range_end that can happen and that does matter.
The only concurrent range_start/end that can happen is through file invalidation
which is fine because subsequent page fault will go through the file layer and
bring back page or return error (if file was truncated for instance).

So as long as you hold the mmap_sem in write mode you should not worry about
concurrent range_start/range_end (well they might happen but only for file
backed vma).

Given that you face the same issue as i have with the range_start/range_end i
will stich up a patch to make it easier to track those.

Cheers,
Jérôme


> 
> Thanks,
> --Shachar
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Shachar Raindel]

> -----Original Message-----
> From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> Sent: Tuesday, September 09, 2014 6:37 PM
> To: Shachar Raindel
> Cc: 1404377069-20585-1-git-send-email-haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org; Haggai Eran;
> linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Jerome Glisse; Sagi Grimberg
> Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support for
> MMU notifiers regarding on demand paging regions
> 
> On Sun, Sep 07, 2014 at 02:35:59PM +0000, Shachar Raindel wrote:
> > Hi,
> >
> > > -----Original Message-----
> > > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > > Sent: Thursday, September 04, 2014 11:25 PM
> > > To: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > Cc: Shachar Raindel; Sagi Grimberg
> > > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support
> for
> > > MMU notifiers regarding on demand paging regions
> > >
> > > > * Add an interval tree implementation for ODP umems. Create an
> > > interval tree
> > > >   for each ucontext (including a count of the number of ODP MRs in
> > > this
> > > >   context, mutex, etc.), and register ODP umems in the interval
> tree.
> > > > * Add MMU notifiers handling functions, using the interval tree to
> > > notify only
> > > >   the relevant umems and underlying MRs.
> > > > * Register to receive MMU notifier events from the MM subsystem
> upon
> > > ODP MR
> > > >   registration (and unregister accordingly).
> > > > * Add a completion object to synchronize the destruction of ODP
> umems.
> > > > * Add mechanism to abort page faults when there's a concurrent
> > > invalidation.
> > > >
> > > > The way we synchronize between concurrent invalidations and page
> > > faults is by
> > > > keeping a counter of currently running invalidations, and a
> sequence
> > > number
> > > > that is incremented whenever an invalidation is caught. The page
> fault
> > > code
> > > > checks the counter and also verifies that the sequence number
> hasn't
> > > > progressed before it updates the umem's page tables. This is
> similar
> > > to what
> > > > the kvm module does.
> > > >
> > > > There's currently a rare race in the code when registering a umem
> in
> > > the
> > > > middle of an ongoing notifier. The proper fix is to either
> serialize
> > > the
> > > > insertion to our umem tree with mm_lock_all or use a ucontext wide
> > > running
> > > > notifiers count for retries decision. Either is ugly and can lead
> to
> > > some sort
> > > > of starvation. The current workaround is ugly as well - now the
> user
> > > can end
> > > > up with mapped addresses that are not in the user's address space
> > > (although it
> > > > is highly unlikely).
> > >
> > > I have been trying to wrap my head around this comment. I am totaly
> > > unfamiliar
> > > with RDMA code, but from quick look at it when registering umem you
> take
> > > the
> > > mmap_sem in read mode so any munmap from userspace would be
> serialize.
> > > Really
> > > the worst that can happen is that a umem pointing to a mmaped file
> that
> > > is
> > > concurently truncated but even then the address is still valid, but
> it
> > > should
> > > result in a SIGBUS which here is obviously harder to report (again
> dunno
> > > how
> > > RDMA works).
> > >
> > > So am i missing something ?
> > >
> >
> > Sadly, taking mmap_sem in read-only mode does not prevent all possible
> invalidations from happening.
> > For example, a call to madvise requesting MADVISE_DONTNEED will lock
> the mmap_sem for reading only, allowing a notifier to run in parallel to
> the MR registration As a result, the following sequence of events could
> happen:
> >
> > Thread 1:                       |   Thread 2
> > --------------------------------+-------------------------
> > madvise                         |
> > down_read(mmap_sem)             |
> > notifier_start                  |
> >                                 |   down_read(mmap_sem)
> >                                 |   register_mr
> > notifier_end                    |
> > reduce_mr_notifiers_count       |
> >
> > The end result of this sequence is an mr with running notifiers count
> of -1, which is bad.
> > The current workaround is to avoid decreasing the notifiers count if
> it is zero, which can cause other issues.
> > The proper fix would be to prevent notifiers from running in parallel
> to registration. For this, taking mmap_sem in write mode might be
> sufficient, but we are not sure about this.
> > We will be happy to hear additional input on this subject, to make
> sure we got it covered properly.
> 
> So in HMM i solve this by having a struct allocated in the start range
> callback
> and the end range callback just ignore things when it can not find the
> matching
> struct.

This kind of mechanism sounds like it has a bigger risk for deadlocking
the system, causing an OOM kill without a real need or significantly 
slowing down the system.
If you are doing non-atomic memory allocations, you can deadlock the
system by requesting memory in the swapper flow.
Even if you are doing atomic memory allocations, you need to handle the
case of failing allocation, the solution to which is unclear to me.
If you are using a pre-allocated pool, what are you doing when you run
out of available entries in the pool? If you are blocking until some
entries free up, what guarantees you that this will not cause a deadlock?

> 
> That being said when registering the mmu_notifier you need 2 things,
> first you
> need a pin on the mm (either mm is current ie current->mm or you took a
> reference
> on it). Second you need to that the mmap smemaphore in write mode so
> that
> no concurrent mmap/munmap/madvise can happen. By doing that you protect
> yourself
> from concurrent range_start/range_end that can happen and that does
> matter.
> The only concurrent range_start/end that can happen is through file
> invalidation
> which is fine because subsequent page fault will go through the file
> layer and
> bring back page or return error (if file was truncated for instance).

Sadly, this is not sufficient for our use case. We are registering
a single MMU notifier handler, and broadcast the notifications to
all relevant listeners, which are stored in an interval tree.

Each listener represents a section of the address space that has been
exposed to the network. Such implementation allows us to limit the impact
of invalidations, and only block racing page faults to the affected areas.

Each of the listeners maintain a counter of the number of invalidate_range
notifications that are currently affecting it. The counter is increased
for each invalidate_range_start callback received, and decrease for each
invalidate_range_end callback received. If we add a listener to the
interval tree after the invalidate_range_start callback happened, but
before the invalidate_range_end callback happened, it will decrease the
counter, reaching negative numbers and breaking the logic.

The mmu_notifiers registration code avoid such issues by taking all
relevant locks on the MM. This effectively blocks all possible notifiers
from happening when registering a new notifier. Sadly, this function is
not exported for modules to use it.

Our options at the moment are:
- Use a tracking mechanism similar to what HMM uses, alongside the
  challenges involved in allocating memory from notifiers

- Use a per-process counter for invalidations, causing a possible
  performance degradation. This can possibly be used as a fallback to the
  first option (i.e. have a pool of X notifier identifiers, once it is
  full, increase/decrease a per-MM counter)

- Export the mm_take_all_locks function for modules. This will allow us
  to lock the MM when adding a new listener.

> 
> So as long as you hold the mmap_sem in write mode you should not worry
> about
> concurrent range_start/range_end (well they might happen but only for
> file
> backed vma).
> 

Sadly, the mmap_sem is not enough to protect us :(.

> Given that you face the same issue as i have with the
> range_start/range_end i
> will stich up a patch to make it easier to track those.
> 

That would be nice, especially if we could easily integrate it into our
code and reduce the code size.

> Cheers,
> Jérôme
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Jerome Glisse]

On Wed, Sep 10, 2014 at 09:00:36AM +0000, Shachar Raindel wrote:
> 
> 
> > -----Original Message-----
> > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > Sent: Tuesday, September 09, 2014 6:37 PM
> > To: Shachar Raindel
> > Cc: 1404377069-20585-1-git-send-email-haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org; Haggai Eran;
> > linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Jerome Glisse; Sagi Grimberg
> > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support for
> > MMU notifiers regarding on demand paging regions
> > 
> > On Sun, Sep 07, 2014 at 02:35:59PM +0000, Shachar Raindel wrote:
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > > > Sent: Thursday, September 04, 2014 11:25 PM
> > > > To: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > Cc: Shachar Raindel; Sagi Grimberg
> > > > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support
> > for
> > > > MMU notifiers regarding on demand paging regions
> > > >
> > > > > * Add an interval tree implementation for ODP umems. Create an
> > > > interval tree
> > > > >   for each ucontext (including a count of the number of ODP MRs in
> > > > this
> > > > >   context, mutex, etc.), and register ODP umems in the interval
> > tree.
> > > > > * Add MMU notifiers handling functions, using the interval tree to
> > > > notify only
> > > > >   the relevant umems and underlying MRs.
> > > > > * Register to receive MMU notifier events from the MM subsystem
> > upon
> > > > ODP MR
> > > > >   registration (and unregister accordingly).
> > > > > * Add a completion object to synchronize the destruction of ODP
> > umems.
> > > > > * Add mechanism to abort page faults when there's a concurrent
> > > > invalidation.
> > > > >
> > > > > The way we synchronize between concurrent invalidations and page
> > > > faults is by
> > > > > keeping a counter of currently running invalidations, and a
> > sequence
> > > > number
> > > > > that is incremented whenever an invalidation is caught. The page
> > fault
> > > > code
> > > > > checks the counter and also verifies that the sequence number
> > hasn't
> > > > > progressed before it updates the umem's page tables. This is
> > similar
> > > > to what
> > > > > the kvm module does.
> > > > >
> > > > > There's currently a rare race in the code when registering a umem
> > in
> > > > the
> > > > > middle of an ongoing notifier. The proper fix is to either
> > serialize
> > > > the
> > > > > insertion to our umem tree with mm_lock_all or use a ucontext wide
> > > > running
> > > > > notifiers count for retries decision. Either is ugly and can lead
> > to
> > > > some sort
> > > > > of starvation. The current workaround is ugly as well - now the
> > user
> > > > can end
> > > > > up with mapped addresses that are not in the user's address space
> > > > (although it
> > > > > is highly unlikely).
> > > >
> > > > I have been trying to wrap my head around this comment. I am totaly
> > > > unfamiliar
> > > > with RDMA code, but from quick look at it when registering umem you
> > take
> > > > the
> > > > mmap_sem in read mode so any munmap from userspace would be
> > serialize.
> > > > Really
> > > > the worst that can happen is that a umem pointing to a mmaped file
> > that
> > > > is
> > > > concurently truncated but even then the address is still valid, but
> > it
> > > > should
> > > > result in a SIGBUS which here is obviously harder to report (again
> > dunno
> > > > how
> > > > RDMA works).
> > > >
> > > > So am i missing something ?
> > > >
> > >
> > > Sadly, taking mmap_sem in read-only mode does not prevent all possible
> > invalidations from happening.
> > > For example, a call to madvise requesting MADVISE_DONTNEED will lock
> > the mmap_sem for reading only, allowing a notifier to run in parallel to
> > the MR registration As a result, the following sequence of events could
> > happen:
> > >
> > > Thread 1:                       |   Thread 2
> > > --------------------------------+-------------------------
> > > madvise                         |
> > > down_read(mmap_sem)             |
> > > notifier_start                  |
> > >                                 |   down_read(mmap_sem)
> > >                                 |   register_mr
> > > notifier_end                    |
> > > reduce_mr_notifiers_count       |
> > >
> > > The end result of this sequence is an mr with running notifiers count
> > of -1, which is bad.
> > > The current workaround is to avoid decreasing the notifiers count if
> > it is zero, which can cause other issues.
> > > The proper fix would be to prevent notifiers from running in parallel
> > to registration. For this, taking mmap_sem in write mode might be
> > sufficient, but we are not sure about this.
> > > We will be happy to hear additional input on this subject, to make
> > sure we got it covered properly.
> > 
> > So in HMM i solve this by having a struct allocated in the start range
> > callback
> > and the end range callback just ignore things when it can not find the
> > matching
> > struct.
> 
> This kind of mechanism sounds like it has a bigger risk for deadlocking
> the system, causing an OOM kill without a real need or significantly 
> slowing down the system.
> If you are doing non-atomic memory allocations, you can deadlock the
> system by requesting memory in the swapper flow.
> Even if you are doing atomic memory allocations, you need to handle the
> case of failing allocation, the solution to which is unclear to me.
> If you are using a pre-allocated pool, what are you doing when you run
> out of available entries in the pool? If you are blocking until some
> entries free up, what guarantees you that this will not cause a deadlock?

So i am using a fixed pool and when it runs out it block in start callback
until one is freed. But as i said i have a patch to use the stack that will
solve this and avoid a pool.

> 
> > 
> > That being said when registering the mmu_notifier you need 2 things,
> > first you
> > need a pin on the mm (either mm is current ie current->mm or you took a
> > reference
> > on it). Second you need to that the mmap smemaphore in write mode so
> > that
> > no concurrent mmap/munmap/madvise can happen. By doing that you protect
> > yourself
> > from concurrent range_start/range_end that can happen and that does
> > matter.
> > The only concurrent range_start/end that can happen is through file
> > invalidation
> > which is fine because subsequent page fault will go through the file
> > layer and
> > bring back page or return error (if file was truncated for instance).
> 
> Sadly, this is not sufficient for our use case. We are registering
> a single MMU notifier handler, and broadcast the notifications to
> all relevant listeners, which are stored in an interval tree.
>
> Each listener represents a section of the address space that has been
> exposed to the network. Such implementation allows us to limit the impact
> of invalidations, and only block racing page faults to the affected areas.
> 
> Each of the listeners maintain a counter of the number of invalidate_range
> notifications that are currently affecting it. The counter is increased
> for each invalidate_range_start callback received, and decrease for each
> invalidate_range_end callback received. If we add a listener to the
> interval tree after the invalidate_range_start callback happened, but
> before the invalidate_range_end callback happened, it will decrease the
> counter, reaching negative numbers and breaking the logic.
> 
> The mmu_notifiers registration code avoid such issues by taking all
> relevant locks on the MM. This effectively blocks all possible notifiers
> from happening when registering a new notifier. Sadly, this function is
> not exported for modules to use it.
> 
> Our options at the moment are:
> - Use a tracking mechanism similar to what HMM uses, alongside the
>   challenges involved in allocating memory from notifiers
> 
> - Use a per-process counter for invalidations, causing a possible
>   performance degradation. This can possibly be used as a fallback to the
>   first option (i.e. have a pool of X notifier identifiers, once it is
>   full, increase/decrease a per-MM counter)
> 
> - Export the mm_take_all_locks function for modules. This will allow us
>   to lock the MM when adding a new listener.

I was not clear enough, you need to take the mmap_sem in write mode accross
mmu_notifier_register(). This is only to partialy solve your issue that if
a mmu_notifier is already register for the mm you are trying to registering
against then there is a chance for you to be inside an active range_start/
range_end section which would lead to invalid counter inside your tracking
structure. But, sadly, taking mmap_sem in write mode is not enough as file
invalidation might still happen concurrently so you will need to make sure
you invalidation counters does not go negative but from page fault point of
view you will be fine because the page fault will synchronize through the
pagecache. So scenario (A and B are to anonymous overlapping address range) :

  APP_TOTO_RDMA_THREAD           |  APP_TOTO_SOME_OTHER_THREAD
                                 |  mmu_notifier_invalidate_range_start(A)
  odp_register()                 |
    down_read(mmap_sem)          |
    mmu_notifier_register()      |
    up_read(mmap_sem)            |
  odp_add_new_region(B)          |
  odp_page_fault(B)              |
    down_read(mmap_sem)          |
    ...                          |
    up_read(mmap_sem)            |
                                 |  mmu_notifier_invalidate_range_end(A)

The odp_page_fault(B) might see invalid cpu page table but you have no idea
about it because you registered after the range_start(). But if you take the
mmap_sem in write mode then the only case where you might still have this
scenario is if A and B are range of a file backed vma and that the file is
undergoing some change (most likely truncation). But the file case is fine
because the odp_page_fault() will go through the pagecache which is properly
synchronize against the current range invalidation.


Now for the the general case outside of mmu_notifier_register() HMM also track
active invalidation range to avoid page faulting into those range as we can not
trust the cpu page table for as long as the range invalidation is on going.

> > 
> > So as long as you hold the mmap_sem in write mode you should not worry
> > about
> > concurrent range_start/range_end (well they might happen but only for
> > file
> > backed vma).
> > 
> 
> Sadly, the mmap_sem is not enough to protect us :(.

This is enough like i explain above, but i am only talking about the mmu
notifier registration. For the general case once you register you only
need to take the mmap_sem in read mode during page fault.

> > Given that you face the same issue as i have with the
> > range_start/range_end i
> > will stich up a patch to make it easier to track those.
> > 
> 
> That would be nice, especially if we could easily integrate it into our
> code and reduce the code size.

Yes it's a "small modification" to the mmu_notifier api, i have been side
tracked on other thing. But i will have it soon.

> 
> > Cheers,
> > Jérôme
> > 
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Shachar Raindel]

> -----Original Message-----
> From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> Sent: Wednesday, September 10, 2014 11:15 PM
> To: Shachar Raindel
> Cc: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Sagi Grimberg
> Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support for
> MMU notifiers regarding on demand paging regions
> 
> On Wed, Sep 10, 2014 at 09:00:36AM +0000, Shachar Raindel wrote:
> >
> >
> > > -----Original Message-----
> > > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > > Sent: Tuesday, September 09, 2014 6:37 PM
> > > To: Shachar Raindel
> > > Cc: 1404377069-20585-1-git-send-email-haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org; Haggai
> Eran;
> > > linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Jerome Glisse; Sagi Grimberg
> > > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support
> for
> > > MMU notifiers regarding on demand paging regions
> > >
> > > On Sun, Sep 07, 2014 at 02:35:59PM +0000, Shachar Raindel wrote:
> > > > Hi,
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > > > > Sent: Thursday, September 04, 2014 11:25 PM
> > > > > To: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > > Cc: Shachar Raindel; Sagi Grimberg
> > > > > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement
> support
> > > for
> > > > > MMU notifiers regarding on demand paging regions
> > > > >

<SNIP>

> > > >
> > > > Sadly, taking mmap_sem in read-only mode does not prevent all
> possible
> > > invalidations from happening.
> > > > For example, a call to madvise requesting MADVISE_DONTNEED will
> lock
> > > the mmap_sem for reading only, allowing a notifier to run in
> parallel to
> > > the MR registration As a result, the following sequence of events
> could
> > > happen:
> > > >
> > > > Thread 1:                       |   Thread 2
> > > > --------------------------------+-------------------------
> > > > madvise                         |
> > > > down_read(mmap_sem)             |
> > > > notifier_start                  |
> > > >                                 |   down_read(mmap_sem)
> > > >                                 |   register_mr
> > > > notifier_end                    |
> > > > reduce_mr_notifiers_count       |
> > > >
> > > > The end result of this sequence is an mr with running notifiers
> count
> > > of -1, which is bad.
> > > > The current workaround is to avoid decreasing the notifiers count
> if
> > > it is zero, which can cause other issues.
> > > > The proper fix would be to prevent notifiers from running in
> parallel
> > > to registration. For this, taking mmap_sem in write mode might be
> > > sufficient, but we are not sure about this.
> > > > We will be happy to hear additional input on this subject, to make
> > > sure we got it covered properly.
> > >
> > > So in HMM i solve this by having a struct allocated in the start
> range
> > > callback
> > > and the end range callback just ignore things when it can not find
> the
> > > matching
> > > struct.
> >
> > This kind of mechanism sounds like it has a bigger risk for
> deadlocking
> > the system, causing an OOM kill without a real need or significantly
> > slowing down the system.
> > If you are doing non-atomic memory allocations, you can deadlock the
> > system by requesting memory in the swapper flow.
> > Even if you are doing atomic memory allocations, you need to handle
> the
> > case of failing allocation, the solution to which is unclear to me.
> > If you are using a pre-allocated pool, what are you doing when you run
> > out of available entries in the pool? If you are blocking until some
> > entries free up, what guarantees you that this will not cause a
> deadlock?
> 
> So i am using a fixed pool and when it runs out it block in start
> callback
> until one is freed. 

This sounds scary. You now create a possible locking dependency between
two code flows which could have run in parallel. This can cause circular
locking bugs, from code which functioned properly until now. For example,
assume code with a single lock, and the following code paths:

Code 1:
notify_start()
lock()
unlock()
notify_end()

Code 2:
lock()
notify_start()
... (no locking)
notify_end()
unlock()



This code can now create the following deadlock:

Thread 1:        | Thread 2:
-----------------+-----------------------------------
notify_start()   |
                 | lock()
lock() - blocking|
                 | notify_start() - blocking for slot




> But as i said i have a patch to use the stack that
> will
> solve this and avoid a pool.

How are you allocating from the stack an entry which you need to keep alive
until another function is called? You can't allocate the entry on the
notify_start stack, so you must do this in all of the call points to the
mmu_notifiers. Given the notifiers listener subscription pattern, this seems
like something which is not practical.

 
> 
> >
> > >
> > > That being said when registering the mmu_notifier you need 2 things,
> > > first you
> > > need a pin on the mm (either mm is current ie current->mm or you
> took a
> > > reference
> > > on it). Second you need to that the mmap smemaphore in write mode so
> > > that
> > > no concurrent mmap/munmap/madvise can happen. By doing that you
> protect
> > > yourself
> > > from concurrent range_start/range_end that can happen and that does
> > > matter.
> > > The only concurrent range_start/end that can happen is through file
> > > invalidation
> > > which is fine because subsequent page fault will go through the file
> > > layer and
> > > bring back page or return error (if file was truncated for
> instance).
> >
> > Sadly, this is not sufficient for our use case. We are registering
> > a single MMU notifier handler, and broadcast the notifications to
> > all relevant listeners, which are stored in an interval tree.
> >
> > Each listener represents a section of the address space that has been
> > exposed to the network. Such implementation allows us to limit the
> impact
> > of invalidations, and only block racing page faults to the affected
> areas.
> >
> > Each of the listeners maintain a counter of the number of
> invalidate_range
> > notifications that are currently affecting it. The counter is
> increased
> > for each invalidate_range_start callback received, and decrease for
> each
> > invalidate_range_end callback received. If we add a listener to the
> > interval tree after the invalidate_range_start callback happened, but
> > before the invalidate_range_end callback happened, it will decrease
> the
> > counter, reaching negative numbers and breaking the logic.
> >
> > The mmu_notifiers registration code avoid such issues by taking all
> > relevant locks on the MM. This effectively blocks all possible
> notifiers
> > from happening when registering a new notifier. Sadly, this function
> is
> > not exported for modules to use it.
> >
> > Our options at the moment are:
> > - Use a tracking mechanism similar to what HMM uses, alongside the
> >   challenges involved in allocating memory from notifiers
> >
> > - Use a per-process counter for invalidations, causing a possible
> >   performance degradation. This can possibly be used as a fallback to
> the
> >   first option (i.e. have a pool of X notifier identifiers, once it is
> >   full, increase/decrease a per-MM counter)
> >
> > - Export the mm_take_all_locks function for modules. This will allow
> us
> >   to lock the MM when adding a new listener.
> 
> I was not clear enough, you need to take the mmap_sem in write mode
> accross
> mmu_notifier_register(). This is only to partialy solve your issue that
> if
> a mmu_notifier is already register for the mm you are trying to
> registering
> against then there is a chance for you to be inside an active
> range_start/
> range_end section which would lead to invalid counter inside your
> tracking
> structure. But, sadly, taking mmap_sem in write mode is not enough as
> file
> invalidation might still happen concurrently so you will need to make
> sure
> you invalidation counters does not go negative but from page fault point
> of
> view you will be fine because the page fault will synchronize through
> the
> pagecache. So scenario (A and B are to anonymous overlapping address
> range) :
> 
>   APP_TOTO_RDMA_THREAD           |  APP_TOTO_SOME_OTHER_THREAD
>                                  |  mmu_notifier_invalidate_range_start(A)
>   odp_register()                 |
>     down_read(mmap_sem)          |
>     mmu_notifier_register()      |
>     up_read(mmap_sem)            |
>   odp_add_new_region(B)          |
>   odp_page_fault(B)              |
>     down_read(mmap_sem)          |
>     ...                          |
>     up_read(mmap_sem)            |
>                                  |  mmu_notifier_invalidate_range_end(A)
> 
> The odp_page_fault(B) might see invalid cpu page table but you have no
> idea
> about it because you registered after the range_start(). But if you take
> the
> mmap_sem in write mode then the only case where you might still have
> this
> scenario is if A and B are range of a file backed vma and that the file
> is
> undergoing some change (most likely truncation). But the file case is
> fine
> because the odp_page_fault() will go through the pagecache which is
> properly
> synchronize against the current range invalidation.

Specifically, if you call mmu_notifier_register you are OK and the above
scenario will not happen. You are supposed to hold mmap_sem for writing,
and mmu_notifier_register is calling mm_take_all_locks, which guarantees
no racing notifier during the registration step.

However, we want to dynamically add sub-notifiers in our code. Each will
get notified only about invalidations touching a specific sub-sections of
the address space. To avoid providing unneeded notifications, we use an
interval tree that filters only the needed notifications.
When adding entries to the interval tree, we cannot lock the mm to prevent
any racing invalidations. As such, we might end up in a case where a newly
registered memory region will get a "notify_end" call without the relevant
"notify_start". Even if we prevent the value from dropping below zero, it
means we can cause data corruption. For example, if we have another
notifier running after the MR registers, which is due to munmap, but we get
first the notify_end of the previous notifier for which we didn't see the
notify_start.

The solution we are coming up with now is using a global counter of running
invalidations for new regions allocated. When the global counter is at zero,
we can safely switch to the region local invalidations counter.


> 
> 
> Now for the the general case outside of mmu_notifier_register() HMM also
> track
> active invalidation range to avoid page faulting into those range as we
> can not
> trust the cpu page table for as long as the range invalidation is on
> going.
> 
> > >
> > > So as long as you hold the mmap_sem in write mode you should not
> worry
> > > about
> > > concurrent range_start/range_end (well they might happen but only
> for
> > > file
> > > backed vma).
> > >
> >
> > Sadly, the mmap_sem is not enough to protect us :(.
> 
> This is enough like i explain above, but i am only talking about the mmu
> notifier registration. For the general case once you register you only
> need to take the mmap_sem in read mode during page fault.
> 

I think we are not broadcasting on the same wavelength here. The issue I'm
worried about is of adding a sub-area to our tracking system. It is built
quite differently from how HMM is built, we are defining areas to track
a-priori, and later on account how many notifiers are blocking page-faults
for each area. You are keeping track of the active notifiers, and check
each page fault against your notifier list. This difference makes for
different locking needs.

> > > Given that you face the same issue as i have with the
> > > range_start/range_end i
> > > will stich up a patch to make it easier to track those.
> > >
> >
> > That would be nice, especially if we could easily integrate it into
> our
> > code and reduce the code size.
> 
> Yes it's a "small modification" to the mmu_notifier api, i have been
> side
> tracked on other thing. But i will have it soon.
> 

Being side tracked is a well-known professional risk in our line of work ;)


> >
> > > Cheers,
> > > Jérôme
> > >
> > >
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Jerome Glisse]

[-- Attachment #1: Type: text/plain, Size: 14121 bytes --]

On Thu, Sep 11, 2014 at 12:19:01PM +0000, Shachar Raindel wrote:
> 
> 
> > -----Original Message-----
> > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > Sent: Wednesday, September 10, 2014 11:15 PM
> > To: Shachar Raindel
> > Cc: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Sagi Grimberg
> > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support for
> > MMU notifiers regarding on demand paging regions
> > 
> > On Wed, Sep 10, 2014 at 09:00:36AM +0000, Shachar Raindel wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > > > Sent: Tuesday, September 09, 2014 6:37 PM
> > > > To: Shachar Raindel
> > > > Cc: 1404377069-20585-1-git-send-email-haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org; Haggai
> > Eran;
> > > > linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Jerome Glisse; Sagi Grimberg
> > > > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support
> > for
> > > > MMU notifiers regarding on demand paging regions
> > > >
> > > > On Sun, Sep 07, 2014 at 02:35:59PM +0000, Shachar Raindel wrote:
> > > > > Hi,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > > > > > Sent: Thursday, September 04, 2014 11:25 PM
> > > > > > To: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > > > Cc: Shachar Raindel; Sagi Grimberg
> > > > > > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement
> > support
> > > > for
> > > > > > MMU notifiers regarding on demand paging regions
> > > > > >
> 
> <SNIP>
> 
> > > > >
> > > > > Sadly, taking mmap_sem in read-only mode does not prevent all
> > possible
> > > > invalidations from happening.
> > > > > For example, a call to madvise requesting MADVISE_DONTNEED will
> > lock
> > > > the mmap_sem for reading only, allowing a notifier to run in
> > parallel to
> > > > the MR registration As a result, the following sequence of events
> > could
> > > > happen:
> > > > >
> > > > > Thread 1:                       |   Thread 2
> > > > > --------------------------------+-------------------------
> > > > > madvise                         |
> > > > > down_read(mmap_sem)             |
> > > > > notifier_start                  |
> > > > >                                 |   down_read(mmap_sem)
> > > > >                                 |   register_mr
> > > > > notifier_end                    |
> > > > > reduce_mr_notifiers_count       |
> > > > >
> > > > > The end result of this sequence is an mr with running notifiers
> > count
> > > > of -1, which is bad.
> > > > > The current workaround is to avoid decreasing the notifiers count
> > if
> > > > it is zero, which can cause other issues.
> > > > > The proper fix would be to prevent notifiers from running in
> > parallel
> > > > to registration. For this, taking mmap_sem in write mode might be
> > > > sufficient, but we are not sure about this.
> > > > > We will be happy to hear additional input on this subject, to make
> > > > sure we got it covered properly.
> > > >
> > > > So in HMM i solve this by having a struct allocated in the start
> > range
> > > > callback
> > > > and the end range callback just ignore things when it can not find
> > the
> > > > matching
> > > > struct.
> > >
> > > This kind of mechanism sounds like it has a bigger risk for
> > deadlocking
> > > the system, causing an OOM kill without a real need or significantly
> > > slowing down the system.
> > > If you are doing non-atomic memory allocations, you can deadlock the
> > > system by requesting memory in the swapper flow.
> > > Even if you are doing atomic memory allocations, you need to handle
> > the
> > > case of failing allocation, the solution to which is unclear to me.
> > > If you are using a pre-allocated pool, what are you doing when you run
> > > out of available entries in the pool? If you are blocking until some
> > > entries free up, what guarantees you that this will not cause a
> > deadlock?
> > 
> > So i am using a fixed pool and when it runs out it block in start
> > callback
> > until one is freed. 
> 
> This sounds scary. You now create a possible locking dependency between
> two code flows which could have run in parallel. This can cause circular
> locking bugs, from code which functioned properly until now. For example,
> assume code with a single lock, and the following code paths:
> 
> Code 1:
> notify_start()
> lock()
> unlock()
> notify_end()
> 
> Code 2:
> lock()
> notify_start()
> ... (no locking)
> notify_end()
> unlock()
> 

This can not happen because all lock taken before notify_start() are
never taken after it and all lock taken inside a start/end section
are never hold accross a notify_start() callback.

> 
> 
> This code can now create the following deadlock:
> 
> Thread 1:        | Thread 2:
> -----------------+-----------------------------------
> notify_start()   |
>                  | lock()
> lock() - blocking|
>                  | notify_start() - blocking for slot
> 
> 
> 
> 
> > But as i said i have a patch to use the stack that
> > will
> > solve this and avoid a pool.
> 
> How are you allocating from the stack an entry which you need to keep alive
> until another function is called? You can't allocate the entry on the
> notify_start stack, so you must do this in all of the call points to the
> mmu_notifiers. Given the notifiers listener subscription pattern, this seems
> like something which is not practical.

Yes the patch add a struct in each callsite of mmu_notifier_invalidate_range
as in all case both start and end are call from same function. The only draw
back is that it increase stack consumption in some of those callsite (not all).
I attach the patch i am thinking of (it is untested) but idea is that through
two new helper function user of mmu_notifier can query active invalid range and
synchronize with those (also require some code in the range_start() callback).

> 
>  
> > 
> > >
> > > >
> > > > That being said when registering the mmu_notifier you need 2 things,
> > > > first you
> > > > need a pin on the mm (either mm is current ie current->mm or you
> > took a
> > > > reference
> > > > on it). Second you need to that the mmap smemaphore in write mode so
> > > > that
> > > > no concurrent mmap/munmap/madvise can happen. By doing that you
> > protect
> > > > yourself
> > > > from concurrent range_start/range_end that can happen and that does
> > > > matter.
> > > > The only concurrent range_start/end that can happen is through file
> > > > invalidation
> > > > which is fine because subsequent page fault will go through the file
> > > > layer and
> > > > bring back page or return error (if file was truncated for
> > instance).
> > >
> > > Sadly, this is not sufficient for our use case. We are registering
> > > a single MMU notifier handler, and broadcast the notifications to
> > > all relevant listeners, which are stored in an interval tree.
> > >
> > > Each listener represents a section of the address space that has been
> > > exposed to the network. Such implementation allows us to limit the
> > impact
> > > of invalidations, and only block racing page faults to the affected
> > areas.
> > >
> > > Each of the listeners maintain a counter of the number of
> > invalidate_range
> > > notifications that are currently affecting it. The counter is
> > increased
> > > for each invalidate_range_start callback received, and decrease for
> > each
> > > invalidate_range_end callback received. If we add a listener to the
> > > interval tree after the invalidate_range_start callback happened, but
> > > before the invalidate_range_end callback happened, it will decrease
> > the
> > > counter, reaching negative numbers and breaking the logic.
> > >
> > > The mmu_notifiers registration code avoid such issues by taking all
> > > relevant locks on the MM. This effectively blocks all possible
> > notifiers
> > > from happening when registering a new notifier. Sadly, this function
> > is
> > > not exported for modules to use it.
> > >
> > > Our options at the moment are:
> > > - Use a tracking mechanism similar to what HMM uses, alongside the
> > >   challenges involved in allocating memory from notifiers
> > >
> > > - Use a per-process counter for invalidations, causing a possible
> > >   performance degradation. This can possibly be used as a fallback to
> > the
> > >   first option (i.e. have a pool of X notifier identifiers, once it is
> > >   full, increase/decrease a per-MM counter)
> > >
> > > - Export the mm_take_all_locks function for modules. This will allow
> > us
> > >   to lock the MM when adding a new listener.
> > 
> > I was not clear enough, you need to take the mmap_sem in write mode
> > accross
> > mmu_notifier_register(). This is only to partialy solve your issue that
> > if
> > a mmu_notifier is already register for the mm you are trying to
> > registering
> > against then there is a chance for you to be inside an active
> > range_start/
> > range_end section which would lead to invalid counter inside your
> > tracking
> > structure. But, sadly, taking mmap_sem in write mode is not enough as
> > file
> > invalidation might still happen concurrently so you will need to make
> > sure
> > you invalidation counters does not go negative but from page fault point
> > of
> > view you will be fine because the page fault will synchronize through
> > the
> > pagecache. So scenario (A and B are to anonymous overlapping address
> > range) :
> > 
> >   APP_TOTO_RDMA_THREAD           |  APP_TOTO_SOME_OTHER_THREAD
> >                                  |  mmu_notifier_invalidate_range_start(A)
> >   odp_register()                 |
> >     down_read(mmap_sem)          |
> >     mmu_notifier_register()      |
> >     up_read(mmap_sem)            |
> >   odp_add_new_region(B)          |
> >   odp_page_fault(B)              |
> >     down_read(mmap_sem)          |
> >     ...                          |
> >     up_read(mmap_sem)            |
> >                                  |  mmu_notifier_invalidate_range_end(A)
> > 
> > The odp_page_fault(B) might see invalid cpu page table but you have no
> > idea
> > about it because you registered after the range_start(). But if you take
> > the
> > mmap_sem in write mode then the only case where you might still have
> > this
> > scenario is if A and B are range of a file backed vma and that the file
> > is
> > undergoing some change (most likely truncation). But the file case is
> > fine
> > because the odp_page_fault() will go through the pagecache which is
> > properly
> > synchronize against the current range invalidation.
> 
> Specifically, if you call mmu_notifier_register you are OK and the above
> scenario will not happen. You are supposed to hold mmap_sem for writing,
> and mmu_notifier_register is calling mm_take_all_locks, which guarantees
> no racing notifier during the registration step.
> 
> However, we want to dynamically add sub-notifiers in our code. Each will
> get notified only about invalidations touching a specific sub-sections of
> the address space. To avoid providing unneeded notifications, we use an
> interval tree that filters only the needed notifications.
> When adding entries to the interval tree, we cannot lock the mm to prevent
> any racing invalidations. As such, we might end up in a case where a newly
> registered memory region will get a "notify_end" call without the relevant
> "notify_start". Even if we prevent the value from dropping below zero, it
> means we can cause data corruption. For example, if we have another
> notifier running after the MR registers, which is due to munmap, but we get
> first the notify_end of the previous notifier for which we didn't see the
> notify_start.
> 
> The solution we are coming up with now is using a global counter of running
> invalidations for new regions allocated. When the global counter is at zero,
> we can safely switch to the region local invalidations counter.

Yes i fully understood that design but as i said this kind of broken and this
is what the attached patch try to address as HMM have the same issue of having
to track all active invalidation range.

> 
> 
> > 
> > 
> > Now for the the general case outside of mmu_notifier_register() HMM also
> > track
> > active invalidation range to avoid page faulting into those range as we
> > can not
> > trust the cpu page table for as long as the range invalidation is on
> > going.
> > 
> > > >
> > > > So as long as you hold the mmap_sem in write mode you should not
> > worry
> > > > about
> > > > concurrent range_start/range_end (well they might happen but only
> > for
> > > > file
> > > > backed vma).
> > > >
> > >
> > > Sadly, the mmap_sem is not enough to protect us :(.
> > 
> > This is enough like i explain above, but i am only talking about the mmu
> > notifier registration. For the general case once you register you only
> > need to take the mmap_sem in read mode during page fault.
> > 
> 
> I think we are not broadcasting on the same wavelength here. The issue I'm
> worried about is of adding a sub-area to our tracking system. It is built
> quite differently from how HMM is built, we are defining areas to track
> a-priori, and later on account how many notifiers are blocking page-faults
> for each area. You are keeping track of the active notifiers, and check
> each page fault against your notifier list. This difference makes for
> different locking needs.
> 
> > > > Given that you face the same issue as i have with the
> > > > range_start/range_end i
> > > > will stich up a patch to make it easier to track those.
> > > >
> > >
> > > That would be nice, especially if we could easily integrate it into
> > our
> > > code and reduce the code size.
> > 
> > Yes it's a "small modification" to the mmu_notifier api, i have been
> > side
> > tracked on other thing. But i will have it soon.
> > 
> 
> Being side tracked is a well-known professional risk in our line of work ;)
> 
> 
> > >
> > > > Cheers,
> > > > Jérôme
> > > >
> > > >

[-- Attachment #2: 0001-mmu_notifier-keep-track-of-active-invalidation-range.patch --]
[-- Type: text/plain, Size: 39737 bytes --]

>From 037195e49fbed468d16b78f0364fe302bc732d12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Thu, 11 Sep 2014 11:22:12 -0400
Subject: [PATCH] mmu_notifier: keep track of active invalidation ranges
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The mmu_notifier_invalidate_range_start() and mmu_notifier_invalidate_range_end()
can be considered as forming an "atomic" section for the cpu page table update
point of view. Between this two function the cpu page table content is unreliable
for the affected range of address.

Current user such as kvm need to know when they can trust a the content of the
cpu page table. This becomes even more important to new users of the mmu_notifier
api (such as HMM or ODP).

This patch use a structure define at all call site to invalidate_range_start()
that is added to a list for the duration of the invalidation. It adds two new
helpers to allow querying if a range is being invalidated or to wait for a range
to become valid.

This two new function does not provide strong synchronization but are intended
to be use as helper. User of the mmu_notifier must also synchronize with themself
inside their range_start() and range_end() callback.

Signed-off-by: Jérôme Glisse <jglisse-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/i915/i915_gem_userptr.c | 13 +++---
 drivers/iommu/amd_iommu_v2.c            |  8 +---
 drivers/misc/sgi-gru/grutlbpurge.c      | 15 +++----
 drivers/xen/gntdev.c                    |  8 ++--
 fs/proc/task_mmu.c                      | 12 +++--
 include/linux/mmu_notifier.h            | 55 ++++++++++++-----------
 mm/fremap.c                             |  8 +++-
 mm/huge_memory.c                        | 78 ++++++++++++++-------------------
 mm/hugetlb.c                            | 49 +++++++++++----------
 mm/memory.c                             | 73 ++++++++++++++++--------------
 mm/migrate.c                            | 16 +++----
 mm/mmu_notifier.c                       | 73 +++++++++++++++++++++++++-----
 mm/mprotect.c                           | 17 ++++---
 mm/mremap.c                             | 14 +++---
 mm/rmap.c                               | 15 +++----
 virt/kvm/kvm_main.c                     | 10 ++---
 16 files changed, 256 insertions(+), 208 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index a13307d..373ffbb 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -123,26 +123,25 @@ restart:
 
 static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
 						       struct mm_struct *mm,
-						       unsigned long start,
-						       unsigned long end,
-						       enum mmu_event event)
+						       const struct mmu_notifier_range *range)
 {
 	struct i915_mmu_notifier *mn = container_of(_mn, struct i915_mmu_notifier, mn);
 	struct interval_tree_node *it = NULL;
-	unsigned long next = start;
+	unsigned long next = range->start;
 	unsigned long serial = 0;
+	/* interval ranges are inclusive, but invalidate range is exclusive */
+	unsigned long end = range.end - 1;
 
-	end--; /* interval ranges are inclusive, but invalidate range is exclusive */
 	while (next < end) {
 		struct drm_i915_gem_object *obj = NULL;
 
 		spin_lock(&mn->lock);
 		if (mn->has_linear)
-			it = invalidate_range__linear(mn, mm, start, end);
+			it = invalidate_range__linear(mn, mm, range->start, end);
 		else if (serial == mn->serial)
 			it = interval_tree_iter_next(it, next, end);
 		else
-			it = interval_tree_iter_first(&mn->objects, start, end);
+			it = interval_tree_iter_first(&mn->objects, range->start, end);
 		if (it != NULL) {
 			obj = container_of(it, struct i915_mmu_object, it)->obj;
 			drm_gem_object_reference(&obj->base);
diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 9a6b837..5945300 100644
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -419,9 +419,7 @@ static void mn_invalidate_page(struct mmu_notifier *mn,
 
 static void mn_invalidate_range_start(struct mmu_notifier *mn,
 				      struct mm_struct *mm,
-				      unsigned long start,
-				      unsigned long end,
-				      enum mmu_event event)
+				      const struct mmu_notifier_range *range)
 {
 	struct pasid_state *pasid_state;
 	struct device_state *dev_state;
@@ -442,9 +440,7 @@ static void mn_invalidate_range_start(struct mmu_notifier *mn,
 
 static void mn_invalidate_range_end(struct mmu_notifier *mn,
 				    struct mm_struct *mm,
-				    unsigned long start,
-				    unsigned long end,
-				    enum mmu_event event)
+				    const struct mmu_notifier_range *range)
 {
 	struct pasid_state *pasid_state;
 	struct device_state *dev_state;
diff --git a/drivers/misc/sgi-gru/grutlbpurge.c b/drivers/misc/sgi-gru/grutlbpurge.c
index e67fed1..44b41b7 100644
--- a/drivers/misc/sgi-gru/grutlbpurge.c
+++ b/drivers/misc/sgi-gru/grutlbpurge.c
@@ -221,8 +221,7 @@ void gru_flush_all_tlb(struct gru_state *gru)
  */
 static void gru_invalidate_range_start(struct mmu_notifier *mn,
 				       struct mm_struct *mm,
-				       unsigned long start, unsigned long end,
-				       enum mmu_event event)
+				       const struct mmu_notifier_range *range)
 {
 	struct gru_mm_struct *gms = container_of(mn, struct gru_mm_struct,
 						 ms_notifier);
@@ -230,14 +229,13 @@ static void gru_invalidate_range_start(struct mmu_notifier *mn,
 	STAT(mmu_invalidate_range);
 	atomic_inc(&gms->ms_range_active);
 	gru_dbg(grudev, "gms %p, start 0x%lx, end 0x%lx, act %d\n", gms,
-		start, end, atomic_read(&gms->ms_range_active));
-	gru_flush_tlb_range(gms, start, end - start);
+		range->start, range->end, atomic_read(&gms->ms_range_active));
+	gru_flush_tlb_range(gms, range->start, range->end - range->start);
 }
 
 static void gru_invalidate_range_end(struct mmu_notifier *mn,
-				     struct mm_struct *mm, unsigned long start,
-				     unsigned long end,
-				     enum mmu_event event)
+				     struct mm_struct *mm,
+				     const struct mmu_notifier_range *range)
 {
 	struct gru_mm_struct *gms = container_of(mn, struct gru_mm_struct,
 						 ms_notifier);
@@ -246,7 +244,8 @@ static void gru_invalidate_range_end(struct mmu_notifier *mn,
 	(void)atomic_dec_and_test(&gms->ms_range_active);
 
 	wake_up_all(&gms->ms_wait_queue);
-	gru_dbg(grudev, "gms %p, start 0x%lx, end 0x%lx\n", gms, start, end);
+	gru_dbg(grudev, "gms %p, start 0x%lx, end 0x%lx\n", gms,
+		range->start, range->end);
 }
 
 static void gru_invalidate_page(struct mmu_notifier *mn, struct mm_struct *mm,
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index fe9da94..51f9188 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -428,19 +428,17 @@ static void unmap_if_in_range(struct grant_map *map,
 
 static void mn_invl_range_start(struct mmu_notifier *mn,
 				struct mm_struct *mm,
-				unsigned long start,
-				unsigned long end,
-				enum mmu_event event)
+				const struct mmu_notifier_range *range)
 {
 	struct gntdev_priv *priv = container_of(mn, struct gntdev_priv, mn);
 	struct grant_map *map;
 
 	spin_lock(&priv->lock);
 	list_for_each_entry(map, &priv->maps, next) {
-		unmap_if_in_range(map, start, end);
+		unmap_if_in_range(map, range->start, range->end);
 	}
 	list_for_each_entry(map, &priv->freeable_maps, next) {
-		unmap_if_in_range(map, start, end);
+		unmap_if_in_range(map, range->start, range->end);
 	}
 	spin_unlock(&priv->lock);
 }
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 0ddb975..532a230 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -828,10 +828,15 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
 			.mm = mm,
 			.private = &cp,
 		};
+		struct mmu_notifier_range range = {
+			.start = 0,
+			.end = -1UL,
+			.event = MMU_ISDIRTY,
+		};
+
 		down_read(&mm->mmap_sem);
 		if (type == CLEAR_REFS_SOFT_DIRTY)
-			mmu_notifier_invalidate_range_start(mm, 0,
-							    -1, MMU_ISDIRTY);
+			mmu_notifier_invalidate_range_start(mm, &range);
 		for (vma = mm->mmap; vma; vma = vma->vm_next) {
 			cp.vma = vma;
 			if (is_vm_hugetlb_page(vma))
@@ -859,8 +864,7 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
 					&clear_refs_walk);
 		}
 		if (type == CLEAR_REFS_SOFT_DIRTY)
-			mmu_notifier_invalidate_range_end(mm, 0,
-							  -1, MMU_ISDIRTY);
+			mmu_notifier_invalidate_range_end(mm, &range);
 		flush_tlb_mm(mm);
 		up_read(&mm->mmap_sem);
 		mmput(mm);
diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index 94f6890..f4a2a74 100644
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -69,6 +69,13 @@ enum mmu_event {
 	MMU_WRITE_PROTECT,
 };
 
+struct mmu_notifier_range {
+	struct list_head list;
+	unsigned long start;
+	unsigned long end;
+	enum mmu_event event;
+};
+
 #ifdef CONFIG_MMU_NOTIFIER
 
 /*
@@ -82,6 +89,12 @@ struct mmu_notifier_mm {
 	struct hlist_head list;
 	/* to serialize the list modifications and hlist_unhashed */
 	spinlock_t lock;
+	/* List of all active range invalidations. */
+	struct list_head ranges;
+	/* Number of active range invalidations. */
+	int nranges;
+	/* For threads waiting on range invalidations. */
+	wait_queue_head_t wait_queue;
 };
 
 struct mmu_notifier_ops {
@@ -199,14 +212,10 @@ struct mmu_notifier_ops {
 	 */
 	void (*invalidate_range_start)(struct mmu_notifier *mn,
 				       struct mm_struct *mm,
-				       unsigned long start,
-				       unsigned long end,
-				       enum mmu_event event);
+				       const struct mmu_notifier_range *range);
 	void (*invalidate_range_end)(struct mmu_notifier *mn,
 				     struct mm_struct *mm,
-				     unsigned long start,
-				     unsigned long end,
-				     enum mmu_event event);
+				     const struct mmu_notifier_range *range);
 };
 
 /*
@@ -252,13 +261,15 @@ extern void __mmu_notifier_invalidate_page(struct mm_struct *mm,
 					  unsigned long address,
 					  enum mmu_event event);
 extern void __mmu_notifier_invalidate_range_start(struct mm_struct *mm,
-						  unsigned long start,
-						  unsigned long end,
-						  enum mmu_event event);
+						  struct mmu_notifier_range *range);
 extern void __mmu_notifier_invalidate_range_end(struct mm_struct *mm,
-						unsigned long start,
-						unsigned long end,
-						enum mmu_event event);
+						struct mmu_notifier_range *range);
+extern bool mmu_notifier_range_is_valid(struct mm_struct *mm,
+					unsigned long start,
+					unsigned long end);
+extern void mmu_notifier_range_wait_valid(struct mm_struct *mm,
+					  unsigned long start,
+					  unsigned long end);
 
 static inline void mmu_notifier_release(struct mm_struct *mm)
 {
@@ -300,21 +311,17 @@ static inline void mmu_notifier_invalidate_page(struct mm_struct *mm,
 }
 
 static inline void mmu_notifier_invalidate_range_start(struct mm_struct *mm,
-						       unsigned long start,
-						       unsigned long end,
-						       enum mmu_event event)
+						       struct mmu_notifier_range *range)
 {
 	if (mm_has_notifiers(mm))
-		__mmu_notifier_invalidate_range_start(mm, start, end, event);
+		__mmu_notifier_invalidate_range_start(mm, range);
 }
 
 static inline void mmu_notifier_invalidate_range_end(struct mm_struct *mm,
-						     unsigned long start,
-						     unsigned long end,
-						     enum mmu_event event)
+						     struct mmu_notifier_range *range)
 {
 	if (mm_has_notifiers(mm))
-		__mmu_notifier_invalidate_range_end(mm, start, end, event);
+		__mmu_notifier_invalidate_range_end(mm, range);
 }
 
 static inline void mmu_notifier_mm_init(struct mm_struct *mm)
@@ -406,16 +413,12 @@ static inline void mmu_notifier_invalidate_page(struct mm_struct *mm,
 }
 
 static inline void mmu_notifier_invalidate_range_start(struct mm_struct *mm,
-						       unsigned long start,
-						       unsigned long end,
-						       enum mmu_event event)
+						       struct mmu_notifier_range *range)
 {
 }
 
 static inline void mmu_notifier_invalidate_range_end(struct mm_struct *mm,
-						     unsigned long start,
-						     unsigned long end,
-						     enum mmu_event event)
+						     struct mmu_notifier_range *range)
 {
 }
 
diff --git a/mm/fremap.c b/mm/fremap.c
index 37b2904..03a5ddc 100644
--- a/mm/fremap.c
+++ b/mm/fremap.c
@@ -148,6 +148,7 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
 	int err = -EINVAL;
 	int has_write_lock = 0;
 	vm_flags_t vm_flags = 0;
+	struct mmu_notifier_range range;
 
 	pr_warn_once("%s (%d) uses deprecated remap_file_pages() syscall. "
 			"See Documentation/vm/remap_file_pages.txt.\n",
@@ -258,9 +259,12 @@ get_write_lock:
 		vma->vm_flags = vm_flags;
 	}
 
-	mmu_notifier_invalidate_range_start(mm, start, start + size, MMU_MUNMAP);
+	range.start = start;
+	range.end = start + size;
+	range.event = MMU_MUNMAP;
+	mmu_notifier_invalidate_range_start(mm, &range);
 	err = vma->vm_ops->remap_pages(vma, start, size, pgoff);
-	mmu_notifier_invalidate_range_end(mm, start, start + size, MMU_MUNMAP);
+	mmu_notifier_invalidate_range_end(mm, &range);
 
 	/*
 	 * We can't clear VM_NONLINEAR because we'd have to do
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index e3efba5..4b116dd 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -988,8 +988,7 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct *mm,
 	pmd_t _pmd;
 	int ret = 0, i;
 	struct page **pages;
-	unsigned long mmun_start;	/* For mmu_notifiers */
-	unsigned long mmun_end;		/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 
 	pages = kmalloc(sizeof(struct page *) * HPAGE_PMD_NR,
 			GFP_KERNEL);
@@ -1027,10 +1026,10 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct *mm,
 		cond_resched();
 	}
 
-	mmun_start = haddr;
-	mmun_end   = haddr + HPAGE_PMD_SIZE;
-	mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end,
-					    MMU_MIGRATE);
+	range.start = haddr;
+	range.end = haddr + HPAGE_PMD_SIZE;
+	range.event = MMU_MIGRATE;
+	mmu_notifier_invalidate_range_start(mm, &range);
 
 	ptl = pmd_lock(mm, pmd);
 	if (unlikely(!pmd_same(*pmd, orig_pmd)))
@@ -1064,8 +1063,7 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct *mm,
 	page_remove_rmap(page);
 	spin_unlock(ptl);
 
-	mmu_notifier_invalidate_range_end(mm, mmun_start,
-					  mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_end(mm, &range);
 
 	ret |= VM_FAULT_WRITE;
 	put_page(page);
@@ -1075,8 +1073,7 @@ out:
 
 out_free_pages:
 	spin_unlock(ptl);
-	mmu_notifier_invalidate_range_end(mm, mmun_start,
-					  mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_end(mm, &range);
 	for (i = 0; i < HPAGE_PMD_NR; i++) {
 		memcg = (void *)page_private(pages[i]);
 		set_page_private(pages[i], 0);
@@ -1095,8 +1092,7 @@ int do_huge_pmd_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
 	struct page *page = NULL, *new_page;
 	struct mem_cgroup *memcg;
 	unsigned long haddr;
-	unsigned long mmun_start;	/* For mmu_notifiers */
-	unsigned long mmun_end;		/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 
 	ptl = pmd_lockptr(mm, pmd);
 	VM_BUG_ON(!vma->anon_vma);
@@ -1166,10 +1162,10 @@ alloc:
 		copy_user_huge_page(new_page, page, haddr, vma, HPAGE_PMD_NR);
 	__SetPageUptodate(new_page);
 
-	mmun_start = haddr;
-	mmun_end   = haddr + HPAGE_PMD_SIZE;
-	mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end,
-					    MMU_MIGRATE);
+	range.start = haddr;
+	range.end = haddr + HPAGE_PMD_SIZE;
+	range.event = MMU_MIGRATE;
+	mmu_notifier_invalidate_range_start(mm, &range);
 
 	spin_lock(ptl);
 	if (page)
@@ -1201,8 +1197,7 @@ alloc:
 	}
 	spin_unlock(ptl);
 out_mn:
-	mmu_notifier_invalidate_range_end(mm, mmun_start,
-					  mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_end(mm, &range);
 out:
 	return ret;
 out_unlock:
@@ -1633,12 +1628,12 @@ static int __split_huge_page_splitting(struct page *page,
 	spinlock_t *ptl;
 	pmd_t *pmd;
 	int ret = 0;
-	/* For mmu_notifiers */
-	const unsigned long mmun_start = address;
-	const unsigned long mmun_end   = address + HPAGE_PMD_SIZE;
+	struct mmu_notifier_range range;
 
-	mmu_notifier_invalidate_range_start(mm, mmun_start,
-					    mmun_end, MMU_HSPLIT);
+	range.start = address;
+	range.end = address + HPAGE_PMD_SIZE;
+	range.event = MMU_HSPLIT;
+	mmu_notifier_invalidate_range_start(mm, &range);
 	pmd = page_check_address_pmd(page, mm, address,
 			PAGE_CHECK_ADDRESS_PMD_NOTSPLITTING_FLAG, &ptl);
 	if (pmd) {
@@ -1653,8 +1648,7 @@ static int __split_huge_page_splitting(struct page *page,
 		ret = 1;
 		spin_unlock(ptl);
 	}
-	mmu_notifier_invalidate_range_end(mm, mmun_start,
-					  mmun_end, MMU_HSPLIT);
+	mmu_notifier_invalidate_range_end(mm, &range);
 
 	return ret;
 }
@@ -2434,8 +2428,7 @@ static void collapse_huge_page(struct mm_struct *mm,
 	int isolated;
 	unsigned long hstart, hend;
 	struct mem_cgroup *memcg;
-	unsigned long mmun_start;	/* For mmu_notifiers */
-	unsigned long mmun_end;		/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 
 	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
 
@@ -2475,10 +2468,10 @@ static void collapse_huge_page(struct mm_struct *mm,
 	pte = pte_offset_map(pmd, address);
 	pte_ptl = pte_lockptr(mm, pmd);
 
-	mmun_start = address;
-	mmun_end   = address + HPAGE_PMD_SIZE;
-	mmu_notifier_invalidate_range_start(mm, mmun_start,
-					    mmun_end, MMU_MIGRATE);
+	range.start = address;
+	range.end = address + HPAGE_PMD_SIZE;
+	range.event = MMU_MIGRATE;
+	mmu_notifier_invalidate_range_start(mm, &range);
 	pmd_ptl = pmd_lock(mm, pmd); /* probably unnecessary */
 	/*
 	 * After this gup_fast can't run anymore. This also removes
@@ -2488,8 +2481,7 @@ static void collapse_huge_page(struct mm_struct *mm,
 	 */
 	_pmd = pmdp_clear_flush(vma, address, pmd);
 	spin_unlock(pmd_ptl);
-	mmu_notifier_invalidate_range_end(mm, mmun_start,
-					  mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_end(mm, &range);
 
 	spin_lock(pte_ptl);
 	isolated = __collapse_huge_page_isolate(vma, address, pte);
@@ -2872,36 +2864,32 @@ void __split_huge_page_pmd(struct vm_area_struct *vma, unsigned long address,
 	struct page *page;
 	struct mm_struct *mm = vma->vm_mm;
 	unsigned long haddr = address & HPAGE_PMD_MASK;
-	unsigned long mmun_start;	/* For mmu_notifiers */
-	unsigned long mmun_end;		/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 
 	BUG_ON(vma->vm_start > haddr || vma->vm_end < haddr + HPAGE_PMD_SIZE);
 
-	mmun_start = haddr;
-	mmun_end   = haddr + HPAGE_PMD_SIZE;
+	range.start = haddr;
+	range.end = haddr + HPAGE_PMD_SIZE;
+	range.event = MMU_MIGRATE;
 again:
-	mmu_notifier_invalidate_range_start(mm, mmun_start,
-					    mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_start(mm, &range);
 	ptl = pmd_lock(mm, pmd);
 	if (unlikely(!pmd_trans_huge(*pmd))) {
 		spin_unlock(ptl);
-		mmu_notifier_invalidate_range_end(mm, mmun_start,
-						  mmun_end, MMU_MIGRATE);
+		mmu_notifier_invalidate_range_end(mm, &range);
 		return;
 	}
 	if (is_huge_zero_pmd(*pmd)) {
 		__split_huge_zero_page_pmd(vma, haddr, pmd);
 		spin_unlock(ptl);
-		mmu_notifier_invalidate_range_end(mm, mmun_start,
-						  mmun_end, MMU_MIGRATE);
+		mmu_notifier_invalidate_range_end(mm, &range);
 		return;
 	}
 	page = pmd_page(*pmd);
 	VM_BUG_ON_PAGE(!page_count(page), page);
 	get_page(page);
 	spin_unlock(ptl);
-	mmu_notifier_invalidate_range_end(mm, mmun_start,
-					  mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_end(mm, &range);
 
 	split_huge_page(page);
 
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index ae98b53..6484793 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2551,17 +2551,16 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
 	int cow;
 	struct hstate *h = hstate_vma(vma);
 	unsigned long sz = huge_page_size(h);
-	unsigned long mmun_start;	/* For mmu_notifiers */
-	unsigned long mmun_end;		/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 	int ret = 0;
 
 	cow = (vma->vm_flags & (VM_SHARED | VM_MAYWRITE)) == VM_MAYWRITE;
 
-	mmun_start = vma->vm_start;
-	mmun_end = vma->vm_end;
+	range.start = vma->vm_start;
+	range.end = vma->vm_end;
+	range.event = MMU_MIGRATE;
 	if (cow)
-		mmu_notifier_invalidate_range_start(src, mmun_start,
-						    mmun_end, MMU_MIGRATE);
+		mmu_notifier_invalidate_range_start(src, &range);
 
 	for (addr = vma->vm_start; addr < vma->vm_end; addr += sz) {
 		spinlock_t *src_ptl, *dst_ptl;
@@ -2612,8 +2611,7 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
 	}
 
 	if (cow)
-		mmu_notifier_invalidate_range_end(src, mmun_start,
-						  mmun_end, MMU_MIGRATE);
+		mmu_notifier_invalidate_range_end(src, &range);
 
 	return ret;
 }
@@ -2631,16 +2629,17 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
 	struct page *page;
 	struct hstate *h = hstate_vma(vma);
 	unsigned long sz = huge_page_size(h);
-	const unsigned long mmun_start = start;	/* For mmu_notifiers */
-	const unsigned long mmun_end   = end;	/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 
 	WARN_ON(!is_vm_hugetlb_page(vma));
 	BUG_ON(start & ~huge_page_mask(h));
 	BUG_ON(end & ~huge_page_mask(h));
 
+	range.start = start;
+	range.end = end;
+	range.event = MMU_MIGRATE;
 	tlb_start_vma(tlb, vma);
-	mmu_notifier_invalidate_range_start(mm, mmun_start,
-					    mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_start(mm, &range);
 again:
 	for (address = start; address < end; address += sz) {
 		ptep = huge_pte_offset(mm, address);
@@ -2711,8 +2710,7 @@ unlock:
 		if (address < end && !ref_page)
 			goto again;
 	}
-	mmu_notifier_invalidate_range_end(mm, mmun_start,
-					  mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_end(mm, &range);
 	tlb_end_vma(tlb, vma);
 }
 
@@ -2809,8 +2807,7 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
 	struct hstate *h = hstate_vma(vma);
 	struct page *old_page, *new_page;
 	int ret = 0, outside_reserve = 0;
-	unsigned long mmun_start;	/* For mmu_notifiers */
-	unsigned long mmun_end;		/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 
 	old_page = pte_page(pte);
 
@@ -2888,10 +2885,11 @@ retry_avoidcopy:
 			    pages_per_huge_page(h));
 	__SetPageUptodate(new_page);
 
-	mmun_start = address & huge_page_mask(h);
-	mmun_end = mmun_start + huge_page_size(h);
-	mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end,
-					    MMU_MIGRATE);
+	range.start = address;
+	range.end = address + huge_page_size(h);
+	range.event = MMU_MIGRATE;
+	mmu_notifier_invalidate_range_start(mm, &range);
+
 	/*
 	 * Retake the page table lock to check for racing updates
 	 * before the page tables are altered
@@ -2911,8 +2909,7 @@ retry_avoidcopy:
 		new_page = old_page;
 	}
 	spin_unlock(ptl);
-	mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end,
-					  MMU_MIGRATE);
+	mmu_notifier_invalidate_range_end(mm, &range);
 out_release_all:
 	page_cache_release(new_page);
 out_release_old:
@@ -3346,11 +3343,15 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
 	pte_t pte;
 	struct hstate *h = hstate_vma(vma);
 	unsigned long pages = 0;
+	struct mmu_notifier_range range;
 
 	BUG_ON(address >= end);
 	flush_cache_range(vma, address, end);
 
-	mmu_notifier_invalidate_range_start(mm, start, end, MMU_MPROT);
+	range.start = start;
+	range.end = end;
+	range.event = MMU_MPROT;
+	mmu_notifier_invalidate_range_start(mm, &range);
 	mutex_lock(&vma->vm_file->f_mapping->i_mmap_mutex);
 	for (; address < end; address += huge_page_size(h)) {
 		spinlock_t *ptl;
@@ -3380,7 +3381,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
 	 */
 	flush_tlb_range(vma, start, end);
 	mutex_unlock(&vma->vm_file->f_mapping->i_mmap_mutex);
-	mmu_notifier_invalidate_range_end(mm, start, end, MMU_MPROT);
+	mmu_notifier_invalidate_range_end(mm, &range);
 
 	return pages << h->order;
 }
diff --git a/mm/memory.c b/mm/memory.c
index 1c212e6..c1c7ccc 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1008,8 +1008,7 @@ int copy_page_range(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 	unsigned long next;
 	unsigned long addr = vma->vm_start;
 	unsigned long end = vma->vm_end;
-	unsigned long mmun_start;	/* For mmu_notifiers */
-	unsigned long mmun_end;		/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 	bool is_cow;
 	int ret;
 
@@ -1045,11 +1044,11 @@ int copy_page_range(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 	 * is_cow_mapping() returns true.
 	 */
 	is_cow = is_cow_mapping(vma->vm_flags);
-	mmun_start = addr;
-	mmun_end   = end;
+	range.start = addr;
+	range.end = end;
+	range.event = MMU_MIGRATE;
 	if (is_cow)
-		mmu_notifier_invalidate_range_start(src_mm, mmun_start,
-						    mmun_end, MMU_MIGRATE);
+		mmu_notifier_invalidate_range_start(src_mm, &range);
 
 	ret = 0;
 	dst_pgd = pgd_offset(dst_mm, addr);
@@ -1066,8 +1065,7 @@ int copy_page_range(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 	} while (dst_pgd++, src_pgd++, addr = next, addr != end);
 
 	if (is_cow)
-		mmu_notifier_invalidate_range_end(src_mm, mmun_start, mmun_end,
-						  MMU_MIGRATE);
+		mmu_notifier_invalidate_range_end(src_mm, &range);
 	return ret;
 }
 
@@ -1370,13 +1368,16 @@ void unmap_vmas(struct mmu_gather *tlb,
 		unsigned long end_addr)
 {
 	struct mm_struct *mm = vma->vm_mm;
+	struct mmu_notifier_range range = {
+		.start = start_addr,
+		.end = end_addr,
+		.event = MMU_MUNMAP,
+	};
 
-	mmu_notifier_invalidate_range_start(mm, start_addr,
-					    end_addr, MMU_MUNMAP);
+	mmu_notifier_invalidate_range_start(mm, &range);
 	for ( ; vma && vma->vm_start < end_addr; vma = vma->vm_next)
 		unmap_single_vma(tlb, vma, start_addr, end_addr, NULL);
-	mmu_notifier_invalidate_range_end(mm, start_addr,
-					  end_addr, MMU_MUNMAP);
+	mmu_notifier_invalidate_range_end(mm, &range);
 }
 
 /**
@@ -1393,16 +1394,20 @@ void zap_page_range(struct vm_area_struct *vma, unsigned long start,
 {
 	struct mm_struct *mm = vma->vm_mm;
 	struct mmu_gather tlb;
-	unsigned long end = start + size;
+	struct mmu_notifier_range range = {
+		.start = start,
+		.end = start + size,
+		.event = MMU_MUNMAP,
+	};
 
 	lru_add_drain();
-	tlb_gather_mmu(&tlb, mm, start, end);
+	tlb_gather_mmu(&tlb, mm, start, range.end);
 	update_hiwater_rss(mm);
-	mmu_notifier_invalidate_range_start(mm, start, end, MMU_MUNMAP);
-	for ( ; vma && vma->vm_start < end; vma = vma->vm_next)
-		unmap_single_vma(&tlb, vma, start, end, details);
-	mmu_notifier_invalidate_range_end(mm, start, end, MMU_MUNMAP);
-	tlb_finish_mmu(&tlb, start, end);
+	mmu_notifier_invalidate_range_start(mm, &range);
+	for ( ; vma && vma->vm_start < range.end; vma = vma->vm_next)
+		unmap_single_vma(&tlb, vma, start, range.end, details);
+	mmu_notifier_invalidate_range_end(mm, &range);
+	tlb_finish_mmu(&tlb, start, range.end);
 }
 
 /**
@@ -1419,15 +1424,19 @@ static void zap_page_range_single(struct vm_area_struct *vma, unsigned long addr
 {
 	struct mm_struct *mm = vma->vm_mm;
 	struct mmu_gather tlb;
-	unsigned long end = address + size;
+	struct mmu_notifier_range range = {
+		.start = address,
+		.end = address + size,
+		.event = MMU_MUNMAP,
+	};
 
 	lru_add_drain();
-	tlb_gather_mmu(&tlb, mm, address, end);
+	tlb_gather_mmu(&tlb, mm, address, range.end);
 	update_hiwater_rss(mm);
-	mmu_notifier_invalidate_range_start(mm, address, end, MMU_MUNMAP);
-	unmap_single_vma(&tlb, vma, address, end, details);
-	mmu_notifier_invalidate_range_end(mm, address, end, MMU_MUNMAP);
-	tlb_finish_mmu(&tlb, address, end);
+	mmu_notifier_invalidate_range_start(mm, &range);
+	unmap_single_vma(&tlb, vma, address, range.end, details);
+	mmu_notifier_invalidate_range_end(mm, &range);
+	tlb_finish_mmu(&tlb, address, range.end);
 }
 
 /**
@@ -2047,8 +2056,7 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
 	int ret = 0;
 	int page_mkwrite = 0;
 	struct page *dirty_page = NULL;
-	unsigned long mmun_start = 0;	/* For mmu_notifiers */
-	unsigned long mmun_end = 0;	/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 	struct mem_cgroup *memcg;
 
 	old_page = vm_normal_page(vma, address, orig_pte);
@@ -2208,10 +2216,10 @@ gotten:
 	if (mem_cgroup_try_charge(new_page, mm, GFP_KERNEL, &memcg))
 		goto oom_free_new;
 
-	mmun_start  = address & PAGE_MASK;
-	mmun_end    = mmun_start + PAGE_SIZE;
-	mmu_notifier_invalidate_range_start(mm, mmun_start,
-					    mmun_end, MMU_MIGRATE);
+	range.start = address & PAGE_MASK;
+	range.end = range.start + PAGE_SIZE;
+	range.event = MMU_MIGRATE;
+	mmu_notifier_invalidate_range_start(mm, &range);
 
 	/*
 	 * Re-check the pte - we dropped the lock
@@ -2282,8 +2290,7 @@ gotten:
 unlock:
 	pte_unmap_unlock(page_table, ptl);
 	if (mmun_end > mmun_start)
-		mmu_notifier_invalidate_range_end(mm, mmun_start,
-						  mmun_end, MMU_MIGRATE);
+		mmu_notifier_invalidate_range_end(mm, &range);
 	if (old_page) {
 		/*
 		 * Don't let another task, with possibly unlocked vma,
diff --git a/mm/migrate.c b/mm/migrate.c
index 30417d5..d866771 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1781,10 +1781,13 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
 	int isolated = 0;
 	struct page *new_page = NULL;
 	int page_lru = page_is_file_cache(page);
-	unsigned long mmun_start = address & HPAGE_PMD_MASK;
-	unsigned long mmun_end = mmun_start + HPAGE_PMD_SIZE;
+	struct mmu_notifier_range range;
 	pmd_t orig_entry;
 
+	range.start = address & HPAGE_PMD_MASK;
+	range.end = range.start + HPAGE_PMD_SIZE;
+	range.event = MMU_MIGRATE;
+
 	/*
 	 * Rate-limit the amount of data that is being migrated to a node.
 	 * Optimal placement is no good if the memory bus is saturated and
@@ -1819,14 +1822,12 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
 	WARN_ON(PageLRU(new_page));
 
 	/* Recheck the target PMD */
-	mmu_notifier_invalidate_range_start(mm, mmun_start,
-					    mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_start(mm, &range);
 	ptl = pmd_lock(mm, pmd);
 	if (unlikely(!pmd_same(*pmd, entry) || page_count(page) != 2)) {
 fail_putback:
 		spin_unlock(ptl);
-		mmu_notifier_invalidate_range_end(mm, mmun_start,
-						  mmun_end, MMU_MIGRATE);
+		mmu_notifier_invalidate_range_end(mm, &range);
 
 		/* Reverse changes made by migrate_page_copy() */
 		if (TestClearPageActive(new_page))
@@ -1879,8 +1880,7 @@ fail_putback:
 	page_remove_rmap(page);
 
 	spin_unlock(ptl);
-	mmu_notifier_invalidate_range_end(mm, mmun_start,
-					  mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_end(mm, &range);
 
 	/* Take an "isolate" reference and put new page on the LRU. */
 	get_page(new_page);
diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
index de039e4..d0edb98 100644
--- a/mm/mmu_notifier.c
+++ b/mm/mmu_notifier.c
@@ -173,9 +173,7 @@ void __mmu_notifier_invalidate_page(struct mm_struct *mm,
 }
 
 void __mmu_notifier_invalidate_range_start(struct mm_struct *mm,
-					   unsigned long start,
-					   unsigned long end,
-					   enum mmu_event event)
+					   struct mmu_notifier_range *range)
 
 {
 	struct mmu_notifier *mn;
@@ -184,31 +182,83 @@ void __mmu_notifier_invalidate_range_start(struct mm_struct *mm,
 	id = srcu_read_lock(&srcu);
 	hlist_for_each_entry_rcu(mn, &mm->mmu_notifier_mm->list, hlist) {
 		if (mn->ops->invalidate_range_start)
-			mn->ops->invalidate_range_start(mn, mm, start,
-							end, event);
+			mn->ops->invalidate_range_start(mn, mm, range);
 	}
 	srcu_read_unlock(&srcu, id);
+
+	/*
+	 * This must happen after the callback so that subsystem can block on
+	 * new invalidation range to synchronize itself.
+	 */
+	spin_lock(&mm->mmu_notifier_mm->lock);
+	list_add_tail(&range->list, &mm->mmu_notifier_mm->ranges);
+	mm->mmu_notifier_mm->nranges++;
+	spin_unlock(&mm->mmu_notifier_mm->lock);
 }
 EXPORT_SYMBOL_GPL(__mmu_notifier_invalidate_range_start);
 
 void __mmu_notifier_invalidate_range_end(struct mm_struct *mm,
-					 unsigned long start,
-					 unsigned long end,
-					 enum mmu_event event)
+					 struct mmu_notifier_range *range)
 {
 	struct mmu_notifier *mn;
 	int id;
 
+	/*
+	 * This must happen before the callback so that subsystem can unblock
+	 * when range invalidation end.
+	 */
+	spin_lock(&mm->mmu_notifier_mm->lock);
+	list_del_init(&range->list);
+	mm->mmu_notifier_mm->nranges--;
+	spin_unlock(&mm->mmu_notifier_mm->lock);
+
 	id = srcu_read_lock(&srcu);
 	hlist_for_each_entry_rcu(mn, &mm->mmu_notifier_mm->list, hlist) {
 		if (mn->ops->invalidate_range_end)
-			mn->ops->invalidate_range_end(mn, mm, start,
-						      end, event);
+			mn->ops->invalidate_range_end(mn, mm, range);
 	}
 	srcu_read_unlock(&srcu, id);
+
+	/*
+	 * Wakeup after callback so they can do their job before any of the
+	 * waiters resume.
+	 */
+	wake_up(&mm->mmu_notifier_mm->wait_queue);
 }
 EXPORT_SYMBOL_GPL(__mmu_notifier_invalidate_range_end);
 
+bool mmu_notifier_range_is_valid(struct mm_struct *mm,
+				 unsigned long start,
+				 unsigned long end)
+{
+	struct mmu_notifier_range range;
+
+	spin_lock(&mm->mmu_notifier_mm->lock);
+	list_for_each_entry(range, &mm->mmu_notifier_mm->ranges, list) {
+		if (!(range->end <= start || range->start >= end)) {
+			spin_unlock(&mm->mmu_notifier_mm->lock);
+			return false;
+		}
+	}
+	spin_unlock(&mm->mmu_notifier_mm->lock);
+	return true;
+}
+EXPORT_SYMBOL_GPL(mmu_notifier_range_is_valid);
+
+void mmu_notifier_range_wait_valid(struct mm_struct *mm,
+				   unsigned long start,
+				   unsigned long end)
+{
+	int nranges = mm->mmu_notifier_mm->nranges;
+
+	while (!mmu_notifier_range_is_valid(mm, start, end)) {
+		wait_event(mm->mmu_notifier_mm->wait_queue,
+			   nranges != mm->mmu_notifier_mm->nranges);
+		nranges = mm->mmu_notifier_mm->nranges;
+	}
+}
+EXPORT_SYMBOL_GPL(mmu_notifier_range_wait_valid);
+
 static int do_mmu_notifier_register(struct mmu_notifier *mn,
 				    struct mm_struct *mm,
 				    int take_mmap_sem)
@@ -238,6 +288,9 @@ static int do_mmu_notifier_register(struct mmu_notifier *mn,
 	if (!mm_has_notifiers(mm)) {
 		INIT_HLIST_HEAD(&mmu_notifier_mm->list);
 		spin_lock_init(&mmu_notifier_mm->lock);
+		INIT_LIST_HEAD(&mmu_notifier_mm->ranges);
+		mmu_notifier_mm->nranges = 0;
+		init_waitqueue_head(&mmu_notifier_mm->wait_queue);
 
 		mm->mmu_notifier_mm = mmu_notifier_mm;
 		mmu_notifier_mm = NULL;
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 886405b..a178b22 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -144,7 +144,9 @@ static inline unsigned long change_pmd_range(struct vm_area_struct *vma,
 	unsigned long next;
 	unsigned long pages = 0;
 	unsigned long nr_huge_updates = 0;
-	unsigned long mni_start = 0;
+	struct mmu_notifier_range range = {
+		.start = 0,
+	};
 
 	pmd = pmd_offset(pud, addr);
 	do {
@@ -155,10 +157,11 @@ static inline unsigned long change_pmd_range(struct vm_area_struct *vma,
 			continue;
 
 		/* invoke the mmu notifier if the pmd is populated */
-		if (!mni_start) {
-			mni_start = addr;
-			mmu_notifier_invalidate_range_start(mm, mni_start,
-							    end, MMU_MPROT);
+		if (!range.start) {
+			range.start = addr;
+			range.end = end;
+			range.event = MMU_MPROT;
+			mmu_notifier_invalidate_range_start(mm, &range);
 		}
 
 		if (pmd_trans_huge(*pmd)) {
@@ -185,8 +188,8 @@ static inline unsigned long change_pmd_range(struct vm_area_struct *vma,
 		pages += this_pages;
 	} while (pmd++, addr = next, addr != end);
 
-	if (mni_start)
-		mmu_notifier_invalidate_range_end(mm, mni_start, end, MMU_MPROT);
+	if (range.start)
+		mmu_notifier_invalidate_range_end(mm, &range);
 
 	if (nr_huge_updates)
 		count_vm_numa_events(NUMA_HUGE_PTE_UPDATES, nr_huge_updates);
diff --git a/mm/mremap.c b/mm/mremap.c
index 6827d2f..83c5eed 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -167,18 +167,17 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 		bool need_rmap_locks)
 {
 	unsigned long extent, next, old_end;
+	struct mmu_notifier_range range;
 	pmd_t *old_pmd, *new_pmd;
 	bool need_flush = false;
-	unsigned long mmun_start;	/* For mmu_notifiers */
-	unsigned long mmun_end;		/* For mmu_notifiers */
 
 	old_end = old_addr + len;
 	flush_cache_range(vma, old_addr, old_end);
 
-	mmun_start = old_addr;
-	mmun_end   = old_end;
-	mmu_notifier_invalidate_range_start(vma->vm_mm, mmun_start,
-					    mmun_end, MMU_MIGRATE);
+	range.start = old_addr;
+	range.end = old_end;
+	range.event = MMU_MIGRATE;
+	mmu_notifier_invalidate_range_start(vma->vm_mm, &range);
 
 	for (; old_addr < old_end; old_addr += extent, new_addr += extent) {
 		cond_resched();
@@ -229,8 +228,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 	if (likely(need_flush))
 		flush_tlb_range(vma, old_end-len, old_addr);
 
-	mmu_notifier_invalidate_range_end(vma->vm_mm, mmun_start,
-					  mmun_end, MMU_MIGRATE);
+	mmu_notifier_invalidate_range_end(vma->vm_mm, &range);
 
 	return len + old_addr - old_end;	/* how much done */
 }
diff --git a/mm/rmap.c b/mm/rmap.c
index 0b67e7d..b8b8a60 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1302,15 +1302,14 @@ static int try_to_unmap_cluster(unsigned long cursor, unsigned int *mapcount,
 	spinlock_t *ptl;
 	struct page *page;
 	unsigned long address;
-	unsigned long mmun_start;	/* For mmu_notifiers */
-	unsigned long mmun_end;		/* For mmu_notifiers */
+	struct mmu_notifier_range range;
 	unsigned long end;
 	int ret = SWAP_AGAIN;
 	int locked_vma = 0;
-	enum mmu_event event = MMU_MIGRATE;
 
+	range.event = MMU_MIGRATE;
 	if (flags & TTU_MUNLOCK)
-		event = MMU_MUNLOCK;
+		range.event = MMU_MUNLOCK;
 
 	address = (vma->vm_start + cursor) & CLUSTER_MASK;
 	end = address + CLUSTER_SIZE;
@@ -1323,9 +1322,9 @@ static int try_to_unmap_cluster(unsigned long cursor, unsigned int *mapcount,
 	if (!pmd)
 		return ret;
 
-	mmun_start = address;
-	mmun_end   = end;
-	mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end, event);
+	range.start = address;
+	range.end = end;
+	mmu_notifier_invalidate_range_start(mm, &range);
 
 	/*
 	 * If we can acquire the mmap_sem for read, and vma is VM_LOCKED,
@@ -1390,7 +1389,7 @@ static int try_to_unmap_cluster(unsigned long cursor, unsigned int *mapcount,
 		(*mapcount)--;
 	}
 	pte_unmap_unlock(pte - 1, ptl);
-	mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end, event);
+	mmu_notifier_invalidate_range_end(mm, &range);
 	if (locked_vma)
 		up_read(&vma->vm_mm->mmap_sem);
 	return ret;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 0ed3e88..8d8c2ce 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -318,9 +318,7 @@ static void kvm_mmu_notifier_change_pte(struct mmu_notifier *mn,
 
 static void kvm_mmu_notifier_invalidate_range_start(struct mmu_notifier *mn,
 						    struct mm_struct *mm,
-						    unsigned long start,
-						    unsigned long end,
-						    enum mmu_event event)
+						    const struct mmu_notifier_range *range)
 {
 	struct kvm *kvm = mmu_notifier_to_kvm(mn);
 	int need_tlb_flush = 0, idx;
@@ -333,7 +331,7 @@ static void kvm_mmu_notifier_invalidate_range_start(struct mmu_notifier *mn,
 	 * count is also read inside the mmu_lock critical section.
 	 */
 	kvm->mmu_notifier_count++;
-	need_tlb_flush = kvm_unmap_hva_range(kvm, start, end);
+	need_tlb_flush = kvm_unmap_hva_range(kvm, range->start, range->end);
 	need_tlb_flush |= kvm->tlbs_dirty;
 	/* we've to flush the tlb before the pages can be freed */
 	if (need_tlb_flush)
@@ -345,9 +343,7 @@ static void kvm_mmu_notifier_invalidate_range_start(struct mmu_notifier *mn,
 
 static void kvm_mmu_notifier_invalidate_range_end(struct mmu_notifier *mn,
 						  struct mm_struct *mm,
-						  unsigned long start,
-						  unsigned long end,
-						  enum mmu_event event)
+						  const struct mmu_notifier_range *range)
 {
 	struct kvm *kvm = mmu_notifier_to_kvm(mn);
 
-- 
1.9.3

Re: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Jerome Glisse]

On Thu, Sep 11, 2014 at 11:32:56AM -0400, Jerome Glisse wrote:
> On Thu, Sep 11, 2014 at 12:19:01PM +0000, Shachar Raindel wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > > Sent: Wednesday, September 10, 2014 11:15 PM
> > > To: Shachar Raindel
> > > Cc: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Sagi Grimberg
> > > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support for
> > > MMU notifiers regarding on demand paging regions
> > > 
> > > On Wed, Sep 10, 2014 at 09:00:36AM +0000, Shachar Raindel wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > > > > Sent: Tuesday, September 09, 2014 6:37 PM
> > > > > To: Shachar Raindel
> > > > > Cc: 1404377069-20585-1-git-send-email-haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org; Haggai
> > > Eran;
> > > > > linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Jerome Glisse; Sagi Grimberg
> > > > > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support
> > > for
> > > > > MMU notifiers regarding on demand paging regions
> > > > >
> > > > > On Sun, Sep 07, 2014 at 02:35:59PM +0000, Shachar Raindel wrote:
> > > > > > Hi,
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Jerome Glisse [mailto:j.glisse-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
> > > > > > > Sent: Thursday, September 04, 2014 11:25 PM
> > > > > > > To: Haggai Eran; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > > > > Cc: Shachar Raindel; Sagi Grimberg
> > > > > > > Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement
> > > support
> > > > > for
> > > > > > > MMU notifiers regarding on demand paging regions
> > > > > > >
> > 
> > <SNIP>
> > 
> > > > > >
> > > > > > Sadly, taking mmap_sem in read-only mode does not prevent all
> > > possible
> > > > > invalidations from happening.
> > > > > > For example, a call to madvise requesting MADVISE_DONTNEED will
> > > lock
> > > > > the mmap_sem for reading only, allowing a notifier to run in
> > > parallel to
> > > > > the MR registration As a result, the following sequence of events
> > > could
> > > > > happen:
> > > > > >
> > > > > > Thread 1:                       |   Thread 2
> > > > > > --------------------------------+-------------------------
> > > > > > madvise                         |
> > > > > > down_read(mmap_sem)             |
> > > > > > notifier_start                  |
> > > > > >                                 |   down_read(mmap_sem)
> > > > > >                                 |   register_mr
> > > > > > notifier_end                    |
> > > > > > reduce_mr_notifiers_count       |
> > > > > >
> > > > > > The end result of this sequence is an mr with running notifiers
> > > count
> > > > > of -1, which is bad.
> > > > > > The current workaround is to avoid decreasing the notifiers count
> > > if
> > > > > it is zero, which can cause other issues.
> > > > > > The proper fix would be to prevent notifiers from running in
> > > parallel
> > > > > to registration. For this, taking mmap_sem in write mode might be
> > > > > sufficient, but we are not sure about this.
> > > > > > We will be happy to hear additional input on this subject, to make
> > > > > sure we got it covered properly.
> > > > >
> > > > > So in HMM i solve this by having a struct allocated in the start
> > > range
> > > > > callback
> > > > > and the end range callback just ignore things when it can not find
> > > the
> > > > > matching
> > > > > struct.
> > > >
> > > > This kind of mechanism sounds like it has a bigger risk for
> > > deadlocking
> > > > the system, causing an OOM kill without a real need or significantly
> > > > slowing down the system.
> > > > If you are doing non-atomic memory allocations, you can deadlock the
> > > > system by requesting memory in the swapper flow.
> > > > Even if you are doing atomic memory allocations, you need to handle
> > > the
> > > > case of failing allocation, the solution to which is unclear to me.
> > > > If you are using a pre-allocated pool, what are you doing when you run
> > > > out of available entries in the pool? If you are blocking until some
> > > > entries free up, what guarantees you that this will not cause a
> > > deadlock?
> > > 
> > > So i am using a fixed pool and when it runs out it block in start
> > > callback
> > > until one is freed. 
> > 
> > This sounds scary. You now create a possible locking dependency between
> > two code flows which could have run in parallel. This can cause circular
> > locking bugs, from code which functioned properly until now. For example,
> > assume code with a single lock, and the following code paths:
> > 
> > Code 1:
> > notify_start()
> > lock()
> > unlock()
> > notify_end()
> > 
> > Code 2:
> > lock()
> > notify_start()
> > ... (no locking)
> > notify_end()
> > unlock()
> > 
> 
> This can not happen because all lock taken before notify_start() are
> never taken after it and all lock taken inside a start/end section
> are never hold accross a notify_start() callback.
> 
> > 
> > 
> > This code can now create the following deadlock:
> > 
> > Thread 1:        | Thread 2:
> > -----------------+-----------------------------------
> > notify_start()   |
> >                  | lock()
> > lock() - blocking|
> >                  | notify_start() - blocking for slot
> > 
> > 
> > 
> > 
> > > But as i said i have a patch to use the stack that
> > > will
> > > solve this and avoid a pool.
> > 
> > How are you allocating from the stack an entry which you need to keep alive
> > until another function is called? You can't allocate the entry on the
> > notify_start stack, so you must do this in all of the call points to the
> > mmu_notifiers. Given the notifiers listener subscription pattern, this seems
> > like something which is not practical.
> 
> Yes the patch add a struct in each callsite of mmu_notifier_invalidate_range
> as in all case both start and end are call from same function. The only draw
> back is that it increase stack consumption in some of those callsite (not all).
> I attach the patch i am thinking of (it is untested) but idea is that through
> two new helper function user of mmu_notifier can query active invalid range and
> synchronize with those (also require some code in the range_start() callback).
> 
> > 
> >  
> > > 
> > > >
> > > > >
> > > > > That being said when registering the mmu_notifier you need 2 things,
> > > > > first you
> > > > > need a pin on the mm (either mm is current ie current->mm or you
> > > took a
> > > > > reference
> > > > > on it). Second you need to that the mmap smemaphore in write mode so
> > > > > that
> > > > > no concurrent mmap/munmap/madvise can happen. By doing that you
> > > protect
> > > > > yourself
> > > > > from concurrent range_start/range_end that can happen and that does
> > > > > matter.
> > > > > The only concurrent range_start/end that can happen is through file
> > > > > invalidation
> > > > > which is fine because subsequent page fault will go through the file
> > > > > layer and
> > > > > bring back page or return error (if file was truncated for
> > > instance).
> > > >
> > > > Sadly, this is not sufficient for our use case. We are registering
> > > > a single MMU notifier handler, and broadcast the notifications to
> > > > all relevant listeners, which are stored in an interval tree.
> > > >
> > > > Each listener represents a section of the address space that has been
> > > > exposed to the network. Such implementation allows us to limit the
> > > impact
> > > > of invalidations, and only block racing page faults to the affected
> > > areas.
> > > >
> > > > Each of the listeners maintain a counter of the number of
> > > invalidate_range
> > > > notifications that are currently affecting it. The counter is
> > > increased
> > > > for each invalidate_range_start callback received, and decrease for
> > > each
> > > > invalidate_range_end callback received. If we add a listener to the
> > > > interval tree after the invalidate_range_start callback happened, but
> > > > before the invalidate_range_end callback happened, it will decrease
> > > the
> > > > counter, reaching negative numbers and breaking the logic.
> > > >
> > > > The mmu_notifiers registration code avoid such issues by taking all
> > > > relevant locks on the MM. This effectively blocks all possible
> > > notifiers
> > > > from happening when registering a new notifier. Sadly, this function
> > > is
> > > > not exported for modules to use it.
> > > >
> > > > Our options at the moment are:
> > > > - Use a tracking mechanism similar to what HMM uses, alongside the
> > > >   challenges involved in allocating memory from notifiers
> > > >
> > > > - Use a per-process counter for invalidations, causing a possible
> > > >   performance degradation. This can possibly be used as a fallback to
> > > the
> > > >   first option (i.e. have a pool of X notifier identifiers, once it is
> > > >   full, increase/decrease a per-MM counter)
> > > >
> > > > - Export the mm_take_all_locks function for modules. This will allow
> > > us
> > > >   to lock the MM when adding a new listener.
> > > 
> > > I was not clear enough, you need to take the mmap_sem in write mode
> > > accross
> > > mmu_notifier_register(). This is only to partialy solve your issue that
> > > if
> > > a mmu_notifier is already register for the mm you are trying to
> > > registering
> > > against then there is a chance for you to be inside an active
> > > range_start/
> > > range_end section which would lead to invalid counter inside your
> > > tracking
> > > structure. But, sadly, taking mmap_sem in write mode is not enough as
> > > file
> > > invalidation might still happen concurrently so you will need to make
> > > sure
> > > you invalidation counters does not go negative but from page fault point
> > > of
> > > view you will be fine because the page fault will synchronize through
> > > the
> > > pagecache. So scenario (A and B are to anonymous overlapping address
> > > range) :
> > > 
> > >   APP_TOTO_RDMA_THREAD           |  APP_TOTO_SOME_OTHER_THREAD
> > >                                  |  mmu_notifier_invalidate_range_start(A)
> > >   odp_register()                 |
> > >     down_read(mmap_sem)          |
> > >     mmu_notifier_register()      |
> > >     up_read(mmap_sem)            |
> > >   odp_add_new_region(B)          |
> > >   odp_page_fault(B)              |
> > >     down_read(mmap_sem)          |
> > >     ...                          |
> > >     up_read(mmap_sem)            |
> > >                                  |  mmu_notifier_invalidate_range_end(A)
> > > 
> > > The odp_page_fault(B) might see invalid cpu page table but you have no
> > > idea
> > > about it because you registered after the range_start(). But if you take
> > > the
> > > mmap_sem in write mode then the only case where you might still have
> > > this
> > > scenario is if A and B are range of a file backed vma and that the file
> > > is
> > > undergoing some change (most likely truncation). But the file case is
> > > fine
> > > because the odp_page_fault() will go through the pagecache which is
> > > properly
> > > synchronize against the current range invalidation.
> > 
> > Specifically, if you call mmu_notifier_register you are OK and the above
> > scenario will not happen. You are supposed to hold mmap_sem for writing,
> > and mmu_notifier_register is calling mm_take_all_locks, which guarantees
> > no racing notifier during the registration step.
> > 
> > However, we want to dynamically add sub-notifiers in our code. Each will
> > get notified only about invalidations touching a specific sub-sections of
> > the address space. To avoid providing unneeded notifications, we use an
> > interval tree that filters only the needed notifications.
> > When adding entries to the interval tree, we cannot lock the mm to prevent
> > any racing invalidations. As such, we might end up in a case where a newly
> > registered memory region will get a "notify_end" call without the relevant
> > "notify_start". Even if we prevent the value from dropping below zero, it
> > means we can cause data corruption. For example, if we have another
> > notifier running after the MR registers, which is due to munmap, but we get
> > first the notify_end of the previous notifier for which we didn't see the
> > notify_start.
> > 
> > The solution we are coming up with now is using a global counter of running
> > invalidations for new regions allocated. When the global counter is at zero,
> > we can safely switch to the region local invalidations counter.
> 
> Yes i fully understood that design but as i said this kind of broken and this
> is what the attached patch try to address as HMM have the same issue of having
> to track all active invalidation range.

I should also stress that my point was that you need mmap_sem in write mode while
registering specificaly because otherwise there is a risk that your global mmu
notifier counter is missing a running invalidate range and thus there is a window
for a one of your new struct that mirror a range to be registered and to use
invalid pages (pages that are about to be freed). So this is very important to
hold the mmap_sem in write mode while you are registering and before you allow
any of your region to be register.

As i said i was not talking about the general case after registering the mmu
notifier.

> 
> > 
> > 
> > > 
> > > 
> > > Now for the the general case outside of mmu_notifier_register() HMM also
> > > track
> > > active invalidation range to avoid page faulting into those range as we
> > > can not
> > > trust the cpu page table for as long as the range invalidation is on
> > > going.
> > > 
> > > > >
> > > > > So as long as you hold the mmap_sem in write mode you should not
> > > worry
> > > > > about
> > > > > concurrent range_start/range_end (well they might happen but only
> > > for
> > > > > file
> > > > > backed vma).
> > > > >
> > > >
> > > > Sadly, the mmap_sem is not enough to protect us :(.
> > > 
> > > This is enough like i explain above, but i am only talking about the mmu
> > > notifier registration. For the general case once you register you only
> > > need to take the mmap_sem in read mode during page fault.
> > > 
> > 
> > I think we are not broadcasting on the same wavelength here. The issue I'm
> > worried about is of adding a sub-area to our tracking system. It is built
> > quite differently from how HMM is built, we are defining areas to track
> > a-priori, and later on account how many notifiers are blocking page-faults
> > for each area. You are keeping track of the active notifiers, and check
> > each page fault against your notifier list. This difference makes for
> > different locking needs.
> > 
> > > > > Given that you face the same issue as i have with the
> > > > > range_start/range_end i
> > > > > will stich up a patch to make it easier to track those.
> > > > >
> > > >
> > > > That would be nice, especially if we could easily integrate it into
> > > our
> > > > code and reduce the code size.
> > > 
> > > Yes it's a "small modification" to the mmu_notifier api, i have been
> > > side
> > > tracked on other thing. But i will have it soon.
> > > 
> > 
> > Being side tracked is a well-known professional risk in our line of work ;)
> > 
> > 
> > > >
> > > > > Cheers,
> > > > > Jérôme
> > > > >
> > > > >

> From 037195e49fbed468d16b78f0364fe302bc732d12 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com>
> Date: Thu, 11 Sep 2014 11:22:12 -0400
> Subject: [PATCH] mmu_notifier: keep track of active invalidation ranges
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> The mmu_notifier_invalidate_range_start() and mmu_notifier_invalidate_range_end()
> can be considered as forming an "atomic" section for the cpu page table update
> point of view. Between this two function the cpu page table content is unreliable
> for the affected range of address.
> 
> Current user such as kvm need to know when they can trust a the content of the
> cpu page table. This becomes even more important to new users of the mmu_notifier
> api (such as HMM or ODP).
> 
> This patch use a structure define at all call site to invalidate_range_start()
> that is added to a list for the duration of the invalidation. It adds two new
> helpers to allow querying if a range is being invalidated or to wait for a range
> to become valid.
> 
> This two new function does not provide strong synchronization but are intended
> to be use as helper. User of the mmu_notifier must also synchronize with themself
> inside their range_start() and range_end() callback.
> 
> Signed-off-by: Jérôme Glisse <jglisse-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/gpu/drm/i915/i915_gem_userptr.c | 13 +++---
>  drivers/iommu/amd_iommu_v2.c            |  8 +---
>  drivers/misc/sgi-gru/grutlbpurge.c      | 15 +++----
>  drivers/xen/gntdev.c                    |  8 ++--
>  fs/proc/task_mmu.c                      | 12 +++--
>  include/linux/mmu_notifier.h            | 55 ++++++++++++-----------
>  mm/fremap.c                             |  8 +++-
>  mm/huge_memory.c                        | 78 ++++++++++++++-------------------
>  mm/hugetlb.c                            | 49 +++++++++++----------
>  mm/memory.c                             | 73 ++++++++++++++++--------------
>  mm/migrate.c                            | 16 +++----
>  mm/mmu_notifier.c                       | 73 +++++++++++++++++++++++++-----
>  mm/mprotect.c                           | 17 ++++---
>  mm/mremap.c                             | 14 +++---
>  mm/rmap.c                               | 15 +++----
>  virt/kvm/kvm_main.c                     | 10 ++---
>  16 files changed, 256 insertions(+), 208 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index a13307d..373ffbb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -123,26 +123,25 @@ restart:
>  
>  static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
>  						       struct mm_struct *mm,
> -						       unsigned long start,
> -						       unsigned long end,
> -						       enum mmu_event event)
> +						       const struct mmu_notifier_range *range)
>  {
>  	struct i915_mmu_notifier *mn = container_of(_mn, struct i915_mmu_notifier, mn);
>  	struct interval_tree_node *it = NULL;
> -	unsigned long next = start;
> +	unsigned long next = range->start;
>  	unsigned long serial = 0;
> +	/* interval ranges are inclusive, but invalidate range is exclusive */
> +	unsigned long end = range.end - 1;
>  
> -	end--; /* interval ranges are inclusive, but invalidate range is exclusive */
>  	while (next < end) {
>  		struct drm_i915_gem_object *obj = NULL;
>  
>  		spin_lock(&mn->lock);
>  		if (mn->has_linear)
> -			it = invalidate_range__linear(mn, mm, start, end);
> +			it = invalidate_range__linear(mn, mm, range->start, end);
>  		else if (serial == mn->serial)
>  			it = interval_tree_iter_next(it, next, end);
>  		else
> -			it = interval_tree_iter_first(&mn->objects, start, end);
> +			it = interval_tree_iter_first(&mn->objects, range->start, end);
>  		if (it != NULL) {
>  			obj = container_of(it, struct i915_mmu_object, it)->obj;
>  			drm_gem_object_reference(&obj->base);
> diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
> index 9a6b837..5945300 100644
> --- a/drivers/iommu/amd_iommu_v2.c
> +++ b/drivers/iommu/amd_iommu_v2.c
> @@ -419,9 +419,7 @@ static void mn_invalidate_page(struct mmu_notifier *mn,
>  
>  static void mn_invalidate_range_start(struct mmu_notifier *mn,
>  				      struct mm_struct *mm,
> -				      unsigned long start,
> -				      unsigned long end,
> -				      enum mmu_event event)
> +				      const struct mmu_notifier_range *range)
>  {
>  	struct pasid_state *pasid_state;
>  	struct device_state *dev_state;
> @@ -442,9 +440,7 @@ static void mn_invalidate_range_start(struct mmu_notifier *mn,
>  
>  static void mn_invalidate_range_end(struct mmu_notifier *mn,
>  				    struct mm_struct *mm,
> -				    unsigned long start,
> -				    unsigned long end,
> -				    enum mmu_event event)
> +				    const struct mmu_notifier_range *range)
>  {
>  	struct pasid_state *pasid_state;
>  	struct device_state *dev_state;
> diff --git a/drivers/misc/sgi-gru/grutlbpurge.c b/drivers/misc/sgi-gru/grutlbpurge.c
> index e67fed1..44b41b7 100644
> --- a/drivers/misc/sgi-gru/grutlbpurge.c
> +++ b/drivers/misc/sgi-gru/grutlbpurge.c
> @@ -221,8 +221,7 @@ void gru_flush_all_tlb(struct gru_state *gru)
>   */
>  static void gru_invalidate_range_start(struct mmu_notifier *mn,
>  				       struct mm_struct *mm,
> -				       unsigned long start, unsigned long end,
> -				       enum mmu_event event)
> +				       const struct mmu_notifier_range *range)
>  {
>  	struct gru_mm_struct *gms = container_of(mn, struct gru_mm_struct,
>  						 ms_notifier);
> @@ -230,14 +229,13 @@ static void gru_invalidate_range_start(struct mmu_notifier *mn,
>  	STAT(mmu_invalidate_range);
>  	atomic_inc(&gms->ms_range_active);
>  	gru_dbg(grudev, "gms %p, start 0x%lx, end 0x%lx, act %d\n", gms,
> -		start, end, atomic_read(&gms->ms_range_active));
> -	gru_flush_tlb_range(gms, start, end - start);
> +		range->start, range->end, atomic_read(&gms->ms_range_active));
> +	gru_flush_tlb_range(gms, range->start, range->end - range->start);
>  }
>  
>  static void gru_invalidate_range_end(struct mmu_notifier *mn,
> -				     struct mm_struct *mm, unsigned long start,
> -				     unsigned long end,
> -				     enum mmu_event event)
> +				     struct mm_struct *mm,
> +				     const struct mmu_notifier_range *range)
>  {
>  	struct gru_mm_struct *gms = container_of(mn, struct gru_mm_struct,
>  						 ms_notifier);
> @@ -246,7 +244,8 @@ static void gru_invalidate_range_end(struct mmu_notifier *mn,
>  	(void)atomic_dec_and_test(&gms->ms_range_active);
>  
>  	wake_up_all(&gms->ms_wait_queue);
> -	gru_dbg(grudev, "gms %p, start 0x%lx, end 0x%lx\n", gms, start, end);
> +	gru_dbg(grudev, "gms %p, start 0x%lx, end 0x%lx\n", gms,
> +		range->start, range->end);
>  }
>  
>  static void gru_invalidate_page(struct mmu_notifier *mn, struct mm_struct *mm,
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index fe9da94..51f9188 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -428,19 +428,17 @@ static void unmap_if_in_range(struct grant_map *map,
>  
>  static void mn_invl_range_start(struct mmu_notifier *mn,
>  				struct mm_struct *mm,
> -				unsigned long start,
> -				unsigned long end,
> -				enum mmu_event event)
> +				const struct mmu_notifier_range *range)
>  {
>  	struct gntdev_priv *priv = container_of(mn, struct gntdev_priv, mn);
>  	struct grant_map *map;
>  
>  	spin_lock(&priv->lock);
>  	list_for_each_entry(map, &priv->maps, next) {
> -		unmap_if_in_range(map, start, end);
> +		unmap_if_in_range(map, range->start, range->end);
>  	}
>  	list_for_each_entry(map, &priv->freeable_maps, next) {
> -		unmap_if_in_range(map, start, end);
> +		unmap_if_in_range(map, range->start, range->end);
>  	}
>  	spin_unlock(&priv->lock);
>  }
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 0ddb975..532a230 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -828,10 +828,15 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
>  			.mm = mm,
>  			.private = &cp,
>  		};
> +		struct mmu_notifier_range range = {
> +			.start = 0,
> +			.end = -1UL,
> +			.event = MMU_ISDIRTY,
> +		};
> +
>  		down_read(&mm->mmap_sem);
>  		if (type == CLEAR_REFS_SOFT_DIRTY)
> -			mmu_notifier_invalidate_range_start(mm, 0,
> -							    -1, MMU_ISDIRTY);
> +			mmu_notifier_invalidate_range_start(mm, &range);
>  		for (vma = mm->mmap; vma; vma = vma->vm_next) {
>  			cp.vma = vma;
>  			if (is_vm_hugetlb_page(vma))
> @@ -859,8 +864,7 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
>  					&clear_refs_walk);
>  		}
>  		if (type == CLEAR_REFS_SOFT_DIRTY)
> -			mmu_notifier_invalidate_range_end(mm, 0,
> -							  -1, MMU_ISDIRTY);
> +			mmu_notifier_invalidate_range_end(mm, &range);
>  		flush_tlb_mm(mm);
>  		up_read(&mm->mmap_sem);
>  		mmput(mm);
> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
> index 94f6890..f4a2a74 100644
> --- a/include/linux/mmu_notifier.h
> +++ b/include/linux/mmu_notifier.h
> @@ -69,6 +69,13 @@ enum mmu_event {
>  	MMU_WRITE_PROTECT,
>  };
>  
> +struct mmu_notifier_range {
> +	struct list_head list;
> +	unsigned long start;
> +	unsigned long end;
> +	enum mmu_event event;
> +};
> +
>  #ifdef CONFIG_MMU_NOTIFIER
>  
>  /*
> @@ -82,6 +89,12 @@ struct mmu_notifier_mm {
>  	struct hlist_head list;
>  	/* to serialize the list modifications and hlist_unhashed */
>  	spinlock_t lock;
> +	/* List of all active range invalidations. */
> +	struct list_head ranges;
> +	/* Number of active range invalidations. */
> +	int nranges;
> +	/* For threads waiting on range invalidations. */
> +	wait_queue_head_t wait_queue;
>  };
>  
>  struct mmu_notifier_ops {
> @@ -199,14 +212,10 @@ struct mmu_notifier_ops {
>  	 */
>  	void (*invalidate_range_start)(struct mmu_notifier *mn,
>  				       struct mm_struct *mm,
> -				       unsigned long start,
> -				       unsigned long end,
> -				       enum mmu_event event);
> +				       const struct mmu_notifier_range *range);
>  	void (*invalidate_range_end)(struct mmu_notifier *mn,
>  				     struct mm_struct *mm,
> -				     unsigned long start,
> -				     unsigned long end,
> -				     enum mmu_event event);
> +				     const struct mmu_notifier_range *range);
>  };
>  
>  /*
> @@ -252,13 +261,15 @@ extern void __mmu_notifier_invalidate_page(struct mm_struct *mm,
>  					  unsigned long address,
>  					  enum mmu_event event);
>  extern void __mmu_notifier_invalidate_range_start(struct mm_struct *mm,
> -						  unsigned long start,
> -						  unsigned long end,
> -						  enum mmu_event event);
> +						  struct mmu_notifier_range *range);
>  extern void __mmu_notifier_invalidate_range_end(struct mm_struct *mm,
> -						unsigned long start,
> -						unsigned long end,
> -						enum mmu_event event);
> +						struct mmu_notifier_range *range);
> +extern bool mmu_notifier_range_is_valid(struct mm_struct *mm,
> +					unsigned long start,
> +					unsigned long end);
> +extern void mmu_notifier_range_wait_valid(struct mm_struct *mm,
> +					  unsigned long start,
> +					  unsigned long end);
>  
>  static inline void mmu_notifier_release(struct mm_struct *mm)
>  {
> @@ -300,21 +311,17 @@ static inline void mmu_notifier_invalidate_page(struct mm_struct *mm,
>  }
>  
>  static inline void mmu_notifier_invalidate_range_start(struct mm_struct *mm,
> -						       unsigned long start,
> -						       unsigned long end,
> -						       enum mmu_event event)
> +						       struct mmu_notifier_range *range)
>  {
>  	if (mm_has_notifiers(mm))
> -		__mmu_notifier_invalidate_range_start(mm, start, end, event);
> +		__mmu_notifier_invalidate_range_start(mm, range);
>  }
>  
>  static inline void mmu_notifier_invalidate_range_end(struct mm_struct *mm,
> -						     unsigned long start,
> -						     unsigned long end,
> -						     enum mmu_event event)
> +						     struct mmu_notifier_range *range)
>  {
>  	if (mm_has_notifiers(mm))
> -		__mmu_notifier_invalidate_range_end(mm, start, end, event);
> +		__mmu_notifier_invalidate_range_end(mm, range);
>  }
>  
>  static inline void mmu_notifier_mm_init(struct mm_struct *mm)
> @@ -406,16 +413,12 @@ static inline void mmu_notifier_invalidate_page(struct mm_struct *mm,
>  }
>  
>  static inline void mmu_notifier_invalidate_range_start(struct mm_struct *mm,
> -						       unsigned long start,
> -						       unsigned long end,
> -						       enum mmu_event event)
> +						       struct mmu_notifier_range *range)
>  {
>  }
>  
>  static inline void mmu_notifier_invalidate_range_end(struct mm_struct *mm,
> -						     unsigned long start,
> -						     unsigned long end,
> -						     enum mmu_event event)
> +						     struct mmu_notifier_range *range)
>  {
>  }
>  
> diff --git a/mm/fremap.c b/mm/fremap.c
> index 37b2904..03a5ddc 100644
> --- a/mm/fremap.c
> +++ b/mm/fremap.c
> @@ -148,6 +148,7 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
>  	int err = -EINVAL;
>  	int has_write_lock = 0;
>  	vm_flags_t vm_flags = 0;
> +	struct mmu_notifier_range range;
>  
>  	pr_warn_once("%s (%d) uses deprecated remap_file_pages() syscall. "
>  			"See Documentation/vm/remap_file_pages.txt.\n",
> @@ -258,9 +259,12 @@ get_write_lock:
>  		vma->vm_flags = vm_flags;
>  	}
>  
> -	mmu_notifier_invalidate_range_start(mm, start, start + size, MMU_MUNMAP);
> +	range.start = start;
> +	range.end = start + size;
> +	range.event = MMU_MUNMAP;
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  	err = vma->vm_ops->remap_pages(vma, start, size, pgoff);
> -	mmu_notifier_invalidate_range_end(mm, start, start + size, MMU_MUNMAP);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  
>  	/*
>  	 * We can't clear VM_NONLINEAR because we'd have to do
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index e3efba5..4b116dd 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -988,8 +988,7 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct *mm,
>  	pmd_t _pmd;
>  	int ret = 0, i;
>  	struct page **pages;
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  
>  	pages = kmalloc(sizeof(struct page *) * HPAGE_PMD_NR,
>  			GFP_KERNEL);
> @@ -1027,10 +1026,10 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct *mm,
>  		cond_resched();
>  	}
>  
> -	mmun_start = haddr;
> -	mmun_end   = haddr + HPAGE_PMD_SIZE;
> -	mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end,
> -					    MMU_MIGRATE);
> +	range.start = haddr;
> +	range.end = haddr + HPAGE_PMD_SIZE;
> +	range.event = MMU_MIGRATE;
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  
>  	ptl = pmd_lock(mm, pmd);
>  	if (unlikely(!pmd_same(*pmd, orig_pmd)))
> @@ -1064,8 +1063,7 @@ static int do_huge_pmd_wp_page_fallback(struct mm_struct *mm,
>  	page_remove_rmap(page);
>  	spin_unlock(ptl);
>  
> -	mmu_notifier_invalidate_range_end(mm, mmun_start,
> -					  mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  
>  	ret |= VM_FAULT_WRITE;
>  	put_page(page);
> @@ -1075,8 +1073,7 @@ out:
>  
>  out_free_pages:
>  	spin_unlock(ptl);
> -	mmu_notifier_invalidate_range_end(mm, mmun_start,
> -					  mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  	for (i = 0; i < HPAGE_PMD_NR; i++) {
>  		memcg = (void *)page_private(pages[i]);
>  		set_page_private(pages[i], 0);
> @@ -1095,8 +1092,7 @@ int do_huge_pmd_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
>  	struct page *page = NULL, *new_page;
>  	struct mem_cgroup *memcg;
>  	unsigned long haddr;
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  
>  	ptl = pmd_lockptr(mm, pmd);
>  	VM_BUG_ON(!vma->anon_vma);
> @@ -1166,10 +1162,10 @@ alloc:
>  		copy_user_huge_page(new_page, page, haddr, vma, HPAGE_PMD_NR);
>  	__SetPageUptodate(new_page);
>  
> -	mmun_start = haddr;
> -	mmun_end   = haddr + HPAGE_PMD_SIZE;
> -	mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end,
> -					    MMU_MIGRATE);
> +	range.start = haddr;
> +	range.end = haddr + HPAGE_PMD_SIZE;
> +	range.event = MMU_MIGRATE;
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  
>  	spin_lock(ptl);
>  	if (page)
> @@ -1201,8 +1197,7 @@ alloc:
>  	}
>  	spin_unlock(ptl);
>  out_mn:
> -	mmu_notifier_invalidate_range_end(mm, mmun_start,
> -					  mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  out:
>  	return ret;
>  out_unlock:
> @@ -1633,12 +1628,12 @@ static int __split_huge_page_splitting(struct page *page,
>  	spinlock_t *ptl;
>  	pmd_t *pmd;
>  	int ret = 0;
> -	/* For mmu_notifiers */
> -	const unsigned long mmun_start = address;
> -	const unsigned long mmun_end   = address + HPAGE_PMD_SIZE;
> +	struct mmu_notifier_range range;
>  
> -	mmu_notifier_invalidate_range_start(mm, mmun_start,
> -					    mmun_end, MMU_HSPLIT);
> +	range.start = address;
> +	range.end = address + HPAGE_PMD_SIZE;
> +	range.event = MMU_HSPLIT;
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  	pmd = page_check_address_pmd(page, mm, address,
>  			PAGE_CHECK_ADDRESS_PMD_NOTSPLITTING_FLAG, &ptl);
>  	if (pmd) {
> @@ -1653,8 +1648,7 @@ static int __split_huge_page_splitting(struct page *page,
>  		ret = 1;
>  		spin_unlock(ptl);
>  	}
> -	mmu_notifier_invalidate_range_end(mm, mmun_start,
> -					  mmun_end, MMU_HSPLIT);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  
>  	return ret;
>  }
> @@ -2434,8 +2428,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>  	int isolated;
>  	unsigned long hstart, hend;
>  	struct mem_cgroup *memcg;
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  
>  	VM_BUG_ON(address & ~HPAGE_PMD_MASK);
>  
> @@ -2475,10 +2468,10 @@ static void collapse_huge_page(struct mm_struct *mm,
>  	pte = pte_offset_map(pmd, address);
>  	pte_ptl = pte_lockptr(mm, pmd);
>  
> -	mmun_start = address;
> -	mmun_end   = address + HPAGE_PMD_SIZE;
> -	mmu_notifier_invalidate_range_start(mm, mmun_start,
> -					    mmun_end, MMU_MIGRATE);
> +	range.start = address;
> +	range.end = address + HPAGE_PMD_SIZE;
> +	range.event = MMU_MIGRATE;
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  	pmd_ptl = pmd_lock(mm, pmd); /* probably unnecessary */
>  	/*
>  	 * After this gup_fast can't run anymore. This also removes
> @@ -2488,8 +2481,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>  	 */
>  	_pmd = pmdp_clear_flush(vma, address, pmd);
>  	spin_unlock(pmd_ptl);
> -	mmu_notifier_invalidate_range_end(mm, mmun_start,
> -					  mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  
>  	spin_lock(pte_ptl);
>  	isolated = __collapse_huge_page_isolate(vma, address, pte);
> @@ -2872,36 +2864,32 @@ void __split_huge_page_pmd(struct vm_area_struct *vma, unsigned long address,
>  	struct page *page;
>  	struct mm_struct *mm = vma->vm_mm;
>  	unsigned long haddr = address & HPAGE_PMD_MASK;
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  
>  	BUG_ON(vma->vm_start > haddr || vma->vm_end < haddr + HPAGE_PMD_SIZE);
>  
> -	mmun_start = haddr;
> -	mmun_end   = haddr + HPAGE_PMD_SIZE;
> +	range.start = haddr;
> +	range.end = haddr + HPAGE_PMD_SIZE;
> +	range.event = MMU_MIGRATE;
>  again:
> -	mmu_notifier_invalidate_range_start(mm, mmun_start,
> -					    mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  	ptl = pmd_lock(mm, pmd);
>  	if (unlikely(!pmd_trans_huge(*pmd))) {
>  		spin_unlock(ptl);
> -		mmu_notifier_invalidate_range_end(mm, mmun_start,
> -						  mmun_end, MMU_MIGRATE);
> +		mmu_notifier_invalidate_range_end(mm, &range);
>  		return;
>  	}
>  	if (is_huge_zero_pmd(*pmd)) {
>  		__split_huge_zero_page_pmd(vma, haddr, pmd);
>  		spin_unlock(ptl);
> -		mmu_notifier_invalidate_range_end(mm, mmun_start,
> -						  mmun_end, MMU_MIGRATE);
> +		mmu_notifier_invalidate_range_end(mm, &range);
>  		return;
>  	}
>  	page = pmd_page(*pmd);
>  	VM_BUG_ON_PAGE(!page_count(page), page);
>  	get_page(page);
>  	spin_unlock(ptl);
> -	mmu_notifier_invalidate_range_end(mm, mmun_start,
> -					  mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  
>  	split_huge_page(page);
>  
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index ae98b53..6484793 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2551,17 +2551,16 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
>  	int cow;
>  	struct hstate *h = hstate_vma(vma);
>  	unsigned long sz = huge_page_size(h);
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  	int ret = 0;
>  
>  	cow = (vma->vm_flags & (VM_SHARED | VM_MAYWRITE)) == VM_MAYWRITE;
>  
> -	mmun_start = vma->vm_start;
> -	mmun_end = vma->vm_end;
> +	range.start = vma->vm_start;
> +	range.end = vma->vm_end;
> +	range.event = MMU_MIGRATE;
>  	if (cow)
> -		mmu_notifier_invalidate_range_start(src, mmun_start,
> -						    mmun_end, MMU_MIGRATE);
> +		mmu_notifier_invalidate_range_start(src, &range);
>  
>  	for (addr = vma->vm_start; addr < vma->vm_end; addr += sz) {
>  		spinlock_t *src_ptl, *dst_ptl;
> @@ -2612,8 +2611,7 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
>  	}
>  
>  	if (cow)
> -		mmu_notifier_invalidate_range_end(src, mmun_start,
> -						  mmun_end, MMU_MIGRATE);
> +		mmu_notifier_invalidate_range_end(src, &range);
>  
>  	return ret;
>  }
> @@ -2631,16 +2629,17 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
>  	struct page *page;
>  	struct hstate *h = hstate_vma(vma);
>  	unsigned long sz = huge_page_size(h);
> -	const unsigned long mmun_start = start;	/* For mmu_notifiers */
> -	const unsigned long mmun_end   = end;	/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  
>  	WARN_ON(!is_vm_hugetlb_page(vma));
>  	BUG_ON(start & ~huge_page_mask(h));
>  	BUG_ON(end & ~huge_page_mask(h));
>  
> +	range.start = start;
> +	range.end = end;
> +	range.event = MMU_MIGRATE;
>  	tlb_start_vma(tlb, vma);
> -	mmu_notifier_invalidate_range_start(mm, mmun_start,
> -					    mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  again:
>  	for (address = start; address < end; address += sz) {
>  		ptep = huge_pte_offset(mm, address);
> @@ -2711,8 +2710,7 @@ unlock:
>  		if (address < end && !ref_page)
>  			goto again;
>  	}
> -	mmu_notifier_invalidate_range_end(mm, mmun_start,
> -					  mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  	tlb_end_vma(tlb, vma);
>  }
>  
> @@ -2809,8 +2807,7 @@ static int hugetlb_cow(struct mm_struct *mm, struct vm_area_struct *vma,
>  	struct hstate *h = hstate_vma(vma);
>  	struct page *old_page, *new_page;
>  	int ret = 0, outside_reserve = 0;
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  
>  	old_page = pte_page(pte);
>  
> @@ -2888,10 +2885,11 @@ retry_avoidcopy:
>  			    pages_per_huge_page(h));
>  	__SetPageUptodate(new_page);
>  
> -	mmun_start = address & huge_page_mask(h);
> -	mmun_end = mmun_start + huge_page_size(h);
> -	mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end,
> -					    MMU_MIGRATE);
> +	range.start = address;
> +	range.end = address + huge_page_size(h);
> +	range.event = MMU_MIGRATE;
> +	mmu_notifier_invalidate_range_start(mm, &range);
> +
>  	/*
>  	 * Retake the page table lock to check for racing updates
>  	 * before the page tables are altered
> @@ -2911,8 +2909,7 @@ retry_avoidcopy:
>  		new_page = old_page;
>  	}
>  	spin_unlock(ptl);
> -	mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end,
> -					  MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  out_release_all:
>  	page_cache_release(new_page);
>  out_release_old:
> @@ -3346,11 +3343,15 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
>  	pte_t pte;
>  	struct hstate *h = hstate_vma(vma);
>  	unsigned long pages = 0;
> +	struct mmu_notifier_range range;
>  
>  	BUG_ON(address >= end);
>  	flush_cache_range(vma, address, end);
>  
> -	mmu_notifier_invalidate_range_start(mm, start, end, MMU_MPROT);
> +	range.start = start;
> +	range.end = end;
> +	range.event = MMU_MPROT;
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  	mutex_lock(&vma->vm_file->f_mapping->i_mmap_mutex);
>  	for (; address < end; address += huge_page_size(h)) {
>  		spinlock_t *ptl;
> @@ -3380,7 +3381,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
>  	 */
>  	flush_tlb_range(vma, start, end);
>  	mutex_unlock(&vma->vm_file->f_mapping->i_mmap_mutex);
> -	mmu_notifier_invalidate_range_end(mm, start, end, MMU_MPROT);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  
>  	return pages << h->order;
>  }
> diff --git a/mm/memory.c b/mm/memory.c
> index 1c212e6..c1c7ccc 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1008,8 +1008,7 @@ int copy_page_range(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  	unsigned long next;
>  	unsigned long addr = vma->vm_start;
>  	unsigned long end = vma->vm_end;
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  	bool is_cow;
>  	int ret;
>  
> @@ -1045,11 +1044,11 @@ int copy_page_range(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  	 * is_cow_mapping() returns true.
>  	 */
>  	is_cow = is_cow_mapping(vma->vm_flags);
> -	mmun_start = addr;
> -	mmun_end   = end;
> +	range.start = addr;
> +	range.end = end;
> +	range.event = MMU_MIGRATE;
>  	if (is_cow)
> -		mmu_notifier_invalidate_range_start(src_mm, mmun_start,
> -						    mmun_end, MMU_MIGRATE);
> +		mmu_notifier_invalidate_range_start(src_mm, &range);
>  
>  	ret = 0;
>  	dst_pgd = pgd_offset(dst_mm, addr);
> @@ -1066,8 +1065,7 @@ int copy_page_range(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  	} while (dst_pgd++, src_pgd++, addr = next, addr != end);
>  
>  	if (is_cow)
> -		mmu_notifier_invalidate_range_end(src_mm, mmun_start, mmun_end,
> -						  MMU_MIGRATE);
> +		mmu_notifier_invalidate_range_end(src_mm, &range);
>  	return ret;
>  }
>  
> @@ -1370,13 +1368,16 @@ void unmap_vmas(struct mmu_gather *tlb,
>  		unsigned long end_addr)
>  {
>  	struct mm_struct *mm = vma->vm_mm;
> +	struct mmu_notifier_range range = {
> +		.start = start_addr,
> +		.end = end_addr,
> +		.event = MMU_MUNMAP,
> +	};
>  
> -	mmu_notifier_invalidate_range_start(mm, start_addr,
> -					    end_addr, MMU_MUNMAP);
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  	for ( ; vma && vma->vm_start < end_addr; vma = vma->vm_next)
>  		unmap_single_vma(tlb, vma, start_addr, end_addr, NULL);
> -	mmu_notifier_invalidate_range_end(mm, start_addr,
> -					  end_addr, MMU_MUNMAP);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  }
>  
>  /**
> @@ -1393,16 +1394,20 @@ void zap_page_range(struct vm_area_struct *vma, unsigned long start,
>  {
>  	struct mm_struct *mm = vma->vm_mm;
>  	struct mmu_gather tlb;
> -	unsigned long end = start + size;
> +	struct mmu_notifier_range range = {
> +		.start = start,
> +		.end = start + size,
> +		.event = MMU_MUNMAP,
> +	};
>  
>  	lru_add_drain();
> -	tlb_gather_mmu(&tlb, mm, start, end);
> +	tlb_gather_mmu(&tlb, mm, start, range.end);
>  	update_hiwater_rss(mm);
> -	mmu_notifier_invalidate_range_start(mm, start, end, MMU_MUNMAP);
> -	for ( ; vma && vma->vm_start < end; vma = vma->vm_next)
> -		unmap_single_vma(&tlb, vma, start, end, details);
> -	mmu_notifier_invalidate_range_end(mm, start, end, MMU_MUNMAP);
> -	tlb_finish_mmu(&tlb, start, end);
> +	mmu_notifier_invalidate_range_start(mm, &range);
> +	for ( ; vma && vma->vm_start < range.end; vma = vma->vm_next)
> +		unmap_single_vma(&tlb, vma, start, range.end, details);
> +	mmu_notifier_invalidate_range_end(mm, &range);
> +	tlb_finish_mmu(&tlb, start, range.end);
>  }
>  
>  /**
> @@ -1419,15 +1424,19 @@ static void zap_page_range_single(struct vm_area_struct *vma, unsigned long addr
>  {
>  	struct mm_struct *mm = vma->vm_mm;
>  	struct mmu_gather tlb;
> -	unsigned long end = address + size;
> +	struct mmu_notifier_range range = {
> +		.start = address,
> +		.end = address + size,
> +		.event = MMU_MUNMAP,
> +	};
>  
>  	lru_add_drain();
> -	tlb_gather_mmu(&tlb, mm, address, end);
> +	tlb_gather_mmu(&tlb, mm, address, range.end);
>  	update_hiwater_rss(mm);
> -	mmu_notifier_invalidate_range_start(mm, address, end, MMU_MUNMAP);
> -	unmap_single_vma(&tlb, vma, address, end, details);
> -	mmu_notifier_invalidate_range_end(mm, address, end, MMU_MUNMAP);
> -	tlb_finish_mmu(&tlb, address, end);
> +	mmu_notifier_invalidate_range_start(mm, &range);
> +	unmap_single_vma(&tlb, vma, address, range.end, details);
> +	mmu_notifier_invalidate_range_end(mm, &range);
> +	tlb_finish_mmu(&tlb, address, range.end);
>  }
>  
>  /**
> @@ -2047,8 +2056,7 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
>  	int ret = 0;
>  	int page_mkwrite = 0;
>  	struct page *dirty_page = NULL;
> -	unsigned long mmun_start = 0;	/* For mmu_notifiers */
> -	unsigned long mmun_end = 0;	/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  	struct mem_cgroup *memcg;
>  
>  	old_page = vm_normal_page(vma, address, orig_pte);
> @@ -2208,10 +2216,10 @@ gotten:
>  	if (mem_cgroup_try_charge(new_page, mm, GFP_KERNEL, &memcg))
>  		goto oom_free_new;
>  
> -	mmun_start  = address & PAGE_MASK;
> -	mmun_end    = mmun_start + PAGE_SIZE;
> -	mmu_notifier_invalidate_range_start(mm, mmun_start,
> -					    mmun_end, MMU_MIGRATE);
> +	range.start = address & PAGE_MASK;
> +	range.end = range.start + PAGE_SIZE;
> +	range.event = MMU_MIGRATE;
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  
>  	/*
>  	 * Re-check the pte - we dropped the lock
> @@ -2282,8 +2290,7 @@ gotten:
>  unlock:
>  	pte_unmap_unlock(page_table, ptl);
>  	if (mmun_end > mmun_start)
> -		mmu_notifier_invalidate_range_end(mm, mmun_start,
> -						  mmun_end, MMU_MIGRATE);
> +		mmu_notifier_invalidate_range_end(mm, &range);
>  	if (old_page) {
>  		/*
>  		 * Don't let another task, with possibly unlocked vma,
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 30417d5..d866771 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1781,10 +1781,13 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
>  	int isolated = 0;
>  	struct page *new_page = NULL;
>  	int page_lru = page_is_file_cache(page);
> -	unsigned long mmun_start = address & HPAGE_PMD_MASK;
> -	unsigned long mmun_end = mmun_start + HPAGE_PMD_SIZE;
> +	struct mmu_notifier_range range;
>  	pmd_t orig_entry;
>  
> +	range.start = address & HPAGE_PMD_MASK;
> +	range.end = range.start + HPAGE_PMD_SIZE;
> +	range.event = MMU_MIGRATE;
> +
>  	/*
>  	 * Rate-limit the amount of data that is being migrated to a node.
>  	 * Optimal placement is no good if the memory bus is saturated and
> @@ -1819,14 +1822,12 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
>  	WARN_ON(PageLRU(new_page));
>  
>  	/* Recheck the target PMD */
> -	mmu_notifier_invalidate_range_start(mm, mmun_start,
> -					    mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  	ptl = pmd_lock(mm, pmd);
>  	if (unlikely(!pmd_same(*pmd, entry) || page_count(page) != 2)) {
>  fail_putback:
>  		spin_unlock(ptl);
> -		mmu_notifier_invalidate_range_end(mm, mmun_start,
> -						  mmun_end, MMU_MIGRATE);
> +		mmu_notifier_invalidate_range_end(mm, &range);
>  
>  		/* Reverse changes made by migrate_page_copy() */
>  		if (TestClearPageActive(new_page))
> @@ -1879,8 +1880,7 @@ fail_putback:
>  	page_remove_rmap(page);
>  
>  	spin_unlock(ptl);
> -	mmu_notifier_invalidate_range_end(mm, mmun_start,
> -					  mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  
>  	/* Take an "isolate" reference and put new page on the LRU. */
>  	get_page(new_page);
> diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
> index de039e4..d0edb98 100644
> --- a/mm/mmu_notifier.c
> +++ b/mm/mmu_notifier.c
> @@ -173,9 +173,7 @@ void __mmu_notifier_invalidate_page(struct mm_struct *mm,
>  }
>  
>  void __mmu_notifier_invalidate_range_start(struct mm_struct *mm,
> -					   unsigned long start,
> -					   unsigned long end,
> -					   enum mmu_event event)
> +					   struct mmu_notifier_range *range)
>  
>  {
>  	struct mmu_notifier *mn;
> @@ -184,31 +182,83 @@ void __mmu_notifier_invalidate_range_start(struct mm_struct *mm,
>  	id = srcu_read_lock(&srcu);
>  	hlist_for_each_entry_rcu(mn, &mm->mmu_notifier_mm->list, hlist) {
>  		if (mn->ops->invalidate_range_start)
> -			mn->ops->invalidate_range_start(mn, mm, start,
> -							end, event);
> +			mn->ops->invalidate_range_start(mn, mm, range);
>  	}
>  	srcu_read_unlock(&srcu, id);
> +
> +	/*
> +	 * This must happen after the callback so that subsystem can block on
> +	 * new invalidation range to synchronize itself.
> +	 */
> +	spin_lock(&mm->mmu_notifier_mm->lock);
> +	list_add_tail(&range->list, &mm->mmu_notifier_mm->ranges);
> +	mm->mmu_notifier_mm->nranges++;
> +	spin_unlock(&mm->mmu_notifier_mm->lock);
>  }
>  EXPORT_SYMBOL_GPL(__mmu_notifier_invalidate_range_start);
>  
>  void __mmu_notifier_invalidate_range_end(struct mm_struct *mm,
> -					 unsigned long start,
> -					 unsigned long end,
> -					 enum mmu_event event)
> +					 struct mmu_notifier_range *range)
>  {
>  	struct mmu_notifier *mn;
>  	int id;
>  
> +	/*
> +	 * This must happen before the callback so that subsystem can unblock
> +	 * when range invalidation end.
> +	 */
> +	spin_lock(&mm->mmu_notifier_mm->lock);
> +	list_del_init(&range->list);
> +	mm->mmu_notifier_mm->nranges--;
> +	spin_unlock(&mm->mmu_notifier_mm->lock);
> +
>  	id = srcu_read_lock(&srcu);
>  	hlist_for_each_entry_rcu(mn, &mm->mmu_notifier_mm->list, hlist) {
>  		if (mn->ops->invalidate_range_end)
> -			mn->ops->invalidate_range_end(mn, mm, start,
> -						      end, event);
> +			mn->ops->invalidate_range_end(mn, mm, range);
>  	}
>  	srcu_read_unlock(&srcu, id);
> +
> +	/*
> +	 * Wakeup after callback so they can do their job before any of the
> +	 * waiters resume.
> +	 */
> +	wake_up(&mm->mmu_notifier_mm->wait_queue);
>  }
>  EXPORT_SYMBOL_GPL(__mmu_notifier_invalidate_range_end);
>  
> +bool mmu_notifier_range_is_valid(struct mm_struct *mm,
> +				 unsigned long start,
> +				 unsigned long end)
> +{
> +	struct mmu_notifier_range range;
> +
> +	spin_lock(&mm->mmu_notifier_mm->lock);
> +	list_for_each_entry(range, &mm->mmu_notifier_mm->ranges, list) {
> +		if (!(range->end <= start || range->start >= end)) {
> +			spin_unlock(&mm->mmu_notifier_mm->lock);
> +			return false;
> +		}
> +	}
> +	spin_unlock(&mm->mmu_notifier_mm->lock);
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(mmu_notifier_range_is_valid);
> +
> +void mmu_notifier_range_wait_valid(struct mm_struct *mm,
> +				   unsigned long start,
> +				   unsigned long end)
> +{
> +	int nranges = mm->mmu_notifier_mm->nranges;
> +
> +	while (!mmu_notifier_range_is_valid(mm, start, end)) {
> +		wait_event(mm->mmu_notifier_mm->wait_queue,
> +			   nranges != mm->mmu_notifier_mm->nranges);
> +		nranges = mm->mmu_notifier_mm->nranges;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(mmu_notifier_range_wait_valid);
> +
>  static int do_mmu_notifier_register(struct mmu_notifier *mn,
>  				    struct mm_struct *mm,
>  				    int take_mmap_sem)
> @@ -238,6 +288,9 @@ static int do_mmu_notifier_register(struct mmu_notifier *mn,
>  	if (!mm_has_notifiers(mm)) {
>  		INIT_HLIST_HEAD(&mmu_notifier_mm->list);
>  		spin_lock_init(&mmu_notifier_mm->lock);
> +		INIT_LIST_HEAD(&mmu_notifier_mm->ranges);
> +		mmu_notifier_mm->nranges = 0;
> +		init_waitqueue_head(&mmu_notifier_mm->wait_queue);
>  
>  		mm->mmu_notifier_mm = mmu_notifier_mm;
>  		mmu_notifier_mm = NULL;
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 886405b..a178b22 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -144,7 +144,9 @@ static inline unsigned long change_pmd_range(struct vm_area_struct *vma,
>  	unsigned long next;
>  	unsigned long pages = 0;
>  	unsigned long nr_huge_updates = 0;
> -	unsigned long mni_start = 0;
> +	struct mmu_notifier_range range = {
> +		.start = 0,
> +	};
>  
>  	pmd = pmd_offset(pud, addr);
>  	do {
> @@ -155,10 +157,11 @@ static inline unsigned long change_pmd_range(struct vm_area_struct *vma,
>  			continue;
>  
>  		/* invoke the mmu notifier if the pmd is populated */
> -		if (!mni_start) {
> -			mni_start = addr;
> -			mmu_notifier_invalidate_range_start(mm, mni_start,
> -							    end, MMU_MPROT);
> +		if (!range.start) {
> +			range.start = addr;
> +			range.end = end;
> +			range.event = MMU_MPROT;
> +			mmu_notifier_invalidate_range_start(mm, &range);
>  		}
>  
>  		if (pmd_trans_huge(*pmd)) {
> @@ -185,8 +188,8 @@ static inline unsigned long change_pmd_range(struct vm_area_struct *vma,
>  		pages += this_pages;
>  	} while (pmd++, addr = next, addr != end);
>  
> -	if (mni_start)
> -		mmu_notifier_invalidate_range_end(mm, mni_start, end, MMU_MPROT);
> +	if (range.start)
> +		mmu_notifier_invalidate_range_end(mm, &range);
>  
>  	if (nr_huge_updates)
>  		count_vm_numa_events(NUMA_HUGE_PTE_UPDATES, nr_huge_updates);
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 6827d2f..83c5eed 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -167,18 +167,17 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
>  		bool need_rmap_locks)
>  {
>  	unsigned long extent, next, old_end;
> +	struct mmu_notifier_range range;
>  	pmd_t *old_pmd, *new_pmd;
>  	bool need_flush = false;
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
>  
>  	old_end = old_addr + len;
>  	flush_cache_range(vma, old_addr, old_end);
>  
> -	mmun_start = old_addr;
> -	mmun_end   = old_end;
> -	mmu_notifier_invalidate_range_start(vma->vm_mm, mmun_start,
> -					    mmun_end, MMU_MIGRATE);
> +	range.start = old_addr;
> +	range.end = old_end;
> +	range.event = MMU_MIGRATE;
> +	mmu_notifier_invalidate_range_start(vma->vm_mm, &range);
>  
>  	for (; old_addr < old_end; old_addr += extent, new_addr += extent) {
>  		cond_resched();
> @@ -229,8 +228,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
>  	if (likely(need_flush))
>  		flush_tlb_range(vma, old_end-len, old_addr);
>  
> -	mmu_notifier_invalidate_range_end(vma->vm_mm, mmun_start,
> -					  mmun_end, MMU_MIGRATE);
> +	mmu_notifier_invalidate_range_end(vma->vm_mm, &range);
>  
>  	return len + old_addr - old_end;	/* how much done */
>  }
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 0b67e7d..b8b8a60 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1302,15 +1302,14 @@ static int try_to_unmap_cluster(unsigned long cursor, unsigned int *mapcount,
>  	spinlock_t *ptl;
>  	struct page *page;
>  	unsigned long address;
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
> +	struct mmu_notifier_range range;
>  	unsigned long end;
>  	int ret = SWAP_AGAIN;
>  	int locked_vma = 0;
> -	enum mmu_event event = MMU_MIGRATE;
>  
> +	range.event = MMU_MIGRATE;
>  	if (flags & TTU_MUNLOCK)
> -		event = MMU_MUNLOCK;
> +		range.event = MMU_MUNLOCK;
>  
>  	address = (vma->vm_start + cursor) & CLUSTER_MASK;
>  	end = address + CLUSTER_SIZE;
> @@ -1323,9 +1322,9 @@ static int try_to_unmap_cluster(unsigned long cursor, unsigned int *mapcount,
>  	if (!pmd)
>  		return ret;
>  
> -	mmun_start = address;
> -	mmun_end   = end;
> -	mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end, event);
> +	range.start = address;
> +	range.end = end;
> +	mmu_notifier_invalidate_range_start(mm, &range);
>  
>  	/*
>  	 * If we can acquire the mmap_sem for read, and vma is VM_LOCKED,
> @@ -1390,7 +1389,7 @@ static int try_to_unmap_cluster(unsigned long cursor, unsigned int *mapcount,
>  		(*mapcount)--;
>  	}
>  	pte_unmap_unlock(pte - 1, ptl);
> -	mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end, event);
> +	mmu_notifier_invalidate_range_end(mm, &range);
>  	if (locked_vma)
>  		up_read(&vma->vm_mm->mmap_sem);
>  	return ret;
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 0ed3e88..8d8c2ce 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -318,9 +318,7 @@ static void kvm_mmu_notifier_change_pte(struct mmu_notifier *mn,
>  
>  static void kvm_mmu_notifier_invalidate_range_start(struct mmu_notifier *mn,
>  						    struct mm_struct *mm,
> -						    unsigned long start,
> -						    unsigned long end,
> -						    enum mmu_event event)
> +						    const struct mmu_notifier_range *range)
>  {
>  	struct kvm *kvm = mmu_notifier_to_kvm(mn);
>  	int need_tlb_flush = 0, idx;
> @@ -333,7 +331,7 @@ static void kvm_mmu_notifier_invalidate_range_start(struct mmu_notifier *mn,
>  	 * count is also read inside the mmu_lock critical section.
>  	 */
>  	kvm->mmu_notifier_count++;
> -	need_tlb_flush = kvm_unmap_hva_range(kvm, start, end);
> +	need_tlb_flush = kvm_unmap_hva_range(kvm, range->start, range->end);
>  	need_tlb_flush |= kvm->tlbs_dirty;
>  	/* we've to flush the tlb before the pages can be freed */
>  	if (need_tlb_flush)
> @@ -345,9 +343,7 @@ static void kvm_mmu_notifier_invalidate_range_start(struct mmu_notifier *mn,
>  
>  static void kvm_mmu_notifier_invalidate_range_end(struct mmu_notifier *mn,
>  						  struct mm_struct *mm,
> -						  unsigned long start,
> -						  unsigned long end,
> -						  enum mmu_event event)
> +						  const struct mmu_notifier_range *range)
>  {
>  	struct kvm *kvm = mmu_notifier_to_kvm(mn);
>  
> -- 
> 1.9.3
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Roland Dreier]

> +     notifiers_seq = atomic_read(&item->odp_data->notifiers_seq) + 1;
> +     atomic_set(&item->odp_data->notifiers_seq,
> +                notifiers_seq);

Is this code really as silly as it looks, or is there some deep reason
for avoiding atomic_inc() that I'm missing?  Do you need atomic_inc(),
since as far as I can tell all modification of notifiers_seq and
notifiers_count happens while holding the umem_mutex?

In general I find it very suspicious that you have these two atomic_t
members, notifiers_seq and notifiers_count, but you never use anything
except atomic_read() and atomic_set() to access them.  Why are they
atomic_t at all?

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Shachar Raindel]

Hi Roland,

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Roland Dreier
> Sent: Tuesday, October 14, 2014 11:30 AM
> Cc: Haggai Eran; linux-rdma@vger.kernel.org; Shachar Raindel; Sagi
> Grimberg
> Subject: Re: [PATCH v1 for-next 06/16] IB/core: Implement support for
> MMU notifiers regarding on demand paging regions
> 
> > +     notifiers_seq = atomic_read(&item->odp_data->notifiers_seq) + 1;
> > +     atomic_set(&item->odp_data->notifiers_seq,
> > +                notifiers_seq);
> 
> Is this code really as silly as it looks, or is there some deep reason
> for avoiding atomic_inc() that I'm missing?  Do you need atomic_inc(),
> since as far as I can tell all modification of notifiers_seq and
> notifiers_count happens while holding the umem_mutex?
> 
> In general I find it very suspicious that you have these two atomic_t
> members, notifiers_seq and notifiers_count, but you never use anything
> except atomic_read() and atomic_set() to access them.  Why are they
> atomic_t at all?
> 

The sequence counter is read while not holding the lock. To avoid
cases where the value is not completely updated due to an increase or
decrease operation, resulting in a completely irrelevant value (can
happen in some weird CPU architectures). We preferred to use an
explicit atomic_t type, which guarantee atomic updates for such cases.

We avoided atomic_inc as it results in a locked operation in intel
platform, which is not needed if the operation is done under a
lock. We therefore do an explicit read-modify-write, avoiding any
locked operations.

We will reexamine the code to determine if we can safely switch to
normal variables.

Thanks,
--Shachar

[PATCH v1 for-next 00/16] On demand paging

[Haggai Eran]

Hi Roland,

I understand that you were reluctant to review these patches as long as there
was an ongoing debate on whether or not the i_mmap_mutex should be changed into
a spinlock. 

It seems that the debate concluded with the decision to change it into a rwsem
[1], as apparently this provides the optimal performance with the new optimistic
spinning patch [2]. 

I believe this means that there will be no problem adding paging support to the
RDMA stack that depends on sleepable MMU notifiers. 

Changes from V0: http://marc.info/?l=linux-rdma&m=139375790322547&w=2

- Rebased against latest upstream / for-next branch.
- Removed dependency on patches that were accepted upstream.
- Removed pre-patches that were accepted upstream [3].
- Add extended uverb call for querying device (patch 1) and use kernel device
  attributes to report ODP capabilities through the new uverb entry instead of
  having a special verb.
- Allow upgrading page access permissions during page faults.
- Minor fixes to issues that came up during regression testing of the patches.

The following set of patches implements on-demand paging (ODP) support
in the RDMA stack and in the mlx5_ib Infiniband driver.

What is on-demand paging?

Applications register memory with an RDMA adapter using system calls,
and subsequently post IO operations that refer to the corresponding
virtual addresses directly to HW. Until now, this was achieved by
pinning the memory during the registration calls. The goal of on demand
paging is to avoid pinning the pages of registered memory regions (MRs).
This will allow users the same flexibility they get when swapping any
other part of their processes address spaces. Instead of requiring the
entire MR to fit in physical memory, we can allow the MR to be larger,
and only fit the current working set in physical memory.

This can make programming with RDMA much simpler. Today, developers that
are working with more data than their RAM can hold need either to
deregister and reregister memory regions throughout their process's
life, or keep a single memory region and copy the data to it. On demand
paging will allow these developers to register a single MR at the
beginning of their process's life, and let the operating system manage
which pages needs to be fetched at a given time. In the future, we might
be able to provide a single memory access key for each process that
would provide the entire process's address as one large memory region,
and the developers wouldn't need to register memory regions at all.

How does page faults generally work?

With pinned memory regions, the driver would map the virtual addresses
to bus addresses, and pass these addresses to the HCA to associate them
with the new MR. With ODP, the driver is now allowed to mark some of the
pages in the MR as not-present. When the HCA attempts to perform memory
access for a communication operation, it notices the page is not
present, and raises a page fault event to the driver. In addition, the
HCA performs whatever operation is required by the transport protocol to
suspend communication until the page fault is resolved.

Upon receiving the page fault interrupt, the driver first needs to know
on which virtual address the page fault occurred, and on what memory
key. When handling send/receive operations, this information is inside
the work queue. The driver reads the needed work queue elements, and
parses them to gather the address and memory key. For other RDMA
operations, the event generated by the HCA only contains the virtual
address and rkey, as there are no work queue elements involved.

Having the rkey, the driver can find the relevant memory region in its
data structures, and calculate the actual pages needed to complete the
operation. It then uses get_user_pages to retrieve the needed pages back
to the memory, obtains dma mapping, and passes the addresses to the HCA.
Finally, the driver notifies the HCA it can continue operation on the
queue pair that encountered the page fault. The pages that
get_user_pages returned are unpinned immediately by releasing their
reference.

How are invalidations handled?

The patches add infrastructure to subscribe the RDMA stack as an mmu
notifier client [4]. Each process that uses ODP register a notifier client.
When receiving page invalidation notifications, they are passed to the
mlx5_ib driver, which updates the HCA with new, not-present mappings.
Only after flushing the HCA's page table caches the notifier returns,
allowing the kernel to release the pages.

What operations are supported?

Currently only send, receive and RDMA write operations are supported on the
RC protocol, and also send operations on the UD protocol. We hope to
implement support for other transports and operations in the future.

The structure of the patchset

Patches 1-6:
The first set of patches adds page fault support to the IB core layer,
allowing MRs to be registered without their pages to be pinned. Patch 1
adds an extended verb to query device attributes, and patch 2
adds capability bits, configuration options, and a method for querying
whether the paging capabilities from user-space. The next two patches (3-4)
make some necessary changes to the ib_umem type. Patches 5 and 6 add
paging support and invalidation support respectively.

Patches 7-12:
This set of patches add small size new functionality to the mlx5 driver and
builds toward paging support. Patch 7 make changes to UMR mechanism
(an internal mechanism used by mlx5 to update device page mappings).
Patch 8 adds infrastructure support for page fault handling to the
mlx5_core module. Patch 9 queries the device for paging capabilities, and
patch 11 adds a function to do partial device page table updates. Finally,
patch 12 adds a helper function to read information from user-space work
queues in the driver's context.

Patches 13-16:
The final part of this patch set finally adds paging support to the mlx5
driver. Patch 13 adds in mlx5_ib the infrastructure to handle page faults
coming from mlx5_core. Patch 14 adds the code to handle UD send page faults
and RC send and receive page faults. Patch 15 adds support for page faults
caused by RDMA write operations, and patch 16 adds invalidation support to
the mlx5 driver, allowing pages to be unmapped dynamically.

[1] [PATCH 0/5] mm: i_mmap_mutex to rwsem
    https://lkml.org/lkml/2013/6/24/683

[2] Re: Performance regression from switching lock to rw-sem for anon-vma tree
    https://lkml.org/lkml/2013/6/17/452

[3] pre-patches that were accepted upstream:
  a74d241 IB/mlx5: Refactor UMR to have its own context struct
  48fea83 IB/mlx5: Set QP offsets and parameters for user QPs and not just for kernel QPs
  b475598 mlx5_core: Store MR attributes in mlx5_mr_core during creation and after UMR
  8605933 IB/mlx5: Add MR to radix tree in reg_mr_callback

[4] Integrating KVM with the Linux Memory Management (presentation),
    Andrea Archangeli
    http://www.linux-kvm.org/wiki/images/3/33/KvmForum2008%24kdf2008_15.pdf


Haggai Eran (11):
  IB/core: Add an extended user verb to query device attributes
  IB/core: Replace ib_umem's offset field with a full address
  IB/core: Add umem function to read data from user-space
  IB/mlx5: Enhance UMR support to allow partial page table update
  net/mlx5_core: Add support for page faults events and low level
    handling
  IB/mlx5: Implement the ODP capability query verb
  IB/mlx5: Changes in memory region creation to support on-demand
    paging
  IB/mlx5: Add mlx5_ib_update_mtt to update page tables after creation
  IB/mlx5: Add function to read WQE from user-space
  IB/mlx5: Page faults handling infrastructure
  IB/mlx5: Handle page faults

Sagi Grimberg (1):
  IB/core: Add flags for on demand paging support

Shachar Raindel (4):
  IB/core: Add support for on demand paging regions
  IB/core: Implement support for MMU notifiers regarding on demand
    paging regions
  IB/mlx5: Add support for RDMA write responder page faults
  IB/mlx5: Implement on demand paging by adding support for MMU
    notifiers

 drivers/infiniband/Kconfig                     |  11 +
 drivers/infiniband/core/Makefile               |   1 +
 drivers/infiniband/core/umem.c                 |  63 +-
 drivers/infiniband/core/umem_odp.c             | 620 ++++++++++++++++++++
 drivers/infiniband/core/umem_rbtree.c          |  94 +++
 drivers/infiniband/core/uverbs.h               |   1 +
 drivers/infiniband/core/uverbs_cmd.c           | 170 ++++--
 drivers/infiniband/core/uverbs_main.c          |   5 +-
 drivers/infiniband/hw/amso1100/c2_provider.c   |   2 +-
 drivers/infiniband/hw/ehca/ehca_mrmw.c         |   2 +-
 drivers/infiniband/hw/ipath/ipath_mr.c         |   2 +-
 drivers/infiniband/hw/mlx5/Makefile            |   1 +
 drivers/infiniband/hw/mlx5/main.c              |  39 +-
 drivers/infiniband/hw/mlx5/mem.c               |  67 ++-
 drivers/infiniband/hw/mlx5/mlx5_ib.h           | 114 +++-
 drivers/infiniband/hw/mlx5/mr.c                | 303 ++++++++--
 drivers/infiniband/hw/mlx5/odp.c               | 770 +++++++++++++++++++++++++
 drivers/infiniband/hw/mlx5/qp.c                | 198 +++++--
 drivers/infiniband/hw/nes/nes_verbs.c          |   4 +-
 drivers/infiniband/hw/ocrdma/ocrdma_verbs.c    |   2 +-
 drivers/infiniband/hw/qib/qib_mr.c             |   2 +-
 drivers/net/ethernet/mellanox/mlx5/core/eq.c   |  11 +-
 drivers/net/ethernet/mellanox/mlx5/core/fw.c   |  35 +-
 drivers/net/ethernet/mellanox/mlx5/core/main.c |   8 +-
 drivers/net/ethernet/mellanox/mlx5/core/qp.c   | 134 ++++-
 include/linux/mlx5/device.h                    |  73 ++-
 include/linux/mlx5/driver.h                    |  20 +-
 include/linux/mlx5/qp.h                        |  63 ++
 include/rdma/ib_umem.h                         |  29 +-
 include/rdma/ib_umem_odp.h                     | 156 +++++
 include/rdma/ib_verbs.h                        |  47 +-
 include/uapi/rdma/ib_user_verbs.h              |  25 +
 32 files changed, 2907 insertions(+), 165 deletions(-)
 create mode 100644 drivers/infiniband/core/umem_odp.c
 create mode 100644 drivers/infiniband/core/umem_rbtree.c
 create mode 100644 drivers/infiniband/hw/mlx5/odp.c
 create mode 100644 include/rdma/ib_umem_odp.h

-- 
1.7.11.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 for-next 06/16] IB/core: Implement support for MMU notifiers regarding on demand paging regions

[Haggai Eran]

From: Shachar Raindel <raindel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

* Add an interval tree implementation for ODP umems. Create an interval tree
  for each ucontext (including a count of the number of ODP MRs in this
  context, mutex, etc.), and register ODP umems in the interval tree.
* Add MMU notifiers handling functions, using the interval tree to notify only
  the relevant umems and underlying MRs.
* Register to receive MMU notifier events from the MM subsystem upon ODP MR
  registration (and unregister accordingly).
* Add a completion object to synchronize the destruction of ODP umems.
* Add mechanism to abort page faults when there's a concurrent invalidation.

The way we synchronize between concurrent invalidations and page faults is by
keeping a counter of currently running invalidations, and a sequence number
that is incremented whenever an invalidation is caught. The page fault code
checks the counter and also verifies that the sequence number hasn't
progressed before it updates the umem's page tables. This is similar to what
the kvm module does.

There's currently a rare race in the code when registering a umem in the
middle of an ongoing notifier. The proper fix is to either serialize the
insertion to our umem tree with mm_lock_all or use a ucontext wide running
notifiers count for retries decision. Either is ugly and can lead to some sort
of starvation. The current workaround is ugly as well - now the user can end
up with mapped addresses that are not in the user's address space (although it
is highly unlikely).

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Shachar Raindel <raindel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Yuval Dagan <yuvalda-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/Kconfig            |   1 +
 drivers/infiniband/core/Makefile      |   2 +-
 drivers/infiniband/core/umem.c        |   2 +-
 drivers/infiniband/core/umem_odp.c    | 337 +++++++++++++++++++++++++++++++++-
 drivers/infiniband/core/umem_rbtree.c |  94 ++++++++++
 drivers/infiniband/core/uverbs_cmd.c  |  16 ++
 include/rdma/ib_umem_odp.h            |  56 ++++++
 include/rdma/ib_verbs.h               |  16 ++
 8 files changed, 512 insertions(+), 12 deletions(-)
 create mode 100644 drivers/infiniband/core/umem_rbtree.c

diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
index 089a2c2..b899531 100644
--- a/drivers/infiniband/Kconfig
+++ b/drivers/infiniband/Kconfig
@@ -41,6 +41,7 @@ config INFINIBAND_USER_MEM
 config INFINIBAND_ON_DEMAND_PAGING
 	bool "InfiniBand on-demand paging support"
 	depends on INFINIBAND_USER_MEM
+	select MMU_NOTIFIER
 	default y
 	---help---
 	  On demand paging support for the InfiniBand subsystem.
diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile
index c58f791..acf7367 100644
--- a/drivers/infiniband/core/Makefile
+++ b/drivers/infiniband/core/Makefile
@@ -11,7 +11,7 @@ obj-$(CONFIG_INFINIBAND_USER_ACCESS) +=	ib_uverbs.o ib_ucm.o \
 ib_core-y :=			packer.o ud_header.o verbs.o sysfs.o \
 				device.o fmr_pool.o cache.o netlink.o
 ib_core-$(CONFIG_INFINIBAND_USER_MEM) += umem.o
-ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o
+ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o umem_rbtree.o
 
 ib_mad-y :=			mad.o smi.o agent.o mad_rmpp.o
 
diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index e9798e0..014977f 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -72,7 +72,7 @@ static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int d
  * ib_umem_get - Pin and DMA map userspace memory.
  *
  * If access flags indicate ODP memory, avoid pinning. Instead, stores
- * the mm for future page fault handling.
+ * the mm for future page fault handling in conjuction with MMU notifiers.
  *
  * @context: userspace context to pin memory for
  * @addr: userspace virtual address to start at
diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
index 0c90ce50..c048269 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -41,26 +41,204 @@
 #include <rdma/ib_umem.h>
 #include <rdma/ib_umem_odp.h>
 
+void ib_umem_notifier_start_account(struct ib_umem *item)
+{
+	int notifiers_count;
+	mutex_lock(&item->odp_data->umem_mutex);
+	/*
+	 * Avoid performing another locked operation, as we are
+	 * already protected by the wrapping mutex.
+	 */
+	notifiers_count = atomic_read(&item->odp_data->notifiers_count) + 1;
+	if (notifiers_count == 1)
+		reinit_completion(&item->odp_data->notifier_completion);
+	atomic_set(&item->odp_data->notifiers_count,
+		   notifiers_count);
+	mutex_unlock(&item->odp_data->umem_mutex);
+}
+EXPORT_SYMBOL(ib_umem_notifier_start_account);
+
+void ib_umem_notifier_end_account(struct ib_umem *item)
+{
+	int notifiers_count, notifiers_seq;
+	mutex_lock(&item->odp_data->umem_mutex);
+	/*
+	 * This sequence increase will notify the QP page fault that
+	 * the page that is going to be mapped in the spte could have
+	 * been freed.
+	 */
+	notifiers_seq = atomic_read(&item->odp_data->notifiers_seq) + 1;
+	atomic_set(&item->odp_data->notifiers_seq,
+		   notifiers_seq);
+	/*
+	 * The above sequence increase must be visible before the
+	 * below count decrease, which is ensured by the smp_wmb below
+	 * in conjunction with the smp_rmb in mmu_notifier_retry().
+	 */
+	smp_wmb();
+
+	notifiers_count = atomic_read(&item->odp_data->notifiers_count);
+	/*
+	 * This is a workaround for the unlikely case where we register a umem
+	 * in the middle of an ongoing notifier.
+	 */
+	if (notifiers_count > 0)
+		notifiers_count -= 1;
+	else
+		pr_warn("Got notifier end call without a previous start call");
+	atomic_set(&item->odp_data->notifiers_count,
+		   notifiers_count);
+	if (notifiers_count == 0)
+		complete_all(&item->odp_data->notifier_completion);
+	mutex_unlock(&item->odp_data->umem_mutex);
+}
+
+
+static int ib_umem_notifier_release_trampoline(struct ib_umem *item, u64 start,
+					       u64 end, void *cookie) {
+	/*
+	 * Increase the number of notifiers running, to
+	 * prevent any further fault handling on this MR.
+	 */
+	ib_umem_notifier_start_account(item);
+	item->odp_data->dying = 1;
+	/* Make sure that the fact the umem is dying is out before we release
+	 * all pending page faults. */
+	smp_wmb();
+	complete_all(&item->odp_data->notifier_completion);
+	item->context->invalidate_range(item, ib_umem_start(item),
+					ib_umem_end(item));
+	return 0;
+}
+
+static void ib_umem_notifier_release(struct mmu_notifier *mn,
+				     struct mm_struct *mm)
+{
+	struct ib_ucontext *context = container_of(mn, struct ib_ucontext, mn);
+
+	if (!context->invalidate_range)
+		return;
+
+	down_read(&context->umem_mutex);
+
+	rbt_ib_umem_for_each_in_range(&context->umem_tree, 0,
+				      ULLONG_MAX,
+				      ib_umem_notifier_release_trampoline,
+				      NULL);
+
+	up_read(&context->umem_mutex);
+}
+
+static int invalidate_page_trampoline(struct ib_umem *item, u64 start,
+				      u64 end, void *cookie)
+{
+	ib_umem_notifier_start_account(item);
+	item->context->invalidate_range(item, start, start + PAGE_SIZE);
+	ib_umem_notifier_end_account(item);
+	return 0;
+}
+
+static void ib_umem_notifier_invalidate_page(struct mmu_notifier *mn,
+					     struct mm_struct *mm,
+					     unsigned long address)
+{
+	struct ib_ucontext *context = container_of(mn, struct ib_ucontext, mn);
+
+	if (!context->invalidate_range)
+		return;
+
+	down_read(&context->umem_mutex);
+	rbt_ib_umem_for_each_in_range(&context->umem_tree, address,
+				      address + PAGE_SIZE,
+				      invalidate_page_trampoline, NULL);
+	up_read(&context->umem_mutex);
+}
+
+static int invalidate_range_start_trampoline(struct ib_umem *item, u64 start,
+					     u64 end, void *cookie)
+{
+	ib_umem_notifier_start_account(item);
+	item->context->invalidate_range(item, start, end);
+	return 0;
+}
+
+static void ib_umem_notifier_invalidate_range_start(struct mmu_notifier *mn,
+						    struct mm_struct *mm,
+						    unsigned long start,
+						    unsigned long end)
+{
+	struct ib_ucontext *context = container_of(mn, struct ib_ucontext, mn);
+
+	if (!context->invalidate_range)
+		return;
+
+	down_read(&context->umem_mutex);
+	rbt_ib_umem_for_each_in_range(&context->umem_tree, start,
+				      end,
+				      invalidate_range_start_trampoline, NULL);
+	up_read(&context->umem_mutex);
+}
+
+static int invalidate_range_end_trampoline(struct ib_umem *item, u64 start,
+					   u64 end, void *cookie)
+{
+	ib_umem_notifier_end_account(item);
+	return 0;
+}
+
+static void ib_umem_notifier_invalidate_range_end(struct mmu_notifier *mn,
+						  struct mm_struct *mm,
+						  unsigned long start,
+						  unsigned long end)
+{
+	struct ib_ucontext *context = container_of(mn, struct ib_ucontext, mn);
+
+	if (!context->invalidate_range)
+		return;
+
+	down_read(&context->umem_mutex);
+	rbt_ib_umem_for_each_in_range(&context->umem_tree, start,
+				      end,
+				      invalidate_range_end_trampoline, NULL);
+	up_read(&context->umem_mutex);
+}
+
+static struct mmu_notifier_ops ib_umem_notifiers = {
+	.release                    = ib_umem_notifier_release,
+	.invalidate_page            = ib_umem_notifier_invalidate_page,
+	.invalidate_range_start     = ib_umem_notifier_invalidate_range_start,
+	.invalidate_range_end       = ib_umem_notifier_invalidate_range_end,
+};
+
 int ib_umem_odp_get(struct ib_ucontext *context, struct ib_umem *umem)
 {
 	int ret_val;
 	struct pid *our_pid;
+	struct mm_struct *mm = get_task_mm(current);
+	BUG_ON(!mm);
 
 	/* Prevent creating ODP MRs in child processes */
 	rcu_read_lock();
 	our_pid = get_task_pid(current->group_leader, PIDTYPE_PID);
 	rcu_read_unlock();
 	put_pid(our_pid);
-	if (context->tgid != our_pid)
-		return -EINVAL;
+	if (context->tgid != our_pid) {
+		ret_val = -EINVAL;
+		goto out_mm;
+	}
 
 	umem->hugetlb = 0;
 	umem->odp_data = kzalloc(sizeof(*umem->odp_data), GFP_KERNEL);
-	if (!umem->odp_data)
-		return -ENOMEM;
+	if (!umem->odp_data) {
+		ret_val = -ENOMEM;
+		goto out_mm;
+	}
+	umem->odp_data->umem = umem;
 
 	mutex_init(&umem->odp_data->umem_mutex);
 
+	init_completion(&umem->odp_data->notifier_completion);
+
 	umem->odp_data->page_list = vzalloc(ib_umem_num_pages(umem) *
 					    sizeof(*umem->odp_data->page_list));
 	if (!umem->odp_data->page_list) {
@@ -75,17 +253,66 @@ int ib_umem_odp_get(struct ib_ucontext *context, struct ib_umem *umem)
 		goto out_page_list;
 	}
 
+	/*
+	 * When using MMU notifiers, we will get a
+	 * notification before the "current" task (and MM) is
+	 * destroyed. We use the umem_mutex lock to synchronize.
+	 */
+	down_write(&context->umem_mutex);
+	context->odp_mrs_count++;
+	if (likely(ib_umem_start(umem) != ib_umem_end(umem)))
+		rbt_ib_umem_insert(&umem->odp_data->interval_tree,
+				   &context->umem_tree);
+	downgrade_write(&context->umem_mutex);
+
+	if (context->odp_mrs_count == 1) {
+		/*
+		 * Note that at this point, no MMU notifier is running
+		 * for this context!
+		 */
+		INIT_HLIST_NODE(&context->mn.hlist);
+		context->mn.ops = &ib_umem_notifiers;
+		/*
+		 * Lock-dep detects a false positive for mmap_sem vs.
+		 * umem_mutex, due to not grasping downgrade_write correctly.
+		 */
+		lockdep_off();
+		ret_val = mmu_notifier_register(&context->mn, mm);
+		lockdep_on();
+		if (ret_val) {
+			pr_err("Failed to register mmu_notifier %d\n", ret_val);
+			ret_val = -EBUSY;
+			goto out_mutex;
+		}
+	}
+
+	up_read(&context->umem_mutex);
+
+	/*
+	 * Note that doing an mmput can cause a notifier for the relevant mm.
+	 * If the notifier is called while we hold the umem_mutex, this will
+	 * cause a deadlock. Therefore, we release the reference only after we
+	 * released the mutex.
+	 */
+	mmput(mm);
 	return 0;
 
+out_mutex:
+	up_read(&context->umem_mutex);
+	vfree(umem->odp_data->dma_list);
 out_page_list:
 	vfree(umem->odp_data->page_list);
 out_odp_data:
 	kfree(umem->odp_data);
+out_mm:
+	mmput(mm);
 	return ret_val;
 }
 
 void ib_umem_odp_release(struct ib_umem *umem)
 {
+	struct ib_ucontext *context = umem->context;
+
 	/*
 	 * Ensure that no more pages are mapped in the umem.
 	 *
@@ -95,6 +322,49 @@ void ib_umem_odp_release(struct ib_umem *umem)
 	ib_umem_odp_unmap_dma_pages(umem, ib_umem_start(umem),
 				    ib_umem_end(umem));
 
+	down_write(&context->umem_mutex);
+	if (likely(ib_umem_start(umem) != ib_umem_end(umem)))
+		rbt_ib_umem_remove(&umem->odp_data->interval_tree,
+				   &context->umem_tree);
+	context->odp_mrs_count--;
+
+	/*
+	 * Downgrade the lock to a read lock. This ensures that the notifiers
+	 * (who lock the mutex for reading) will be able to finish, and we
+	 * will be able to enventually obtain the mmu notifiers SRCU. Note
+	 * that since we are doing it atomically, no other user could register
+	 * and unregister while we do the check.
+	 */
+	downgrade_write(&context->umem_mutex);
+	if (!context->odp_mrs_count) {
+		struct task_struct *owning_process = NULL;
+		struct mm_struct *owning_mm        = NULL;
+		owning_process = get_pid_task(context->tgid,
+					      PIDTYPE_PID);
+		if (owning_process == NULL)
+			/*
+			 * The process is already dead, notifier were removed
+			 * already.
+			 */
+			goto out;
+
+		owning_mm = get_task_mm(owning_process);
+		if (owning_mm == NULL)
+			/*
+			 * The process' mm is already dead, notifier were
+			 * removed already.
+			 */
+			goto out_put_task;
+		mmu_notifier_unregister(&context->mn, owning_mm);
+
+		mmput(owning_mm);
+
+out_put_task:
+		put_task_struct(owning_process);
+	}
+out:
+	up_read(&context->umem_mutex);
+
 	vfree(umem->odp_data->dma_list);
 	vfree(umem->odp_data->page_list);
 	kfree(umem);
@@ -111,7 +381,8 @@ void ib_umem_odp_release(struct ib_umem *umem)
  *               the sequence number is taken from
  *               umem->odp_data->notifiers_seq.
  *
- * The function returns -EFAULT if the DMA mapping operation fails.
+ * The function returns -EFAULT if the DMA mapping operation fails. It returns
+ * -EAGAIN if a concurrent invalidation prevents us from updating the page. It
  *
  * The page is released via put_page even if the operation failed. For
  * on-demand pinning, the page is released whenever it isn't stored in the
@@ -120,6 +391,7 @@ void ib_umem_odp_release(struct ib_umem *umem)
 static int ib_umem_odp_map_dma_single_page(
 		struct ib_umem *umem,
 		int page_index,
+		u64 base_virt_addr,
 		struct page *page,
 		u64 access_mask,
 		unsigned long current_seq)
@@ -127,8 +399,18 @@ static int ib_umem_odp_map_dma_single_page(
 	struct ib_device *dev = umem->context->device;
 	dma_addr_t dma_addr;
 	int stored_page = 0;
+	int remove_existing_mapping = 0;
 	int ret = 0;
 	mutex_lock(&umem->odp_data->umem_mutex);
+	/*
+	 * Note: we avoid writing if seq is different from the initial seq, to
+	 * handle case of a racing notifier. This check also allows us to bail
+	 * early if we have a notifier running in parallel with us.
+	 */
+	if (ib_umem_mmu_notifier_retry(umem, current_seq)) {
+		ret = -EAGAIN;
+		goto out;
+	}
 	if (!(umem->odp_data->dma_list[page_index])) {
 		dma_addr = ib_dma_map_page(dev,
 					   page,
@@ -146,14 +428,27 @@ static int ib_umem_odp_map_dma_single_page(
 	} else {
 		pr_err("error: got different pages in IB device and from get_user_pages. IB device page: %p, gup page: %p\n",
 		       umem->odp_data->page_list[page_index], page);
+		/* Better remove the mapping now, to prevent any further
+		 * damage. */
+		remove_existing_mapping = 1;
 	}
 
 out:
 	mutex_unlock(&umem->odp_data->umem_mutex);
 
-	if (!stored_page)
+	/* On Demand Paging - avoid pinning the page */
+	if (umem->context->invalidate_range || !stored_page)
 		put_page(page);
 
+	if (remove_existing_mapping && umem->context->invalidate_range) {
+		invalidate_page_trampoline(
+			umem,
+			base_virt_addr + (page_index * PAGE_SIZE),
+			base_virt_addr + ((page_index+1)*PAGE_SIZE),
+			NULL);
+		ret = -EAGAIN;
+	}
+
 	return ret;
 }
 
@@ -166,6 +461,8 @@ out:
  *
  * Returns the number of pages mapped in success, negative error code
  * for failure.
+ * An -EAGAIN error code is returned when a concurrent mmu notifier prevents
+ * the function from completing its task.
  *
  * @umem: the umem to map and pin
  * @user_virt: the address from which we need to map.
@@ -187,6 +484,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
 	struct page       **local_page_list = NULL;
 	u64 off;
 	int j, k, ret = 0, start_idx, npages = 0;
+	u64 base_virt_addr;
 
 	if (access_mask == 0)
 		return -EINVAL;
@@ -201,6 +499,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
 
 	off = user_virt & (~PAGE_MASK);
 	user_virt = user_virt & PAGE_MASK;
+	base_virt_addr = user_virt;
 	bcnt += off; /* Charge for the first page offset as well. */
 
 	start_idx = (user_virt - ib_umem_start(umem)) >> PAGE_SHIFT;
@@ -242,8 +541,8 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
 		user_virt += npages << PAGE_SHIFT;
 		for (j = 0; j < npages; ++j) {
 			ret = ib_umem_odp_map_dma_single_page(
-				umem, k, local_page_list[j], access_mask,
-				current_seq);
+				umem, k, base_virt_addr, local_page_list[j],
+				access_mask, current_seq);
 			if (ret < 0)
 				break;
 			k++;
@@ -280,6 +579,11 @@ void ib_umem_odp_unmap_dma_pages(struct ib_umem *umem, u64 virt,
 	struct ib_device *dev = umem->context->device;
 	virt  = max_t(u64, virt,  ib_umem_start(umem));
 	bound = min_t(u64, bound, ib_umem_end(umem));
+	/* Note that during the run of this function, the
+	 * notifiers_count of the MR is > 0, preventing any racing
+	 * faults from completion. We might be racing with other
+	 * invalidations, so we must make sure we free each page only
+	 * once. */
 	for (addr = virt; addr < bound; addr += (u64)umem->page_size) {
 		idx = (addr - ib_umem_start(umem)) / PAGE_SIZE;
 		mutex_lock(&umem->odp_data->umem_mutex);
@@ -294,8 +598,21 @@ void ib_umem_odp_unmap_dma_pages(struct ib_umem *umem, u64 virt,
 			ib_dma_unmap_page(dev, dma_addr, PAGE_SIZE,
 					  DMA_BIDIRECTIONAL);
 			if (umem->writable)
-				set_page_dirty_lock(head_page);
-			put_page(page);
+				/*
+				 * set_page_dirty prefers being called with
+				 * the page lock. However, MMU notifiers are
+				 * called sometimes with and sometimes without
+				 * the lock. We rely on the umem_mutex instead
+				 * to prevent other mmu notifiers from
+				 * continuing and allowing the page mapping to
+				 * be removed.
+				 */
+				set_page_dirty(head_page);
+			/* on demand pinning support */
+			if (!umem->context->invalidate_range)
+				put_page(page);
+			umem->odp_data->page_list[idx] = NULL;
+			umem->odp_data->dma_list[idx] = 0;
 		}
 		mutex_unlock(&umem->odp_data->umem_mutex);
 	}
diff --git a/drivers/infiniband/core/umem_rbtree.c b/drivers/infiniband/core/umem_rbtree.c
new file mode 100644
index 0000000..727d788
--- /dev/null
+++ b/drivers/infiniband/core/umem_rbtree.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2014 Mellanox Technologies. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interval_tree_generic.h>
+#include <linux/sched.h>
+#include <linux/gfp.h>
+#include <rdma/ib_umem_odp.h>
+
+/*
+ * The ib_umem list keeps track of memory regions for which the HW
+ * device request to receive notification when the related memory
+ * mapping is changed.
+ *
+ * ib_umem_lock protects the list.
+ */
+
+static inline u64 node_start(struct umem_odp_node *n)
+{
+	struct ib_umem_odp *umem_odp =
+			container_of(n, struct ib_umem_odp, interval_tree);
+
+	return ib_umem_start(umem_odp->umem);
+}
+
+/* Note that the representation of the intervals in the interval tree
+ * considers the ending point as contained in the interval, while the
+ * function ib_umem_end returns the first address which is not contained
+ * in the umem.
+ */
+static inline u64 node_last(struct umem_odp_node *n)
+{
+	struct ib_umem_odp *umem_odp =
+			container_of(n, struct ib_umem_odp, interval_tree);
+
+	return ib_umem_end(umem_odp->umem) - 1;
+}
+
+INTERVAL_TREE_DEFINE(struct umem_odp_node, rb, u64, __subtree_last,
+		     node_start, node_last, , rbt_ib_umem)
+
+/* @last is not a part of the interval. See comment for function
+ * node_last.
+ */
+int rbt_ib_umem_for_each_in_range(struct rb_root *root,
+				  u64 start, u64 last,
+				  umem_call_back cb,
+				  void *cookie)
+{
+	int ret_val = 0;
+	struct umem_odp_node *node;
+	struct ib_umem_odp *umem;
+
+	if (unlikely(start == last))
+		return ret_val;
+
+	for (node = rbt_ib_umem_iter_first(root, start, last - 1); node;
+			node = rbt_ib_umem_iter_next(node, start, last - 1)) {
+		umem = container_of(node, struct ib_umem_odp, interval_tree);
+		ret_val = cb(umem->umem, start, last, cookie) || ret_val;
+	}
+
+	return ret_val;
+}
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index fe709ca..a81d0c7 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -289,6 +289,9 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 	struct ib_uverbs_get_context_resp resp;
 	struct ib_udata                   udata;
 	struct ib_device                 *ibdev = file->device->ib_dev;
+#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
+	struct ib_device_attr		  dev_attr;
+#endif
 	struct ib_ucontext		 *ucontext;
 	struct file			 *filp;
 	int ret;
@@ -331,6 +334,19 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 	rcu_read_unlock();
 	ucontext->closing = 0;
 
+#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
+	ucontext->umem_tree = RB_ROOT;
+	init_rwsem(&ucontext->umem_mutex);
+	ucontext->odp_mrs_count = 0;
+
+	ret = ib_query_device(ibdev, &dev_attr);
+	if (ret)
+		goto err_free;
+	if (!(dev_attr.device_cap_flags & IB_DEVICE_ON_DEMAND_PAGING))
+		ucontext->invalidate_range = NULL;
+
+#endif
+
 	resp.num_comp_vectors = file->device->num_comp_vectors;
 
 	ret = get_unused_fd_flags(O_CLOEXEC);
diff --git a/include/rdma/ib_umem_odp.h b/include/rdma/ib_umem_odp.h
index 375ce28..9b93206 100644
--- a/include/rdma/ib_umem_odp.h
+++ b/include/rdma/ib_umem_odp.h
@@ -34,6 +34,12 @@
 #define IB_UMEM_ODP_H
 
 #include <rdma/ib_umem.h>
+#include <linux/interval_tree.h>
+
+struct umem_odp_node {
+	u64 __subtree_last;
+	struct rb_node rb;
+};
 
 struct ib_umem_odp {
 	/*
@@ -58,6 +64,14 @@ struct ib_umem_odp {
 
 	atomic_t		notifiers_seq;
 	atomic_t		notifiers_count;
+
+	struct ib_umem		*umem;
+
+	/* Tree tracking */
+	struct umem_odp_node	interval_tree;
+
+	struct completion	notifier_completion;
+	int			dying;
 };
 
 #ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
@@ -85,6 +99,48 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 start_offset, u64 bcnt,
 void ib_umem_odp_unmap_dma_pages(struct ib_umem *umem, u64 start_offset,
 				 u64 bound);
 
+void rbt_ib_umem_insert(struct umem_odp_node *node, struct rb_root *root);
+void rbt_ib_umem_remove(struct umem_odp_node *node, struct rb_root *root);
+typedef int (*umem_call_back)(struct ib_umem *item, u64 start, u64 end,
+			      void *cookie);
+/*
+ * Call the callback on each ib_umem in the range. Returns the logical or of
+ * the return values of the functions called.
+ */
+int rbt_ib_umem_for_each_in_range(struct rb_root *root, u64 start, u64 end,
+				  umem_call_back cb, void *cookie);
+
+struct umem_odp_node *rbt_ib_umem_iter_first(struct rb_root *root,
+					     u64 start, u64 last);
+struct umem_odp_node *rbt_ib_umem_iter_next(struct umem_odp_node *node,
+					    u64 start, u64 last);
+
+static inline int ib_umem_mmu_notifier_retry(struct ib_umem *item,
+					     unsigned long mmu_seq)
+{
+	/*
+	 * This code is strongly based on the KVM code from
+	 * mmu_notifier_retry. Should be called with
+	 * item->odp_data->umem_mutex locked.
+	 */
+	if (unlikely(atomic_read(&item->odp_data->notifiers_count)))
+		return 1;
+	/*
+	 * Ensure the read of mmu_notifier_count happens before the read
+	 * of mmu_notifier_seq.  This interacts with the smp_wmb() in
+	 * mmu_notifier_invalidate_range_end to make sure that the caller
+	 * either sees the old (non-zero) value of mmu_notifier_count or
+	 * the new (incremented) value of mmu_notifier_seq.
+	 */
+	smp_rmb();
+	if (atomic_read(&item->odp_data->notifiers_seq) != mmu_seq)
+		return 1;
+	return 0;
+}
+
+void ib_umem_notifier_start_account(struct ib_umem *item);
+void ib_umem_notifier_end_account(struct ib_umem *item);
+
 #else /* CONFIG_INFINIBAND_ON_DEMAND_PAGING */
 
 static inline int ib_umem_odp_get(struct ib_ucontext *context,
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 6d7f75e..54df9a8 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -51,6 +51,7 @@
 #include <uapi/linux/if_ether.h>
 
 #include <linux/atomic.h>
+#include <linux/mmu_notifier.h>
 #include <asm/uaccess.h>
 
 extern struct workqueue_struct *ib_wq;
@@ -1142,6 +1143,8 @@ struct ib_fmr_attr {
 	u8	page_shift;
 };
 
+struct ib_umem;
+
 struct ib_ucontext {
 	struct ib_device       *device;
 	struct list_head	pd_list;
@@ -1157,6 +1160,19 @@ struct ib_ucontext {
 
 	/* For ODP support: */
 	struct pid             *tgid;
+#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
+	struct rb_root      umem_tree;
+	/*
+	 * Protects .umem_rbroot and tree, as well as odp_mrs_count and
+	 * mmu notifiers registration.
+	 */
+	struct rw_semaphore	umem_mutex;
+	void (*invalidate_range)(struct ib_umem *umem,
+				 unsigned long start, unsigned long end);
+
+	struct mmu_notifier	mn;
+	int                     odp_mrs_count;
+#endif
 };
 
 struct ib_uobject {
-- 
1.7.11.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html