Re: [Xen-devel] [PATCH 8 of 9] Modify all internal p2m functions to use the new fine-grained locking

[Tim Deegan <tim@xxxxxxx>]

Hi, 

At 07:24 -0700 on 02 Nov (1320218674), andres@xxxxxxxxxxxxxxxx wrote:
> >> +/* No matter what value you get out of a query, the p2m has been locked
> >> for
> >> + * that range. No matter what you do, you need to drop those locks.
> >> + * You need to pass back the mfn obtained when locking, not the new
> >> one,
> >> + * as the refcount of the original mfn was bumped. */
> >
> > Surely the caller doesn't need to remember the old MFN for this?  After
> > allm, the whole point of the lock was that nobody else could change the
> > p2m entry under our feet!
> >
> > In any case, I thing there needs to be a big block comment a bit futher
> > up that describes what all this locking and refcounting does, and why.
> 
> Comment will be added. I was being doubly-paranoid. I can undo the
> get_page/put_page of the old mfn. I'm not a 100% behind it.

I meant to suggest that the p2m code should me able to do the
get_page/put_page without the caller remembering the mfn, since by
definition it should be able to look it up in the unlock, knowing no-one
else can have changed it. 

> I don't think these names are the most terrible -- we've all seen far
> worse :) I mean, the naming encodes the arguments, and I don't see an
> intrinsic advantage to
> gfn_to_mfn(d, g, t, p2m_guest, p2m_unlocked)
> over
> gfn_to_mfn_guest_unlocked(d,g,t)

Yep, it's definitely not the worst. :)  It's really just a question of
verbosity in the headers.

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel