Re: [Xen-devel] [PATCH v3 4/4] xen/arm: p2m: Remove translation table when it's empty

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2015-12-01 at 17:52 +0000, Julien Grall wrote:
> Currently, the translation table is left in place even if no entries is

"are in use" (in fact s/inuse/in use/ throughout.

> inuse. Because of how the p2m code has been implemented, replacing a
> translation table by a block (i.e superpage) is not supported. Therefore,
> any mapping of a superpage size will be split in smaller chunks making
  ÂÂ
   remapping ? To make it clear it doesn't apply to an initial mapping?

> the translation less efficient.
> 
> Replacing a table by a block when a new mapping is added would be too
> complicated because it requires to check if all the upper levels are not

            Ârequires us to check

> inuse and free them if necessary.
> 
> Instead, we will remove the empty translation table when the mapping are

                             Âmappings  Âare

> removed. To avoid going through all the table checking if no entry is
> inuse, a counter representing the number of entry currently inuse is
> kept per table translation and updated when an entry change state (i.e

                           Âchanges

> valid <-> invalid).
> 
> As Xen allocates a page for each translation table, it's possible to
> store the counter in the struct page_info. A new field p2m_refcount as

                                   has

> been introduced in the inuse union for this purpose. This is fine as the
> page is only used by the P2M code and nobody touch the other field of

                        touches

> the union type_info.
> 
> For record, type_info has not been used because it would require

 For the record,

> more work to use it properly as Xen on ARM doesn't yet have the concept
> of type.
> 
> Once Xen has finished to remove a mapping and all the reference to each

       Âfinished removing Âa mapping...     Âreferences

> translation table has been updated, the level will be lookup backward to

          have                 looked at backwards?

I'm not sure about that second one, maybe the "the higher levels will be
looked at in turn to check..." is better? Hrm, no that doesn't really work
with the rest of the paragraph.

    Once Xen has finished to remove a mapping and all the references to each
    translation table have been updated, then the higher levels will be
    processed and freed as needed. This will allow to propagate the number
    of references and free multiple translation table at different level in
    one go.

?

> check if we need first need to free an unused translation table at an
> higher level and then the lower levels. This will allow to propagate the
> number of reference and free multiple translation table at different
> level
> in one go.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

The code looks good, I can fixup the commit log if we can agree what to
write for the second and final comments above (hopefully the rest are just
uncontroversial grammar fixes).

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel