From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
"Peter Zijlstra" <peterz@infradead.org>,
"Jérôme Glisse" <jglisse@redhat.com>,
"Oded Gabbay" <oded.gabbay@amd.com>
Subject: Re: [PATCH] mm/mmu_notifier: rename mmu_notifier_synchronize() to <...>_barrier()
Date: Mon, 5 Nov 2018 12:49:34 -0800 [thread overview]
Message-ID: <20181105204934.GA27247@linux.intel.com> (raw)
In-Reply-To: <20181105121833.200d5b53300a7ef4df7d349d@linux-foundation.org>
On Mon, Nov 05, 2018 at 12:18:33PM -0800, Andrew Morton wrote:
> On Mon, 5 Nov 2018 11:29:55 -0800 Sean Christopherson <sean.j.christopherson@intel.com> wrote:
>
> > ...and update its comment to explicitly reference its association with
> > mmu_notifier_call_srcu().
> >
> > Contrary to its name, mmu_notifier_synchronize() does not synchronize
> > the notifier's SRCU instance, but rather waits for RCU callbacks to
> > finished, i.e. it invokes rcu_barrier(). The RCU documentation is
> > quite clear on this matter, explicitly calling out that rcu_barrier()
> > does not imply synchronize_rcu(). The misnomer could lean an unwary
> > developer to incorrectly assume that mmu_notifier_synchronize() can
> > be used in conjunction with mmu_notifier_unregister_no_release() to
> > implement a variation of mmu_notifier_unregister() that synchronizes
> > SRCU without invoking ->release. A Documentation-allergic and hasty
> > developer could be further confused by the fact that rcu_barrier() is
> > indeed a pass-through to synchronize_rcu()... in tiny SRCU.
>
> Fair enough.
>
> > --- a/mm/mmu_notifier.c
> > +++ b/mm/mmu_notifier.c
> > @@ -35,12 +35,12 @@ void mmu_notifier_call_srcu(struct rcu_head *rcu,
> > }
> > EXPORT_SYMBOL_GPL(mmu_notifier_call_srcu);
> >
> > -void mmu_notifier_synchronize(void)
> > +void mmu_notifier_barrier(void)
> > {
> > - /* Wait for any running method to finish. */
> > + /* Wait for any running RCU callbacks (see above) to finish. */
> > srcu_barrier(&srcu);
> > }
> > -EXPORT_SYMBOL_GPL(mmu_notifier_synchronize);
> > +EXPORT_SYMBOL_GPL(mmu_notifier_barrier);
> >
> > /*
> > * This function can't run concurrently against mmu_notifier_register
>
> But as it has no callers, why retain it?
I was hesitant to remove it altogether since it was explicitly added to
complement mmu_notifier_call_srcu()[1] even though the initial user of
mmu_notifier_call_srcu() didn't use mmu_notifier_synchronize()[2]. I
assume there was a good reason for adding the barrier function, but
maybe that's a bad assumption.
[1] b972216e27d1 ("mmu_notifier: add call_srcu and sync function for listener to delay call and sync")
[2] https://lore.kernel.org/patchwork/patch/515318/
next prev parent reply other threads:[~2018-11-05 20:49 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-05 19:29 [PATCH] mm/mmu_notifier: rename mmu_notifier_synchronize() to <...>_barrier() Sean Christopherson
2018-11-05 20:18 ` Andrew Morton
2018-11-05 20:49 ` Sean Christopherson [this message]
2018-11-05 21:14 ` Matthew Wilcox
2018-11-06 13:34 ` Sean Christopherson
2018-11-06 13:34 ` Sean Christopherson
2018-11-06 13:26 ` Michal Hocko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181105204934.GA27247@linux.intel.com \
--to=sean.j.christopherson@intel.com \
--cc=akpm@linux-foundation.org \
--cc=jglisse@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=oded.gabbay@amd.com \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.