lkml.org 
[lkml]   [2014]   [Dec]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/7] mmu_notifier: keep track of active invalidation ranges v2
On 22/12/2014 18:48, j.glisse@gmail.com wrote:
> 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)
> {
> + /*
> + * Initialize list no matter what in case a mmu_notifier register after
> + * a range_start but before matching range_end.
> + */
> + INIT_LIST_HEAD(&range->list);

I don't see how can an mmu_notifier register after a range_start but
before a matching range_end. The mmu_notifier registration locks all mm
locks, and that should prevent any invalidation from running, right?

> if (mm_has_notifiers(mm))
> - __mmu_notifier_invalidate_range_start(mm, start, end, event);
> + __mmu_notifier_invalidate_range_start(mm, range);
> }

...

> 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;
> @@ -185,21 +183,36 @@ 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);

Don't you have a race here because you add the range struct after the
callback?

-------------------------------------------------------------------------
Thread A | Thread B
-------------------------------------------------------------------------
call mmu notifier callback |
clear SPTE |
| device page fault
| mmu_notifier_range_is_valid returns true
| install new SPTE
add event struct to list |
mm clears/modifies the PTE |
-------------------------------------------------------------------------

So we are left with different entries in the host page table and the
secondary page table.

I would think you'd want the event struct to be added to the list before
the callback is run.

Best regards,
Haggai




\
 
 \ /
  Last update: 2014-12-25 10:01    [W:0.090 / U:1.572 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site