Re: [PATCH v3 3/8] sched, smp: Trace IPIs sent via send_call_function_single_ipi()

2023-01-07 Thread Ingo Molnar


* Valentin Schneider  wrote:

> send_call_function_single_ipi() is the thing that sends IPIs at the bottom
> of smp_call_function*() via either generic_exec_single() or
> smp_call_function_many_cond(). Give it an IPI-related tracepoint.
> 
> Note that this ends up tracing any IPI sent via __smp_call_single_queue(),
> which covers __ttwu_queue_wakelist() and irq_work_queue_on() "for free".
> 
> Signed-off-by: Valentin Schneider 
> Reviewed-by: Steven Rostedt (Google) 

Acked-by: Ingo Molnar 

Patch series logistics:

 - No objections from the scheduler side, this feature looks pretty useful.

 - Certain patches are incomplete, others are noted as being merged 
   separately, so I presume you'll send an updated/completed series 
   eventually?

 - We can merge this via the scheduler tree I suspect, as most callbacks 
   affected relate to tip:sched/core and tmp:smp/core - but if you have 
   some other preferred tree that's fine too.

Thanks,

Ingo

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH v3] mm: Avoid unnecessary page fault retires on shared memory types

2022-05-27 Thread Ingo Molnar


* Peter Xu  wrote:

> This patch provides a ~12% perf boost on my aarch64 test VM with a simple
> program sequentially dirtying 400MB shmem file being mmap()ed and these are
> the time it needs:
>
>   Before: 650.980 ms (+-1.94%)
>   After:  569.396 ms (+-1.38%)

Nice!

>  arch/x86/mm/fault.c   |  4 ++++

Reviewed-by: Ingo Molnar 

Minor comment typo:

> + /*
> +  * We should do the same as VM_FAULT_RETRY, but let's not
> +  * return -EBUSY since that's not reflecting the reality on
> +  * what has happened - we've just fully completed a page
> +  * fault, with the mmap lock released.  Use -EAGAIN to show
> +  * that we want to take the mmap lock _again_.
> +  */

s/reflecting the reality on what has happened
 /reflecting the reality of what has happened

>   ret = handle_mm_fault(vma, address, fault_flags, NULL);
> +
> + if (ret & VM_FAULT_COMPLETED) {
> + /*
> +  * NOTE: it's a pity that we need to retake the lock here
> +  * to pair with the unlock() in the callers. Ideally we
> +  * could tell the callers so they do not need to unlock.
> +  */
> + mmap_read_lock(mm);
> + *unlocked = true;
> + return 0;

Indeed that's a pity - I guess more performance could be gained here, 
especially in highly parallel threaded workloads?

Thanks,

Ingo

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH V9] mm/debug: Add tests validating architecture page table helpers

2019-11-11 Thread Ingo Molnar


* Anshuman Khandual  wrote:

> +config DEBUG_VM_PGTABLE
> + bool "Debug arch page table for semantics compliance"
> + depends on MMU
> + depends on DEBUG_VM
> + depends on ARCH_HAS_DEBUG_VM_PGTABLE
> + help
> +   This option provides a debug method which can be used to test
> +   architecture page table helper functions on various platforms in
> +   verifying if they comply with expected generic MM semantics. This
> +   will help architecture code in making sure that any changes or
> +   new additions of these helpers still conform to expected
> +   semantics of the generic MM.
> +
> +   If unsure, say N.

Since CONFIG_DEBUG_VM is generally disabled in distro kernel packages, 
why not make this 'default y' to maximize testing coverage? The code size 
increase should be minimal, and DEBUG_VM increases size anyway.

Other than that this looks good to me:

  Reviewed-by: Ingo Molnar 

Thanks,

Ingo

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH V4 2/2] mm/pgtable/debug: Add test validating architecture page table helpers

2019-10-07 Thread Ingo Molnar


* Kirill A. Shutemov  wrote:

