Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync

[Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>]

Marcelo Tosatti wrote:

>>> I'd prefer to not touch it.
>> This patch avoids walk all parents and i think this overload is really unnecessary.
>> It has other tricks in this codepath but i not noticed? :-)
> 
> My point is that there is no point in optimizing something unless its
> performance sensitive.

Hi Marcelo,

I think optimizing not only means 'performance' but also means 'smaller code'(maybe 'cleanup'
is more suitable) and 'logic optimize'(do little things), i'm not sure this patch whether can
improve system performance obviously but it optimize the code logic and reduce code size, and
it not harm other code and system performance, right? :-)

Actually, the origin code has a bug, the code segment in mmu_parent_walk():

|	if (!sp->multimapped && sp->parent_pte) {
|		......
|		return;
|	}
|	hlist_for_each_entry(pte_chain, node, &sp->parent_ptes, link)
|		for (i = 0; i < NR_PTE_CHAIN_ENTRIES; ++i) {
|			......
|		}

So, if sp->parent_pte == NULL, it's unsafe...

> And as i recall, mmu_unsync_walk was much more
> sensitive performance wise than parent walking. Actually, gfn_to_memslot 
> seems more important since its also noticeable on EPT/NPT hosts.

Yeah, i also noticed these and i'm looking into these code.

Thanks,
Xiao