On Fri, Jan 04, 2013 at 02:19:17PM -0800, Yinghai Lu wrote: > On Fri, Jan 4, 2013 at 1:19 PM, Borislav Petkov <b...@alien8.de> wrote: > > On Thu, Jan 03, 2013 at 04:48:24PM -0800, Yinghai Lu wrote: > >> +int kernel_mapping_init(pgd_t *pgd_page, unsigned long addr, unsigned > >> long end) > >> +{ > >> + struct x86_mapping_info info = { > >> + .alloc_pgt_page = alloc_pgt_page, > >> + .pmd_flag = __PAGE_KERNEL_LARGE, > >> + .kernel_mapping = true, > >> + }; > >> + > >> + return kernel_ident_mapping_init(&info, pgd_page, addr, end); > > > > This patch looks good so far except this: > > kernel_ident_mapping_init says it initializes ident mapping but > > this is wrong and the type of mapping is actually controlled by > > info.kernel_mapping. > > it is not wrong, and it could do two things. > kernel_mapping > ident_mapping
I know that. But it has kernel_ident_mapping in the name although it can do both. IMO, it should be like this: int kernel_mapping_init_range(pgd_t *pgd_page, unsigned long addr, unsigned long end) ... return kernel_mapping_init(&info, pgd_page, addr, end); so that your workhorse is kernel_mapping_init and it can do both kernel and ident mapping and the wrapper is kernel_mapping_init_range() which gets a range of (addr, end), preps the &info descriptor and calls the workhorse. See what I mean? > > So this function which gets &info, etc should be called > > kernel_mapping_init, AFAICT. And wrt the one wrapping > > kernel_ident_mapping_init, I can't seem to find where it is called. > > What's up? > > this kernel_mapping_init is for -v8 ..., should be dropped if -v7 is > used at last. Well, this *DEFINITELY* should be in the commit message so that reviewers don't go crazy looking for functions used in other branches. Maan, this was absolutely insane. -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/