[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 05/10] xen/arm: introduce alloc_staticmem_pages




On 24/05/2021 11:10, Penny Zheng wrote:
Hi Julien
Hi Penny,

+    if ( !pg )
+        return NULL;
+
+    for ( i = 0; i < nr_pfns; i++)
+    {
+        /*
+         * Reference count must continuously be zero for free pages
+         * of static memory(PGC_reserved).
+         */
+        ASSERT(pg[i].count_info & PGC_reserved);
+        if ( (pg[i].count_info & ~PGC_reserved) != PGC_state_free )
+        {
+            printk(XENLOG_ERR
+                    "Reference count must continuously be zero for free pages"
+                    "pg[%u] MFN %"PRI_mfn" c=%#lx t=%#x\n",
+                    i, mfn_x(page_to_mfn(pg + i)),
+                    pg[i].count_info, pg[i].tlbflush_timestamp);
+            BUG();
So we would crash Xen if the caller pass a wrong range. Is it what we want?

Also, who is going to prevent concurrent access?

Sure, to fix concurrency issue, I may need to add one spinlock like `static
DEFINE_SPINLOCK(staticmem_lock);`

In current alloc_heap_pages, it will do similar check, that pages in free state
MUST have zero reference count. I guess, if condition not met, there is no need
to proceed.

Another thought on concurrency problem, when constructing patch v2, do we need 
to
consider concurrency here?
heap_lock is to take care concurrent allocation on the one heap, but static 
memory is
always reserved for only one specific domain.
In theory yes, but you are relying on the admin to correctly write the device-tree nodes.
You are probably not going to hit the problem today because the domains 
are created one by one. But, as you may want to allocate memory at 
runtime, this is quite important to get the code protected from 
concurrent access.
Here, you will likely want to use the heaplock rather than a new lock. 
So you are also protect against concurrent access to count_info from 
other part of Xen.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.