> On Mon, Oct 07, 2019 at 03:06:17PM +0200, Ingo Molnar wrote:
> > 
> > * Anshuman Khandual  wrote:
> > 
> > > This adds a test module which will validate architecture page table 
> > > helpers
> > > and accessors regarding compliance with generic MM semantics expectations.
> > > This will help various architectures in validating changes to the existing
> > > page table helpers or addition of new ones.
> > > 
> > > Test page table and memory pages creating it's entries at various level 
> > > are
> > > all allocated from system memory with required alignments. If memory pages
> > > with required size and alignment could not be allocated, then all 
> > > depending
> > > individual tests are skipped.
> > 
> > > diff --git a/arch/x86/include/asm/pgtable_64_types.h 
> > > b/arch/x86/include/asm/pgtable_64_types.h
> > > index 52e5f5f2240d..b882792a3999 100644
> > > --- a/arch/x86/include/asm/pgtable_64_types.h
> > > +++ b/arch/x86/include/asm/pgtable_64_types.h
> > > @@ -40,6 +40,8 @@ static inline bool pgtable_l5_enabled(void)
> > >  #define pgtable_l5_enabled() 0
> > >  #endif /* CONFIG_X86_5LEVEL */
> > >  
> > > +#define mm_p4d_folded(mm) (!pgtable_l5_enabled())
> > > +
> > >  extern unsigned int pgdir_shift;
> > >  extern unsigned int ptrs_per_p4d;
> > 
> > Any deep reason this has to be a macro instead of proper C?
> 
> It's a way to override the generic mm_p4d_folded(). It can be rewritten
> as inline function + define. Something like:
> 
> #define mm_p4d_folded mm_p4d_folded
> static inline bool mm_p4d_folded(struct mm_struct *mm)
> {
>   return !pgtable_l5_enabled();
> }
> 
> But I don't see much reason to be more verbose here than needed.

C type checking? Documentation? Yeah, I know it's just a one-liner, but 
the principle of the death by a thousand cuts applies here.

BTW., any reason this must be in the low level pgtable_64_types.h type 
header, instead of one of the API level header files?

Thanks,

Ingo

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH V4 2/2] mm/pgtable/debug: Add test validating architecture page table helpers

2019-10-07 Thread Ingo Molnar


* Anshuman Khandual  wrote:

> This adds a test module which will validate architecture page table helpers
> and accessors regarding compliance with generic MM semantics expectations.
> This will help various architectures in validating changes to the existing
> page table helpers or addition of new ones.
> 
> Test page table and memory pages creating it's entries at various level are
> all allocated from system memory with required alignments. If memory pages
> with required size and alignment could not be allocated, then all depending
> individual tests are skipped.

> diff --git a/arch/x86/include/asm/pgtable_64_types.h 
> b/arch/x86/include/asm/pgtable_64_types.h
> index 52e5f5f2240d..b882792a3999 100644
> --- a/arch/x86/include/asm/pgtable_64_types.h
> +++ b/arch/x86/include/asm/pgtable_64_types.h
> @@ -40,6 +40,8 @@ static inline bool pgtable_l5_enabled(void)
>  #define pgtable_l5_enabled() 0
>  #endif /* CONFIG_X86_5LEVEL */
>  
> +#define mm_p4d_folded(mm) (!pgtable_l5_enabled())
> +
>  extern unsigned int pgdir_shift;
>  extern unsigned int ptrs_per_p4d;

Any deep reason this has to be a macro instead of proper C?

> diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
> index 327b3ebf23bf..683131b1ee7d 100644
> --- a/mm/Kconfig.debug
> +++ b/mm/Kconfig.debug
> @@ -117,3 +117,18 @@ config DEBUG_RODATA_TEST
>  depends on STRICT_KERNEL_RWX
>  ---help---
>This option enables a testcase for the setting rodata read-only.
> +
> +config DEBUG_ARCH_PGTABLE_TEST
> + bool "Test arch page table helpers for semantics compliance"
> + depends on MMU
> + depends on DEBUG_KERNEL
> + depends on !(ARM || IA64)

