On Sat, Jun 08, 2019 at 01:08:37AM +0530, Souptick Joarder wrote:
> On Thu, May 23, 2019 at 9:05 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
> >
> > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> >
> > No other register/unregister kernel API attempts to provide this kind of
> > protection as it is inherently racy, so just drop it.
> >
> > Callers should provide their own protection, it appears nouveau already
> > does, but just in case drop a debugging POISON.
> >
> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> > mm/hmm.c | 9 ++-------
> > 1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/mm/hmm.c b/mm/hmm.c
> > index 46872306f922bb..6c3b7398672c29 100644
> > +++ b/mm/hmm.c
> > @@ -286,18 +286,13 @@ EXPORT_SYMBOL(hmm_mirror_register);
> > */
> > void hmm_mirror_unregister(struct hmm_mirror *mirror)
> > {
> > - struct hmm *hmm = READ_ONCE(mirror->hmm);
> > -
> > - if (hmm == NULL)
> > - return;
> > + struct hmm *hmm = mirror->hmm;
>
> How about remove struct hmm *hmm and replace the code like below -
>
> down_write(&mirror->hmm->mirrors_sem);
> list_del_init(&mirror->list);
> up_write(&mirror->hmm->mirrors_sem);
> hmm_put(hmm);
> memset(&mirror->hmm, POISON_INUSE, sizeof(mirror->hmm));
>
> Similar to hmm_mirror_register().
I think we get there in patch 10, right?
When the series is all done the function looks like this:
void hmm_mirror_unregister(struct hmm_mirror *mirror)
{
struct hmm *hmm = mirror->hmm;
down_write(&hmm->mirrors_sem);
list_del(&mirror->list);
up_write(&hmm->mirrors_sem);
hmm_put(hmm);
memset(&mirror->hmm, POISON_INUSE, sizeof(mirror->hmm));
}
I think this mostly matches what you wrote above, or do you think we
should s/hmm/mirror->hmm/ anyhow? I think Ralph just added that :)
Regards,
Jason
