On Sat, Jun 8, 2019 at 1:07 AM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
>
> 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?
No, Patch 10 of this series has modified hmm_range_unregister().
>
> 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 :)
I prefer, s/hmm/mirror->hmm and remove struct hmm *hmm :)
