Re: [PATCH v3] mm/pdx: Add comments throughout the codebase for pdx

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.07.2023 18:07, Alejandro Vallejo wrote:
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -31,6 +31,17 @@
>   *   (i.e. all devices assigned to) a guest share a single DMA address space
>   *   and, by default, Xen will ensure dfn == pfn.
>   *
> + * pdx: Page InDeX
> + *   Indices into the frame table holding the per-page's book-keeping
> + *   metadata. A compression scheme may be used, so there's a possibly non
> + *   identity mapping between valid(mfn) <-> valid(pdx). See the comments
> + *   in pdx.c for an in-depth explanation of that mapping. This also has a
> + *   knock-on effect on the directmap, as "compressed" pfns have no
> + *   corresponding mapped frames.

Didn't you mean to keep the directmap part optional, which would call for
saying "may" here (twice)? Yet then ...

> --- a/xen/include/xen/pdx.h
> +++ b/xen/include/xen/pdx.h
> @@ -1,6 +1,73 @@
>  #ifndef __XEN_PDX_H__
>  #define __XEN_PDX_H__
>  
> +/*
> + * PDX (Page inDeX)
> + *
> + * This file deals with optimisations pertaining to frame table and
> + * directmap indexing, A pdx is an index into the frame table, which
> + * typically also means an index into the directmap[1]. However, having an
> + * identity relationship between mfn and pdx could waste copious amounts of
> + * memory in empty frame table entries and page tables. There are some
> + * techniques to bring memory wastage down.
> + *
> + * [1] Some ports apply further modifications to a pdx before indexing the
> + *     directmap. This doesn't change the fact that the same compression
> + *     present in the frame table is also present in the directmap
> + *     whenever said map is present.

.. you mention it here as non-optional as well. Didn't you tell me that
Arm doesn't use compressed indexes into the directmap?

Jan