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

Re: [Xen-devel] [PATCH for-4.5 v6 10/17] xen/arm: p2m type definitions and changes





On Tue, Sep 16, 2014 at 10:38 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
Hi Ian,


On 16/09/14 06:27, Ian Campbell wrote:
On Tue, 2014-09-16 at 10:49 +0200, Tamas K Lengyel wrote:


On Tue, Sep 16, 2014 at 12:35 AM, Ian Campbell
<ian.campbell@xxxxxxxxxx> wrote:
         On Mon, 2014-09-15 at 16:02 +0200, Tamas K Lengyel wrote:
         > Define p2m_access_t in ARM and add necessary changes for
         page table
         > construction routines to pass the default access
         information. Also,
         > define the Radix tree that will hold access permission
         settings as
         > the PTE's don't have enough software programmable bits
         available.

         So my main concern here is the overhead for non-xenaccess
         users. I think
         it amounts to a few extra fields in the p2m_domain struct
         which I can
         see here and presumably some NULL vs. non-NULL type checks
         which I guess
         we will get to later. The important thing is that the fast
         paths for the
         common case don't get a lot of extra overhead.

         WRT the xenaccess performance did you consider any options
         other than a
         radix tree (which seems quite expensive to me)? e.g. perhaps
         allocating
         (only when needed) as second page for each real T page as a
         shadow/extended region? Perhaps pointed to by a filed in the
         real PT
         struct page_info. I'm sure there are other possible ideas too.

         Ian,


Those would all be possible solutions. I used the Radix tree
implementation already in Xen as a matter of convenience and because
it has an acceptable overhead. I certainly don't oppose further
optimizing this code, I'm just not sure if it needs to happen now,
provided feature freeze is rapidly approaching. The main concern
should be impact on non-xenaccess code-paths, which I agree are a
showstopper of any feature like this. If you say it can't be merged
unless the xenaccess code-path is also optimized I'm afraid this
series will be postponed till 4.6 as I won't have the time to test out
which approach puts the least overhead on the system under what
usage-scenarios etc in this timeframe.

WRT merging I'm only concerned about the impact on non-xenaccess uses,
the stuff about the xenaccess-on case was just idle wondering, sorry for
not making that clear.

I didn't write this code, but read it multiple time and ask the similar question to Tamas few version ago.

So, the radix tree is only used when the access type of the page is different than access_rwx. This is the default access when xenaccess is not used.

AFAIU, the only overhead we have is the new fields in the arch_domain structure.

Regards,

--
Julien Grall


Yeap, that's the basic outline of things (except that one thing I mentioned in the other thread, which is already fixed in v7 of the series). I looked into switching to an extended struct page_info and that looks like a good alternative to consider. It seems to be like a faster lookup to get that struct with get_page_from_gva then the radix lookup if the radix tree is large.

Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

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