On 2022/2/17 21:32, Matthew Wilcox wrote: > 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. >
