On Fri, 19 Jul 2013 13:40:04 -0700 Dave Hansen <d...@sr71.net> wrote:
> > Andrew, this fixes up the TLB flush vmstats for UP. It's on top > of the previous patch, but I'm happy to combine them and send a > replacement if you'd prefer. > > This also removes the NR_TLB_LOCAL_FLUSH_ONE_KERNEL counter. We > do not have a good API on UP to separate out the kernel from the > non-kernel flushes. It's probably not an important distinction > anyway. > > Compile and boot tested on 64-bit SMP and UP. Compile tested > for x86_32 SMP. > > -- > > The previous patch doing vmstats for TLB flushes effectively > missed UP since arch/x86/mm/tlb.c is only compiled for SMP. > > UP systems do not do remote TLB flushes, so compile those > counters out on UP. > > arch/x86/kernel/cpu/mtrr/generic.c calls __flush_tlb() directly. > This is probably an optimization since both the mtrr code and > __flush_tlb() write cr4. It would probably be safe to make that > a flush_tlb_all() (and then get these statistics), but the mtrr > code is ancient and I'm hesitant to touch it other than to just > stick in the counters. Do we really want to do this? I agree that UP isn't super-important, particularly on x86, and the benefit here is small. Often I mention things just to check that they have been considered. Considered-and-rejected is better than forgot-about-that. > ... > > --- linux.git/include/linux/vm_event_item.h~compile-useless-stats-out-on-up > 2013-07-19 08:21:37.408237538 -0700 > +++ linux.git-davehans/include/linux/vm_event_item.h 2013-07-19 > 09:13:16.903143205 -0700 > @@ -70,11 +70,12 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS > THP_ZERO_PAGE_ALLOC, > THP_ZERO_PAGE_ALLOC_FAILED, > #endif > +#ifdef CONFIG_SMP > NR_TLB_REMOTE_FLUSH, /* cpu tried to flush others' tlbs */ > NR_TLB_REMOTE_FLUSH_RECEIVED,/* cpu received ipi for flush */ > +#endif > NR_TLB_LOCAL_FLUSH_ALL, > NR_TLB_LOCAL_FLUSH_ONE, > - NR_TLB_LOCAL_FLUSH_ONE_KERNEL, > NR_VM_EVENT_ITEMS I was all excited, expecting documentation for these as discussed yesterday, but it was not to be :( > +/* "_up" is for UniProcessor > + * > + * This is a helper for other header functions. *Not* > + * intended to be called directly. All global TLB > + * flushes need to either call this, or do the bump the > + * vm statistics themselves. > + */ Comment seems a bit sickly. Have a pill: --- a/arch/x86/include/asm/tlbflush.h~mm-vmstats-track-tlb-flush-stats-on-up-too-fix +++ a/arch/x86/include/asm/tlbflush.h @@ -85,11 +85,10 @@ static inline void __flush_tlb_one(unsig #ifndef CONFIG_SMP -/* "_up" is for UniProcessor +/* "_up" is for UniProcessor. * - * This is a helper for other header functions. *Not* - * intended to be called directly. All global TLB - * flushes need to either call this, or do the bump the + * This is a helper for other header functions. *Not* intended to be called + * directly. All global TLB flushes need to either call this, or to bump the * vm statistics themselves. */ static inline void __flush_tlb_up(void) -- 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/