Re: [PATCH 3/7] xen/p2m: put reference for superpage

[Julien Grall <julien@xxxxxxx>]

On 07/05/2024 14:30, Luca Fancellu wrote:
> Hi Julien,
Hi Luca,

> > On 7 May 2024, at 14:20, Julien Grall <julien@xxxxxxx> wrote:
> >
> > Hi Luca,
> >
> > On 23/04/2024 09:25, Luca Fancellu wrote:
> > > From: Penny Zheng <Penny.Zheng@xxxxxxx>
> > > We are doing foreign memory mapping for static shared memory, and
> > > there is a great possibility that it could be super mapped.
> > Is this because we are mapping more than one page at the time? Can you point me 
> > to the code?
> So, to be honest this patch was originally in Penny’s serie, my knowledge of 
> this side of the codebase
> is very limited and so I pushed this one basically untouched.
>
>  From what I can see in the serie the mappings are made in 
> handle_shared_mem_bank, and map_regions_p2mt
> is called for one page at the time (allocated through the function 
> allocate_domheap_memory (new function introduced in
> the serie).
>
> So is that the case that this patch is not needed?
I looked at the code and, if I am not mistake, we are passing 
PFN_DOWN(psize) to map_regions_p2mt. At the moment, it is unclear to me 
why would psize be < PAGE_SIZE.
> > > But today, p2m_put_l3_page could not handle superpages.
> > This was done on purpose. Xen is not preemptible and therefore we need to be 
> > cautious how much work is done within the p2m code.
> >
> > With the below proposal, for 1GB mapping, we may end up to call put_page() up 
> > to 512 * 512 = 262144 times. put_page() can free memory. This could be a very 
> > long operation.
> >
> > Have you benchmark how long it would take?
> I did not, since its purpose was unclear to me and was not commented in the 
> last serie from Penny.
Honestly, I can't remember why it wasn't commented.

Cheers,

--
Julien Grall