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

Re: [PATCH v8 03/15] x86/mm: rewrite virt_to_xen_l*e



Hi Jan,

On 18/08/2020 09:49, Jan Beulich wrote:
On 13.08.2020 19:22, Julien Grall wrote:
Hi,

On 13/08/2020 17:08, Hongyan Xia wrote:
On Fri, 2020-08-07 at 16:05 +0200, Jan Beulich wrote:
On 27.07.2020 16:21, Hongyan Xia wrote:
From: Wei Liu <wei.liu2@xxxxxxxxxx>

Rewrite those functions to use the new APIs. Modify its callers to
unmap
the pointer returned. Since alloc_xen_pagetable_new() is almost
never
useful unless accompanied by page clearing and a mapping, introduce
a
helper alloc_map_clear_xen_pt() for this sequence.

Note that the change of virt_to_xen_l1e() also requires
vmap_to_mfn() to
unmap the page, which requires domain_page.h header in vmap.

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Signed-off-by: Hongyan Xia <hongyxia@xxxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

---
Changed in v8:
- s/virtual address/linear address/.
- BUG_ON() on NULL return in vmap_to_mfn().

The justification for this should be recorded in the description. In

Will do.

reply to v7 I did even suggest how to easily address the issue you
did notice with large pages, as well as alternative behavior for
vmap_to_mfn().

One thing about adding SMALL_PAGES is that vmap is common code and I am
not sure if the Arm side is happy with it.

At the moment, Arm is only using small mapping but I plan to change that soon 
because we have regions that can be fairly big.

Regardless that, the issue with vmap_to_mfn() is rather x86 specific. So I 
don't particularly like the idea to expose such trick in common code.

Even on x86, I think this is not the right approach. Such band-aid will impact 
the performance as, assuming superpages are used, it will take longer to map 
and add pressure on the TLBs.

I am aware that superpages will be useful for LiveUpdate, but is there any use 
cases in upstream?

Superpage use by vmalloc() is purely occasional: You'd have to vmalloc()
2Mb or more _and_ the page-wise allocation ought to return 512
consecutive pages in the right order. Getting 512 consecutive pages is
possible in practice, but with the page allocator allocating top-down it
is very unlikely for them to be returned in increasing-sorted order.
So your assumption here is vmap_to_mfn() can only be called on vmalloc-ed() area. While this may be the case in Xen today, the name clearly suggest it can be called on all vmap-ed region.


If not, could we just use the BUG_ON() and implement correctly vmap_to_mfn() in 
a follow-up?

My main concern with the BUG_ON() is that it detects a problem long after
it was introduced (when the mapping was established). I'd rather see a
BUG_ON() added there if use of MAP_SMALL_PAGES is deemed unsuitable.

From what you wrote, I would agree that vmalloc() is unlikely going to use superpages. But this is not going to solve the underlying problem with the rest of the vmap area.

So are you suggesting to use MAP_SMALL_PAGES for *all* the vmap()?

Cheers,

--
Julien Grall



 


Rackspace

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