Please add a proper enabling switch for architectures to opt in.

Please also add it to Documentation/features/list-arch.sh so that it's 
listed as a 'TODO' entry on architectures where the tests are not enabled 
yet.

> + help
> +   This options provides a kernel module which can be used to test
> +   architecture page table helper functions on various platform in
> +   verifying if they comply with expected generic MM semantics. This
> +   will help architectures code in making sure that any changes or
> +   new additions of these helpers will still conform to generic MM
> +   expected semantics.

Typos and grammar fixed:

help
  This option provides a kernel module which can be used to test
  architecture page table helper functions on various platforms in
  verifying if they comply with expected generic MM semantics. This
  will help architecture code in making sure that any changes or
  new additions of these helpers still conform to expected 
  semantics of the generic MM.

Also, more fundamentally: isn't a kernel module too late for such a debug 
check, should something break due to a core MM change? Have these debug 
checks caught any bugs or inconsistencies before?

Why not call this as some earlier MM debug check, after enabling paging 
but before executing user-space binaries or relying on complex MM ops 
within the kernel, called at a stage when those primitives are all 
expected to work fine?

It seems to me that arch_pgtable_tests_init) won't even context-switch 
normally, right?

Finally, instead of inventing yet another randomly named .config debug 
switch, please fit it into the regular MM debug options which go along 
the CONFIG_DEBUG_VM* naming scheme.

Might even make sense to enable these new debug checks by default if 
CONFIG_DEBUG_VM=y, that way we'll get a *lot* more debug coverage than 
some random module somewhere that few people will know about, let alone 
run.

Thanks,

Ingo

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH 12/26] x86: clean up ioremap

2019-08-17 Thread Ingo Molnar


* Christoph Hellwig  wrote:

> Use ioremap as the main implemented function, and defined
> ioremap_nocache to it as a deprecated alias.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  arch/x86/include/asm/io.h | 8 ++--
>  arch/x86/mm/ioremap.c | 8 
>  arch/x86/mm/pageattr.c| 4 ++--
>  3 files changed, 8 insertions(+), 12 deletions(-)

Acked-by: Ingo Molnar 

Thanks,

Ingo

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH] Extract initrd free logic from arch-specific code.

2018-04-02 Thread Ingo Molnar

* Shea Levy  wrote:

> > Please also put it into Documentation/features/.
> 
> I switched this patch series (the latest revision v6 was just posted) to
> using weak symbols instead of Kconfig. Does it still warrant documentation?

Probably not.

Thanks,

Ingo

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: [PATCH] Extract initrd free logic from arch-specific code.

2018-03-30 Thread Ingo Molnar

* Shea Levy  wrote:

> Now only those architectures that have custom initrd free requirements
> need to define free_initrd_mem.
> 
> Signed-off-by: Shea Levy 

Please put the Kconfig symbol name this patch introduces both into the title, 
so 
that people know what to grep for.

