On Fri, Mar 25, 2022, 7:31 AM Roger Pau Monné <[email protected]> wrote:
> On Fri, Mar 25, 2022 at 07:15:59AM -0400, Tamas K Lengyel wrote: > > On Fri, Mar 25, 2022, 6:59 AM Roger Pau Monné <[email protected]> > wrote: > > > > > On Tue, Mar 22, 2022 at 01:41:37PM -0400, Tamas K Lengyel wrote: > > > > Add option to the fork memop to skip populating the fork with special > > > pages. > > > > These special pages are only necessary when setting up forks to be > fully > > > > functional with a toolstack. For short-lived forks where no > toolstack is > > > active > > > > these pages are uneccesary. > > > > > > Replying here because there's no cover letter AFAICT. > > > > > > For this kind of performance related changes it would be better if you > > > could provide some figures about the performance impact. It would help > > > if we knew whether avoiding mapping the vAPIC page means you can > > > create 0.1% more forks per-minute or 20%. > > > > > > If you really want to speed up the forking path it might be good to > start > > > by perf sampling Xen in order to find the bottlenecks? > > > > > > > Sure but for experiment systems I don't think its necessary to collect > that > > data. > > It helps weight whether the extra logic is worth the performance > benefit IMO. Here it might not matter that much since you say there's > also a non-performance reason for the change. > > > There is also a non-performance reason why we want to keep special pages > > from being populated, in cases we really want the forks physmap to start > > empty for better control over its state. There was already a case where > > having special pages mapped in ended up triggering unexpected Xen > behaviors > > leading to chain of events not easy to follow. For example if page 0 gets > > brought in while the vCPU is being created it ends up as a misconfigured > > ept entry if nested virtualization is enabled. That leads to ept > > misconfiguration exits instead of epf faults. Simply enforcing no entry > in > > the physmap until forking is complete eliminates the chance of something > > like that happening again and makes reasoning about the VM's behavior > from > > the start easier. > > Could we have this added to the commit message then, and the option > renamed to 'empty_p2m' or some such. Then you should also ASSERT that > at the end of the fork process the p2m is indeed empty, not sure if > checking d->arch.paging.hap.p2m_pages == 0 would accomplish that? > Sure, I can do that. Thanks, Tamas >
