On Wed, 27 Feb 2008, Andrea Arcangeli wrote:

> +struct mmu_notifier_head {
> +     struct hlist_head head;
> +     spinlock_t lock;
> +};

Still think that the lock here is not of too much use and can be easily 
replaced by mmap_sem.

> +#define mmu_notifier(function, mm, args...)                          \
> +     do {                                                            \
> +             struct mmu_notifier *__mn;                              \
> +             struct hlist_node *__n;                                 \
> +                                                                     \
> +             if (unlikely(!hlist_empty(&(mm)->mmu_notifier.head))) { \
> +                     rcu_read_lock();                                \
> +                     hlist_for_each_entry_rcu(__mn, __n,             \
> +                                              &(mm)->mmu_notifier.head, \
> +                                              hlist)                 \
> +                             if (__mn->ops->function)                \
> +                                     __mn->ops->function(__mn,       \
> +                                                         mm,         \
> +                                                         args);      \
> +                     rcu_read_unlock();                              \
> +             }                                                       \
> +     } while (0)

Andrew recomended local variables for parameters used multile times. This 
means the mm parameter here.

> +/*
> + * Notifiers that use the parameters that they were passed so that the
> + * compiler does not complain about unused variables but does proper
> + * parameter checks even if !CONFIG_MMU_NOTIFIER.
> + * Macros generate no code.
> + */
> +#define mmu_notifier(function, mm, args...)                         \
> +     do {                                                           \
> +             if (0) {                                               \
> +                     struct mmu_notifier *__mn;                     \
> +                                                                    \
> +                     __mn = (struct mmu_notifier *)(0x00ff);        \
> +                     __mn->ops->function(__mn, mm, args);           \
> +             };                                                     \
> +     } while (0)

Note also Andrew's comments on the use of 0x00ff...

> +/*
> + * No synchronization. This function can only be called when only a single
> + * process remains that performs teardown.
> + */
> +void mmu_notifier_release(struct mm_struct *mm)
> +{
> +     struct mmu_notifier *mn;
> +     struct hlist_node *n, *tmp;
> +
> +     if (unlikely(!hlist_empty(&mm->mmu_notifier.head))) {
> +             hlist_for_each_entry_safe(mn, n, tmp,
> +                                       &mm->mmu_notifier.head, hlist) {
> +                     hlist_del(&mn->hlist);
> +                     if (mn->ops->release)
> +                             mn->ops->release(mn, mm);
> +             }
> +     }
> +}

One could avoid a hlist_for_each_entry_safe here by simply always deleting 
the first object. 

Also re the _notify variants: The binding to pte_clear_flush_young etc 
will become a problem for notifiers that want to sleep because 
pte_clear_flush is usually called with the pte lock held. See f.e. 
try_to_unmap_one, page_mkclean_one etc.

It would be better if the notifier calls could be moved outside of the 
pte lock.




-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Reply via email to