On Thu, Feb 17, 2022 at 07:09:48PM +0800, Miaohe Lin wrote: > Use helper function mmu_notifier_synchronize() to ensure all mmu_notifiers > are freed. Minor readability improvement. Is it though? > @@ -334,15 +334,15 @@ static void mn_hlist_release(struct mmu_notifier_subscriptions *subscriptions, > srcu_read_unlock(&srcu, id); > > /* > - * synchronize_srcu here prevents mmu_notifier_release from returning to > - * exit_mmap (which would proceed with freeing all pages in the mm) > - * until the ->release method returns, if it was invoked by > - * mmu_notifier_unregister. > + * mmu_notifier_synchronize here prevents mmu_notifier_release from > + * returning to exit_mmap (which would proceed with freeing all pages > + * in the mm) until the ->release method returns, if it was invoked > + * by mmu_notifier_unregister. > * > * The notifier_subscriptions can't go away from under us because > * one mm_count is held by exit_mmap. > */ > - synchronize_srcu(&srcu); > + mmu_notifier_synchronize(); We just read_unlocked the &srcu. Now I have to jump to the definition of mmu_notifier_synchronize() to find out that it's now waiting for the very same srcu. I think this abstraction makes the code harder to read, not easier. > } > > void __mmu_notifier_release(struct mm_struct *mm) > @@ -851,7 +851,7 @@ void mmu_notifier_unregister(struct mmu_notifier *subscription, > * Wait for any running method to finish, of course including > * ->release if it was run by mmu_notifier_release instead of us. > */ > - synchronize_srcu(&srcu); > + mmu_notifier_synchronize(); Same here.
