Re: [PATCH 05/15] HMM: introduce heterogeneous memory management v4.

[Jerome Glisse]

On Mon, Aug 03, 2015 at 01:20:13PM +0530, Girish KS wrote:
> On 18-Jul-2015 12:47 am, "Jérôme Glisse" <jglisse@xxxxxxxxxx> wrote:
> >

[...]

> > +int hmm_mirror_register(struct hmm_mirror *mirror)
> > +{
> > +       struct mm_struct *mm = current->mm;
> > +       struct hmm *hmm = NULL;
> > +       int ret = 0;
> > +
> > +       /* Sanity checks. */
> > +       BUG_ON(!mirror);
> > +       BUG_ON(!mirror->device);
> > +       BUG_ON(!mm);
> > +
> > +       /*
> > +        * Initialize the mirror struct fields, the mlist init and del
> dance is
> > +        * necessary to make the error path easier for driver and for hmm.
> > +        */
> > +       kref_init(&mirror->kref);
> > +       INIT_HLIST_NODE(&mirror->mlist);
> > +       INIT_LIST_HEAD(&mirror->dlist);
> > +       spin_lock(&mirror->device->lock);
> > +       list_add(&mirror->dlist, &mirror->device->mirrors);
> > +       spin_unlock(&mirror->device->lock);
> > +
> > +       down_write(&mm->mmap_sem);
> > +
> > +       hmm = mm->hmm ? hmm_ref(hmm) : NULL;
> 
> Instead of hmm mm->hmm would be the right param to be passed.  Here even
> though mm->hmm is true hmm_ref returns NULL. Because hmm is not updated
> after initialization in the beginning.

ENOPARSE ? While this can be simplified to hmm = hmm_ref(mm->hmm); I do not
see what you mean. The mm struct might already have a valid hmm field set,
and that valid hmm struct might also already be in the process of being
destroy. So hmm_ref() might either return the same hmm pointer if the hmm
object is not about to be release or NULL. But at this point there is no
certainty on the return value of hmm_ref().

Note that because we have the mmap sem in write mode we know it is safe
to dereference mm->hmm and even to overwrite that field it if it is being
destroy concurently.

Cheers,
Jérôme
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/