On Thu, 2010-10-14 at 02:43 +0100, Jeremy Fitzhardinge wrote:
> On 10/12/2010 03:14 AM, Ian Campbell wrote:
> > static RESERVE_BRK_ARRAY(unsigned long **, p2m_top, P2M_TOP_PER_PAGE);
> > static RESERVE_BRK_ARRAY(unsigned long, p2m_top_mfn, P2M_TOP_PER_PAGE);
> > +static RESERVE_BRK_ARRAY(unsigned long *, p2m_mid_mfn_p, P2M_TOP_PER_PAGE);
>
> This should be called "p2m_top_mfn_p" to have consistent naming, since
> its at the top of the hierarchy and is indexed by topidx. The "mid" in
> the name makes it look like it should be indexed by mididx, but then it
> wouldn't make sense to have just one of these (since there needs to be a
> mid for each entry in top). The fact that it points to mids is
> irrelevant (or at least implied by the fact that its a top).
OK.
> > @@ -322,14 +339,19 @@ void xen_build_mfn_list_list(void)
> > mid = p2m_top[topidx];
> >
> > /* Don't bother allocating any mfn mid levels if
> > - they're just missing */
> > - if (mid[mididx] == p2m_missing)
> > + * they're just missing, just update the stored mfn,
> > + * since all could have changed over a migrate.
> > + */
> > + if (mid == p2m_mid_missing) {
> > + p2m_top_mfn[topidx] = virt_to_mfn(p2m_mid_missing);
> > + pfn += P2M_MID_PER_PAGE - 1;
>
> Why -1? How does this interact with the "pfn += P2M_PER_PAGE" in the
> for loop? Should it be "(P2M_MID_PER_PAGE - 1) * P2M_PER_PAGE", with
> the pfn += P2M_PER_PAGE moved to the bottom of the loop rather than in
> the header?
The -1 was supposed to offset the ++ in the for header, except as you
point out its actually a +=.
I'll figure out what the correct increment should be.
> Is this test even necessary? Is it to save redundant re-testing of the
> mid level?
It avoids descending P2M_MID_PER_PAGE times per p2m_mis_missing top
level entry into leaf entries which we know are going to be p2m_missing
because they are referred to by p2m_mid_missing. Perhaps its a premature
optimisation, if the test and increment end up too complex once I've
fixed them I may just drop it.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home