On 8/7/25 5:57 PM, Jan Beulich wrote:
On 07.08.2025 15:35, Oleksii Kurochko wrote:
On 8/5/25 12:43 PM, Jan Beulich wrote:
On 31.07.2025 17:58, Oleksii Kurochko wrote:
+static int p2m_alloc_root_table(struct p2m_domain *p2m)
+{
+ struct domain *d = p2m->domain;
+ struct page_info *page;
+ const unsigned int nr_root_pages = P2M_ROOT_PAGES;
+
+ /*
+ * Return back nr_root_pages to assure the root table memory is also
+ * accounted against the P2M pool of the domain.
+ */
+ if ( !paging_ret_pages_to_domheap(d, nr_root_pages) )
+ return -ENOMEM;
+
+ page = p2m_allocate_root(d);
+ if ( !page )
+ return -ENOMEM;
+
+ p2m->root = page;
+
+ return 0;
+}
In the success case, shouldn't you bump the paging pool's total_pages by
P2M_ROOT_PAGES? (As the freeing side is missing so far, it's not easy to
tell whether there's [going to be] a balancing problem in the long run.
In the short run there certainly is.)
I think that total_pages should be updated only in case when page is added
to freelist.
In the case of p2m root table, we just returning some pages to domheap and
durint that decreasing an amount of total_pages as freelist has lesser pages,
and then just allocate pages from domheap without adding them to freelist.
But how's freeing of a root table going to look like?
We have saved pointer to first page of P2M_ROOT_PAGES allocated for root page
table which is stored in p2m->root. Then when a domain is going to be destroyed,
then do something like:
for ( i = 0; i < P2M_ROOT_PAGES; i++ )
clear_and_clean_page(p2m->root + i);
...
Logically that group
of 4 pages would be put back into the pool. And from that the pool's
total_pages should reflect that right after successful allocation.
... I think instead of having the loop mentioned above we could add root table
pages to p2m->pages (as you suggested) in p2m_allocate_root() and then a domain
is being destroyed just do the following:
while ( (pg = page_list_remove_head(&p2m->pages)) )
{
p2m_free_page(p2m->domain, pg);
And it will be a job of internals of p2m_free_page() -> paging_free_page() to
adjust freelist's total_pages and return back page(s) allocated for root table
to the freelist. (Note: the current implementation of paging_free_page() just
add a page to freelist without updating of freelist's total_pages what looks
incorrect. And it will be enough as total_pages is present only for freelist
and there is not separate total_pages (or something similar) for p2m->pages).
~ Oleksii
