Re: mm_pages_next() question
On Wed, Apr 01, 2009 at 11:40:30AM +0300, Avi Kivity wrote: > Marcelo Tosatti wrote: >> On Sun, Mar 29, 2009 at 03:24:08PM +0300, Avi Kivity wrote: >> static int mmu_pages_next(struct kvm_mmu_pages *pvec, struct mmu_page_path *parents, int i) { int n; for (n = i+1; n < pvec->nr; n++) { struct kvm_mmu_page *sp = pvec->page[n].sp; if (sp->role.level == PT_PAGE_TABLE_LEVEL) { parents->idx[0] = pvec->page[n].idx; return n; } parents->parent[sp->role.level-2] = sp; parents->idx[sp->role.level-1] = pvec->page[n].idx; } return n; } >>> Do we need to break out of the loop if we switch parents during the >>> loop (since that will give us a different mmu_page_path)? Or are >>> callers careful to only pass pvecs which belong to the same shadow >>> page? >>> >> >> This function builds mmu_page_path for a number of pagetable (leaf) >> pages. Whenever the path changes, mmu_page_path will be rebuilt. >> >> The pages in the pvec must be organized as follows: >> >> level4, level3, level2, level1, level1, level1, , level3, level2, >> level1, level1, ... >> >> So you don't have to repeat higher levels for a number of leaf pages. >> > > I'm still missing something. That if () tests for level == > PT_PAGE_TABLE_LEVEL. So it looks like we'll have batch sizes of 4, 1, > 1, 1, ... 3, 1, 1, 1, ...? The input is the pvec array, organized as follows: 4, 3, 2, 1, 1, 1, 2, 1, 1, 3, 2, 1. The output will be: 1 (pvec position 4), 1 (pos 5), 1 (pos 6). All of them with the same path. Then 1 (pos 8), 1 (pos 9). With the same path as before but level 2 being the page in position 7. So the if() tests for level == PT_PAGE_TABLE_LEVEL because these are the pages we're interested in walking (the ones that can be unsync). If we're not PT_PAGE_TABLE_LEVEL, we walk the pvec building mmu_page_path. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mm_pages_next() question
Marcelo Tosatti wrote: On Sun, Mar 29, 2009 at 03:24:08PM +0300, Avi Kivity wrote: static int mmu_pages_next(struct kvm_mmu_pages *pvec, struct mmu_page_path *parents, int i) { int n; for (n = i+1; n < pvec->nr; n++) { struct kvm_mmu_page *sp = pvec->page[n].sp; if (sp->role.level == PT_PAGE_TABLE_LEVEL) { parents->idx[0] = pvec->page[n].idx; return n; } parents->parent[sp->role.level-2] = sp; parents->idx[sp->role.level-1] = pvec->page[n].idx; } return n; } Do we need to break out of the loop if we switch parents during the loop (since that will give us a different mmu_page_path)? Or are callers careful to only pass pvecs which belong to the same shadow page? This function builds mmu_page_path for a number of pagetable (leaf) pages. Whenever the path changes, mmu_page_path will be rebuilt. The pages in the pvec must be organized as follows: level4, level3, level2, level1, level1, level1, , level3, level2, level1, level1, ... So you don't have to repeat higher levels for a number of leaf pages. I'm still missing something. That if () tests for level == PT_PAGE_TABLE_LEVEL. So it looks like we'll have batch sizes of 4, 1, 1, 1, ... 3, 1, 1, 1, ...? -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mm_pages_next() question
On Sun, Mar 29, 2009 at 03:24:08PM +0300, Avi Kivity wrote: >> static int mmu_pages_next(struct kvm_mmu_pages *pvec, >> struct mmu_page_path *parents, >> int i) >> { >> int n; >> >> for (n = i+1; n < pvec->nr; n++) { >> struct kvm_mmu_page *sp = pvec->page[n].sp; >> >> if (sp->role.level == PT_PAGE_TABLE_LEVEL) { >> parents->idx[0] = pvec->page[n].idx; >> return n; >> } >> >> parents->parent[sp->role.level-2] = sp; >> parents->idx[sp->role.level-1] = pvec->page[n].idx; >> } >> >> return n; >> } > > Do we need to break out of the loop if we switch parents during the loop > (since that will give us a different mmu_page_path)? Or are callers > careful to only pass pvecs which belong to the same shadow page? This function builds mmu_page_path for a number of pagetable (leaf) pages. Whenever the path changes, mmu_page_path will be rebuilt. The pages in the pvec must be organized as follows: level4, level3, level2, level1, level1, level1, , level3, level2, level1, level1, ... So you don't have to repeat higher levels for a number of leaf pages. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
mm_pages_next() question
static int mmu_pages_next(struct kvm_mmu_pages *pvec, struct mmu_page_path *parents, int i) { int n; for (n = i+1; n < pvec->nr; n++) { struct kvm_mmu_page *sp = pvec->page[n].sp; if (sp->role.level == PT_PAGE_TABLE_LEVEL) { parents->idx[0] = pvec->page[n].idx; return n; } parents->parent[sp->role.level-2] = sp; parents->idx[sp->role.level-1] = pvec->page[n].idx; } return n; } Do we need to break out of the loop if we switch parents during the loop (since that will give us a different mmu_page_path)? Or are callers careful to only pass pvecs which belong to the same shadow page? -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html