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