[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

Hi Ian,

On 15/12/15 10:25, Ian Campbell wrote:
> 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?

remapping is good here.

>> 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.
> ?

This is clearer than what I wrote. I'm happy if you replace it.

>> 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).

I agree with all the comments. Thank you for fixing it.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.