On Fri, Feb 10, 2017 at 2:56 PM, Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx> wrote: > On 02/10/2017 04:02 PM, Dmitry Vyukov wrote: >> On Fri, Feb 10, 2017 at 1:15 PM, Andrey Ryabinin >> <aryabinin@xxxxxxxxxxxxx> wrote: >>> >>> >>> On 02/10/2017 02:18 PM, Thomas Gleixner wrote: >>>> On Fri, 10 Feb 2017, Dmitry Vyukov wrote: >>>>> This is the right thing to do per se, but I am concerned that now >>>>> people will just suffers from slow boot (it can take literally >>>>> minutes) and will not realize the root cause nor that it's fixable >>>>> (e.g. with rodata=n) and will probably just blame KASAN for slowness. >>>>> >>>>> Could we default this rodata check to n under KASAN? Or at least print >>>>> some explanatory warning message before doing marking rodata (it >>>>> should be printed right before "hang", so if you stare at it for a >>>>> minute during each boot you realize that it may be related)? Or >>>>> something along these lines. FWIW in my builds I just always disable >>>>> the check. >>>> >>>> That certainly makes sense and we emit such warnings in other places >>>> already (lockdep, trace_printk ...) >>>> >>> >>> Agreed, but perhaps it would be better to make this code faster for KASAN=y? >>> The main problem here is that we have many pgd entries containing kasan_zero_pud values >>> and ptdump walker checks kasan_zero_pud many times. >>> Instead, we could check it only once and skip further kasan_zero_pud's. >>> >>> I can't say I like this hack very much, but it wins me almost 20 seconds of boot time. >>> Any objections? Looks good to me. >> Now I remember that we already discussed it in this thread: >> https://lkml.org/lkml/2016/11/8/775 >> >> Andrey, you proposed: >> >> "I didn't look at any code, but we probably could can remember last >> visited pgd and skip next pgd if it's the same as previous." >> >> Do you still think it's a good idea? > > Ah, indeed. It will do roughly the same but with less of code churn, see bellow. > >> Walking the same pgd multiple times does not make sense (right?). And >> it could probably speedup non-kasan builds to some degree in some >> contexts. And the code will be free of additional ifdefs. >> > > We could make it without ifdefs but this would be useless for KASAN=n > as page table entries normally unique. So I'm thinking to add #ifdef > at least for documentation purposes. > > > > diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c > index 8aa6bea..1599a5c 100644 > --- a/arch/x86/mm/dump_pagetables.c > +++ b/arch/x86/mm/dump_pagetables.c > @@ -373,6 +373,11 @@ static inline bool is_hypervisor_range(int idx) > #endif > } > > +static bool pgd_already_checked(pgd_t *prev_pgd, pgd_t *pgd, bool checkwx) > +{ > + return checkwx && prev_pgd && (pgd_val(*prev_pgd) == pgd_val(*pgd)); > +} > + > static void ptdump_walk_pgd_level_core(struct seq_file *m, pgd_t *pgd, > bool checkwx) > { > @@ -381,6 +386,7 @@ static void ptdump_walk_pgd_level_core(struct seq_file *m, pgd_t *pgd, > #else > pgd_t *start = swapper_pg_dir; > #endif > + pgd_t *prev_pgd = NULL; > pgprotval_t prot; > int i; > struct pg_state st = {}; > @@ -396,7 +402,8 @@ static void ptdump_walk_pgd_level_core(struct seq_file *m, pgd_t *pgd, > > for (i = 0; i < PTRS_PER_PGD; i++) { > st.current_address = normalize_addr(i * PGD_LEVEL_MULT); > - if (!pgd_none(*start) && !is_hypervisor_range(i)) { > + if (!pgd_none(*start) && !is_hypervisor_range(i) && > + !pgd_already_checked(prev_pgd, start, checkwx)) { > if (pgd_large(*start) || !pgd_present(*start)) { > prot = pgd_flags(*start); > note_page(m, &st, __pgprot(prot), 1); > @@ -408,6 +415,7 @@ static void ptdump_walk_pgd_level_core(struct seq_file *m, pgd_t *pgd, > note_page(m, &st, __pgprot(0), 1); > > cond_resched(); > + prev_pgd = start; > start++; > } > > -- > You received this message because you are subscribed to the Google Groups "kasan-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@xxxxxxxxxxxxxxxx. > To post to this group, send email to kasan-dev@xxxxxxxxxxxxxxxx. > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/730837a1-ee6f-9891-0421-93616dd1c4eb%40virtuozzo.com. > For more options, visit https://groups.google.com/d/optout. -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html