> ---
>  arch/alpha/mm/init.c  |  8 
>  arch/arc/mm/init.c|  7 ---
>  arch/arm/Kconfig  |  1 +
>  arch/arm64/Kconfig|  1 +
>  arch/blackfin/Kconfig |  1 +
>  arch/c6x/mm/init.c|  7 ---
>  arch/cris/Kconfig |  1 +
>  arch/frv/mm/init.c| 11 ---
>  arch/h8300/mm/init.c  |  7 ---
>  arch/hexagon/Kconfig  |  1 +
>  arch/ia64/Kconfig |  1 +
>  arch/m32r/Kconfig |  1 +
>  arch/m32r/mm/init.c   | 11 ---
>  arch/m68k/mm/init.c   |  7 ---
>  arch/metag/Kconfig|  1 +
>  arch/microblaze/mm/init.c |  7 ---
>  arch/mips/Kconfig |  1 +
>  arch/mn10300/Kconfig  |  1 +
>  arch/nios2/mm/init.c  |  7 ---
>  arch/openrisc/mm/init.c   |  7 ---
>  arch/parisc/mm/init.c |  7 ---
>  arch/powerpc/mm/mem.c |  7 ---
>  arch/riscv/mm/init.c  |  6 --
>  arch/s390/Kconfig |  1 +
>  arch/score/Kconfig|  1 +
>  arch/sh/mm/init.c |  7 ---
>  arch/sparc/Kconfig|  1 +
>  arch/tile/Kconfig |  1 +
>  arch/um/kernel/mem.c  |  7 ---
>  arch/unicore32/Kconfig|  1 +
>  arch/x86/Kconfig  |  1 +
>  arch/xtensa/Kconfig   |  1 +
>  init/initramfs.c  |  7 +++
>  usr/Kconfig   |  4 
>  34 files changed, 28 insertions(+), 113 deletions(-)

Please also put it into Documentation/features/.

> diff --git a/usr/Kconfig b/usr/Kconfig
> index 43658b8a975e..7a94f6df39bf 100644
> --- a/usr/Kconfig
> +++ b/usr/Kconfig
> @@ -233,3 +233,7 @@ config INITRAMFS_COMPRESSION
>   default ".lzma" if RD_LZMA
>   default ".bz2"  if RD_BZIP2
>   default ""
> +
> +config HAVE_ARCH_FREE_INITRD_MEM
> + bool
> + default n

Help text would be nice, to tell arch maintainers what the purpose of this 
switch 
is.

Also, a nit, I think this should be named "ARCH_HAS_FREE_INITRD_MEM", which is 
the 
dominant pattern:

triton:~/tip> git grep 'select.*ARCH' arch/x86/Kconfig* | cut -f2 | cut -d_ 
-f1-2 | sort | uniq -c | sort -n
...
  2 select ARCH_USES
  2 select ARCH_WANTS
  3 select ARCH_MIGHT
  3 select ARCH_WANT
  4 select ARCH_SUPPORTS
  4 select ARCH_USE
 16 select HAVE_ARCH
 23 select ARCH_HAS

It also reads nicely in English:

  "arch has free_initrd_mem()"

While the other makes little sense:

  "have arch free_initrd_mem()"

?

Thanks,

Ingo

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


Re: Build failures in -next due to 'locking/atomic, arch/arc: Implement atomic_fetch_{add,sub,and,andnot,or,xor}()'

2016-06-17 Thread Ingo Molnar

* Peter Zijlstra  wrote:

> On Fri, Jun 17, 2016 at 04:39:42PM +0200, Peter Zijlstra wrote:
> > On Fri, Jun 17, 2016 at 07:36:56AM -0700, Guenter Roeck wrote:
> > > Hi Peter,
> > > 
> > > I am seeing build failures in -next when trying to build arc / arcv2 
> > > targets.
> > > 
> > > arch/arc/include/asm/atomic.h:74:2: error: ‘SCOND_FAIL_RETRY_VAR_DEF’ 
> > > undeclared
> > > arch/arc/include/asm/atomic.h:87:2: error: expected ‘:’ or ‘)’ before 
> > > ‘SCOND_FAIL_RETRY_ASM’
> > > 
> > > Problems seem to be caused by 'locking/atomic, arch/arc: Implement
> > > atomic_fetch_{add,sub,and,andnot,or,xor}()'. Both 
> > > SCOND_FAIL_RETRY_VAR_DEF and
> > > SCOND_FAIL_RETRY_ASM are undefined.
> > 
> > Crud, I messed up the rebase against the backoff reverts. Lemme go do
> > fixups.
> 
> I've misplaced my arc compiler, but does this make it go again?

There's no arc compiler on korg's cross-building directory.

Thanks,

Ingo

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc