On 7/28/25 11:09 AM, Jan Beulich wrote:
On 28.07.2025 10:52, Oleksii Kurochko wrote:
On 7/23/25 11:46 AM, Jan Beulich wrote:
I assume that I have in this case to take some pages for an intermediate page
table from freelist P2M pool, set an owner domain to NULL (pg->inuse.domain =
NULL).
Then in this case it isn't clear why pg->list can't be re-used to link several
pages
for intermediate page table purposes + metadata? Is it because pg->list can be
not
empty? In this case it isn't clear if I could use a page, which has threaded
pages.
Actually looks like I was mis-remembering. Pages removed from freelist indeed
aren't put on any other list, so the linking fields are available for use. I
guess I had x86 shadow code in mind, where the linking fields are further used.
Perhaps, I misunderstood you about "linking fields", but it seems like I can't
reuse
struct page_info->list as it is used by page_list_add() which is called by
p2m_alloc_page()
to allocate page(s) for an intermediate page table:
static inline void
page_list_add(struct page_info *page, struct page_list_head *head)
{
list_add(&page->list, head);
}
struct page_info * paging_alloc_page(struct domain *d)
{
struct page_info *pg;
spin_lock(&d->arch.paging.lock);
pg = page_list_remove_head(&d->arch.paging.freelist);
spin_unlock(&d->arch.paging.lock);
INIT_LIST_HEAD(&pg->list);
return pg;
}
static struct page_info *p2m_alloc_page(struct domain *d)
{
struct page_info *pg = paging_alloc_page(d);
if ( pg )
page_list_add(pg, &p2m_get_hostp2m(d)->pages);
return pg;
}
So I have to reuse another field from struct page_info. It seems like it won't
be an
issue if to add a new struct page_list_entry metadata_list to 'union v':
union {
/* Page is in use */
struct {
/* Owner of this page (NULL if page is anonymous). */
struct domain *domain;
} inuse;
/* Page is on a free list. */
struct {
/* Order-size of the free chunk this page is the head of. */
unsigned int order;
} free;
+
+ struct page_list_entry metadata_list;
} v;
Am I missing something?
Well, you're doubling the size of that union then, aren't you? As was mentioned
quite some time ago, struct page_info needs quite a bit of care when you mean
to add new fields there. Question is whether for the purpose here you actually
need a doubly-linked list. A single pointer would be fine to put there.
Agree, a single pointer will be more then enough.
I'm thinking if it is possible to do something with the case if someone will try
to use:
#define page_get_owner(p) (p)->v.inuse.domain
for a page which was allocated for metadata storage. Shouldn't I have a separate
list for such pages and a macro which will check if a page is in this list?
Similar a list which we have for p2m pages in struct p2m_domain:
...
/* Pages used to construct the p2m */
struct page_list_head pages;
...
Of course, such pages are allocated by alloc_domheap_page(d, MEMF_no_owner),
so there is no owner. But if someone will accidentally use this macro for such
pages then it will be an issue as ->domain likely won't be a NULL anymore.
~ Oleksii
