On Mon, Aug 03, 2015 at 12:21:14PM +0000, GIRISH K S wrote:
> On Mon, Aug 03, 2015 at 01:20:13PM +0530, Girish KS wrote:
> > On 18-Jul-2015 12:47 am, "J��e Glisse" 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().
>
> I didn't mean hmm = hmm_ref(mm->hmm);. I ll try to put it in a better way.
> The hmm local variable is initialized to NULL in the start of the function
> (struct hmm *hmm = NULL;), and this is not modified till it is passed to
> hmm_ref. So hmm_ref would always return a NULL irrespective of mm->hmm is
> NULL or valid address.
> So the statement hmm = mm->hmm ? hmm_ref(hmm) : NULL; should be replaced
> as hmm = mm->hmm ? hmm_ref(mm->hmm) : NULL;.
Oh yeah typo probably outcome of many patch reorg i did.
Cheers,
Jérôme
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
