Re: [PATCH 3/4] arch: define CONFIG_PAGE_SIZE_*KB on all architectures

2024-02-27 Thread Heiko Carstens
On Mon, Feb 26, 2024 at 05:14:13PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> Most architectures only support a single hardcoded page size. In order
> to ensure that each one of these sets the corresponding Kconfig symbols,
> change over the PAGE_SHIFT definition to the common one and allow
> only the hardware page size to be selected.
> 
> Signed-off-by: Arnd Bergmann 
> ---
...
>  arch/s390/Kconfig  | 1 +
>  arch/s390/include/asm/page.h   | 2 +-
...
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index fe565f3a3a91..b61c74c10050 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -199,6 +199,7 @@ config S390
>   select HAVE_MOD_ARCH_SPECIFIC
>   select HAVE_NMI
>   select HAVE_NOP_MCOUNT
> + select HAVE_PAGE_SIZE_4KB
>   select HAVE_PCI
>   select HAVE_PERF_EVENTS
>   select HAVE_PERF_REGS
> diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
> index 73b9c3bf377f..ded9548d11d9 100644
> --- a/arch/s390/include/asm/page.h
> +++ b/arch/s390/include/asm/page.h
> @@ -11,7 +11,7 @@
>  #include 
>  #include 
>  
> -#define _PAGE_SHIFT  12
> +#define _PAGE_SHIFT  CONFIG_PAGE_SHIFT

Acked-by: Heiko Carstens 

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


Re: [PATCH v3 00/24] Remove COMMAND_LINE_SIZE from uapi

2023-02-14 Thread Heiko Carstens
On Tue, Feb 14, 2023 at 09:58:17AM +0100, Geert Uytterhoeven wrote:
> Hi Heiko,
> 
> On Tue, Feb 14, 2023 at 9:39 AM Heiko Carstens  wrote:
> > On Tue, Feb 14, 2023 at 08:49:01AM +0100, Alexandre Ghiti wrote:
> > > This all came up in the context of increasing COMMAND_LINE_SIZE in the
> > > RISC-V port.  In theory that's a UABI break, as COMMAND_LINE_SIZE is the
> > > maximum length of /proc/cmdline and userspace could staticly rely on
> > > that to be correct.
> > >
> > > Usually I wouldn't mess around with changing this sort of thing, but
> > > PowerPC increased it with a5980d064fe2 ("powerpc: Bump COMMAND_LINE_SIZE
> > > to 2048").  There are also a handful of examples of COMMAND_LINE_SIZE
> > > increasing, but they're from before the UAPI split so I'm not quite sure
> > > what that means: e5a6a1c90948 ("powerpc: derive COMMAND_LINE_SIZE from
> > > asm-generic"), 684d2fd48e71 ("[S390] kernel: Append scpdata to kernel
> > > boot command line"), 22242681cff5 ("MIPS: Extend COMMAND_LINE_SIZE"),
> > > and 2b74b85693c7 ("sh: Derive COMMAND_LINE_SIZE from
> > > asm-generic/setup.h.").
> > >
> > > It seems to me like COMMAND_LINE_SIZE really just shouldn't have been
> > > part of the uapi to begin with, and userspace should be able to handle
> > > /proc/cmdline of whatever length it turns out to be.  I don't see any
> > > references to COMMAND_LINE_SIZE anywhere but Linux via a quick Google
> > > search, but that's not really enough to consider it unused on my end.
> > >
> > > The feedback on the v1 seemed to indicate that COMMAND_LINE_SIZE really
> > > shouldn't be part of uapi, so this now touches all the ports.  I've
> > > tried to split this all out and leave it bisectable, but I haven't
> > > tested it all that aggressively.
> >
> > Just to confirm this assumption a bit more: that's actually the same
> > conclusion that we ended up with when commit 3da0243f906a ("s390: make
> > command line configurable") went upstream.
> 
> Commit 622021cd6c560ce7 ("s390: make command line configurable"),
> I assume?

Yes, sorry for that. I got distracted while writing and used the wrong
branch to look this up.

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


Re: [PATCH v3 00/24] Remove COMMAND_LINE_SIZE from uapi

2023-02-14 Thread Heiko Carstens
On Tue, Feb 14, 2023 at 08:49:01AM +0100, Alexandre Ghiti wrote:
> This all came up in the context of increasing COMMAND_LINE_SIZE in the
> RISC-V port.  In theory that's a UABI break, as COMMAND_LINE_SIZE is the
> maximum length of /proc/cmdline and userspace could staticly rely on
> that to be correct.
> 
> Usually I wouldn't mess around with changing this sort of thing, but
> PowerPC increased it with a5980d064fe2 ("powerpc: Bump COMMAND_LINE_SIZE
> to 2048").  There are also a handful of examples of COMMAND_LINE_SIZE
> increasing, but they're from before the UAPI split so I'm not quite sure
> what that means: e5a6a1c90948 ("powerpc: derive COMMAND_LINE_SIZE from
> asm-generic"), 684d2fd48e71 ("[S390] kernel: Append scpdata to kernel
> boot command line"), 22242681cff5 ("MIPS: Extend COMMAND_LINE_SIZE"),
> and 2b74b85693c7 ("sh: Derive COMMAND_LINE_SIZE from
> asm-generic/setup.h.").
> 
> It seems to me like COMMAND_LINE_SIZE really just shouldn't have been
> part of the uapi to begin with, and userspace should be able to handle
> /proc/cmdline of whatever length it turns out to be.  I don't see any
> references to COMMAND_LINE_SIZE anywhere but Linux via a quick Google
> search, but that's not really enough to consider it unused on my end.
> 
> The feedback on the v1 seemed to indicate that COMMAND_LINE_SIZE really
> shouldn't be part of uapi, so this now touches all the ports.  I've
> tried to split this all out and leave it bisectable, but I haven't
> tested it all that aggressively.

Just to confirm this assumption a bit more: that's actually the same
conclusion that we ended up with when commit 3da0243f906a ("s390: make
command line configurable") went upstream.

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


Re: [PATCH v3 24/24] s390: Remove empty

2023-02-14 Thread Heiko Carstens
On Tue, Feb 14, 2023 at 08:49:25AM +0100, Alexandre Ghiti wrote:
> From: Palmer Dabbelt 
> 
> Signed-off-by: Palmer Dabbelt 
> ---
>  arch/s390/include/asm/setup.h  | 1 -
>  arch/s390/include/uapi/asm/setup.h | 1 -
>  2 files changed, 2 deletions(-)
>  delete mode 100644 arch/s390/include/uapi/asm/setup.h

Acked-by: Heiko Carstens 

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


Re: [PATCH v3] arch: rename all internal names __xchg to __arch_xchg

2023-01-03 Thread Heiko Carstens
On Fri, Dec 30, 2022 at 03:15:52PM +0100, Andrzej Hajda wrote:
> __xchg will be used for non-atomic xchg macro.
> 
> Signed-off-by: Andrzej Hajda 
> Reviewed-by: Arnd Bergmann 
> ---
> v2: squashed all arch patches into one
> v3: fixed alpha/xchg_local, thx to l...@intel.com
> ---
...
>  arch/s390/include/asm/cmpxchg.h  | 4 ++--
> diff --git a/arch/s390/include/asm/cmpxchg.h b/arch/s390/include/asm/cmpxchg.h
> index 84c3f0d576c5b1..efc16f4aac8643 100644
> --- a/arch/s390/include/asm/cmpxchg.h
> +++ b/arch/s390/include/asm/cmpxchg.h
> @@ -14,7 +14,7 @@
>  
>  void __xchg_called_with_bad_pointer(void);
>  
> -static __always_inline unsigned long __xchg(unsigned long x,
> +static __always_inline unsigned long __arch_xchg(unsigned long x,
>   unsigned long address, int size)

Please adjust the alignment of the second line.

> @@ -77,7 +77,7 @@ static __always_inline unsigned long __xchg(unsigned long x,
>   __typeof__(*(ptr)) __ret;   \
>   \
>   __ret = (__typeof__(*(ptr)))\
> - __xchg((unsigned long)(x), (unsigned long)(ptr),\
> + __arch_xchg((unsigned long)(x), (unsigned long)(ptr),   \
>  sizeof(*(ptr))); \

Same here.

The same is true for a couple of other architectures - not sure if
they care however.

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


Re: [PATCH] mm: remove kern_addr_valid() completely

2022-10-18 Thread Heiko Carstens
On Tue, Oct 18, 2022 at 03:40:14PM +0800, Kefeng Wang wrote:
> Most architectures(except arm64/x86/sparc) simply return 1 for
> kern_addr_valid(), which is only used in read_kcore(), and it
> calls copy_from_kernel_nofault() which could check whether the
> address is a valid kernel address, so no need kern_addr_valid(),
> let's remove unneeded kern_addr_valid() completely.
> 
> Signed-off-by: Kefeng Wang 
> ---
...
>  arch/s390/include/asm/pgtable.h   |  2 -

For s390:
Acked-by: Heiko Carstens 

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


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

2022-05-31 Thread Heiko Carstens
On Mon, May 30, 2022 at 02:34:50PM -0400, Peter Xu wrote:
> I observed that for each of the shared file-backed page faults, we're very
> likely to retry one more time for the 1st write fault upon no page.  It's
> because we'll need to release the mmap lock for dirty rate limit purpose
> with balance_dirty_pages_ratelimited() (in fault_dirty_shared_page()).
> 
> Then after that throttling we return VM_FAULT_RETRY.
> 
> We did that probably because VM_FAULT_RETRY is the only way we can return
> to the fault handler at that time telling it we've released the mmap lock.
> 
> However that's not ideal because it's very likely the fault does not need
> to be retried at all since the pgtable was well installed before the
> throttling, so the next continuous fault (including taking mmap read lock,
> walk the pgtable, etc.) could be in most cases unnecessary.
> 
> It's not only slowing down page faults for shared file-backed, but also add
> more mmap lock contention which is in most cases not needed at all.
> 
> To observe this, one could try to write to some shmem page and look at
> "pgfault" value in /proc/vmstat, then we should expect 2 counts for each
> shmem write simply because we retried, and vm event "pgfault" will capture
> that.
> 
> To make it more efficient, add a new VM_FAULT_COMPLETED return code just to
> show that we've completed the whole fault and released the lock.  It's also
> a hint that we should very possibly not need another fault immediately on
> this page because we've just completed it.
> 
> 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%)
> 
> I believe it could help more than that.
> 
> We need some special care on GUP and the s390 pgfault handler (for gmap
> code before returning from pgfault), the rest changes in the page fault
> handlers should be relatively straightforward.
> 
> Another thing to mention is that mm_account_fault() does take this new
> fault as a generic fault to be accounted, unlike VM_FAULT_RETRY.
> 
> I explicitly didn't touch hmm_vma_fault() and break_ksm() because they do
> not handle VM_FAULT_RETRY even with existing code, so I'm literally keeping
> them as-is.
> 
> Acked-by: Geert Uytterhoeven 
> Acked-by: Peter Zijlstra (Intel) 
> Acked-by: Johannes Weiner 
> Acked-by: Vineet Gupta 
> Acked-by: Guo Ren 
> Acked-by: Max Filippov 
> Acked-by: Christian Borntraeger 
> Acked-by: Michael Ellerman  (powerpc)
> Acked-by: Catalin Marinas 
> Reviewed-by: Alistair Popple 
> Reviewed-by: Ingo Molnar 
> Signed-off-by: Peter Xu 
> ---
...
>  arch/s390/mm/fault.c  | 12 
> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index e173b6187ad5..973dcd05c293 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -433,6 +433,17 @@ static inline vm_fault_t do_exception(struct pt_regs 
> *regs, int access)
>   goto out_up;
>   goto out;
>   }
> +
> + /* The fault is fully completed (including releasing mmap lock) */
> + if (fault & VM_FAULT_COMPLETED) {
> + if (gmap) {
> + mmap_read_lock(mm);
> + goto out_gmap;
> + }
> + fault = 0;
> + goto out;
> + }
> +
>   if (unlikely(fault & VM_FAULT_ERROR))
>   goto out_up;
>  
> @@ -452,6 +463,7 @@ static inline vm_fault_t do_exception(struct pt_regs 
> *regs, int access)
>   mmap_read_lock(mm);
>   goto retry;
>   }
> +out_gmap:
>   if (IS_ENABLED(CONFIG_PGSTE) && gmap) {
>   address =  __gmap_link(gmap, current->thread.gmap_addr,
>  address);

FWIW:
Acked-by: Heiko Carstens 

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


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

2022-05-30 Thread Heiko Carstens
On Mon, May 30, 2022 at 12:00:52PM -0400, Peter Xu wrote:
> On Mon, May 30, 2022 at 11:52:54AM -0400, Peter Xu wrote:
> > On Mon, May 30, 2022 at 11:35:10AM +0200, Christian Borntraeger wrote:
> > > > diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> > > > index 4608cc962ecf..e1d40ca341b7 100644
> > > > --- a/arch/s390/mm/fault.c
> > > > +++ b/arch/s390/mm/fault.c
> > > > @@ -436,12 +436,11 @@ static inline vm_fault_t do_exception(struct 
> > > > pt_regs *regs, int access)
> > > > /* The fault is fully completed (including releasing mmap lock) 
> > > > */
> > > > if (fault & VM_FAULT_COMPLETED) {
> > > > -   /*
> > > > -* Gmap will need the mmap lock again, so retake it.  
> > > > TODO:
> > > > -* only conditionally take the lock when CONFIG_PGSTE 
> > > > set.
> > > > -*/
> > > > -   mmap_read_lock(mm);
> > > > -   goto out_gmap;
> > > > +   if (gmap) {
> > > > +   mmap_read_lock(mm);
> > > > +   goto out_gmap;
> > > > +   }
fault = 0;  <
> > > > +   goto out;
> 
> Hmm, right after I replied I found "goto out" could be problematic, since
> all s390 callers of do_exception() will assume it an error condition (side
> note: "goto out_gmap" contains one step to clear "fault" to 0).  I'll
> replace this with "return 0" instead if it looks good to both of you.
> 
> I'll wait for a confirmation before reposting.  Thanks,

Right, that was stupid. Thanks for double checking!

However could you please add "fault = 0" just in front of the goto out
like above? I'd like to avoid having returns and gotos mixed.

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


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

2022-05-29 Thread Heiko Carstens
On Fri, May 27, 2022 at 03:39:36PM -0400, Peter Xu wrote:
> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index e173b6187ad5..4608cc962ecf 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -433,6 +433,17 @@ static inline vm_fault_t do_exception(struct pt_regs 
> *regs, int access)
>   goto out_up;
>   goto out;
>   }
> +
> + /* The fault is fully completed (including releasing mmap lock) */
> + if (fault & VM_FAULT_COMPLETED) {
> + /*
> +  * Gmap will need the mmap lock again, so retake it.  TODO:
> +  * only conditionally take the lock when CONFIG_PGSTE set.
> +  */
> + mmap_read_lock(mm);
> + goto out_gmap;
> + }
> +
>   if (unlikely(fault & VM_FAULT_ERROR))
>   goto out_up;
>  

Guess the patch below on top of your patch is what we want.
Just for clarification: if gmap is not NULL then the process is a kvm
process. So, depending on the workload, this optimization makes sense.

diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 4608cc962ecf..e1d40ca341b7 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -436,12 +436,11 @@ static inline vm_fault_t do_exception(struct pt_regs 
*regs, int access)
 
/* The fault is fully completed (including releasing mmap lock) */
if (fault & VM_FAULT_COMPLETED) {
-   /*
-* Gmap will need the mmap lock again, so retake it.  TODO:
-* only conditionally take the lock when CONFIG_PGSTE set.
-*/
-   mmap_read_lock(mm);
-   goto out_gmap;
+   if (gmap) {
+   mmap_read_lock(mm);
+   goto out_gmap;
+   }
+   goto out;
}
 
if (unlikely(fault & VM_FAULT_ERROR))

___
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 Heiko Carstens
On Tue, May 24, 2022 at 07:45:31PM -0400, Peter Xu wrote:
> I observed that for each of the shared file-backed page faults, we're very
> likely to retry one more time for the 1st write fault upon no page.  It's
> because we'll need to release the mmap lock for dirty rate limit purpose
> with balance_dirty_pages_ratelimited() (in fault_dirty_shared_page()).
> 
> Then after that throttling we return VM_FAULT_RETRY.
> 
> We did that probably because VM_FAULT_RETRY is the only way we can return
> to the fault handler at that time telling it we've released the mmap lock.
> 
> However that's not ideal because it's very likely the fault does not need
> to be retried at all since the pgtable was well installed before the
> throttling, so the next continuous fault (including taking mmap read lock,
> walk the pgtable, etc.) could be in most cases unnecessary.
> 
> It's not only slowing down page faults for shared file-backed, but also add
> more mmap lock contention which is in most cases not needed at all.
> 
> To observe this, one could try to write to some shmem page and look at
> "pgfault" value in /proc/vmstat, then we should expect 2 counts for each
> shmem write simply because we retried, and vm event "pgfault" will capture
> that.
> 
> To make it more efficient, add a new VM_FAULT_COMPLETED return code just to
> show that we've completed the whole fault and released the lock.  It's also
> a hint that we should very possibly not need another fault immediately on
> this page because we've just completed it.
> 
> 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%)
> 
> I believe it could help more than that.
> 
> We need some special care on GUP and the s390 pgfault handler (for gmap
> code before returning from pgfault), the rest changes in the page fault
> handlers should be relatively straightforward.
> 
> Another thing to mention is that mm_account_fault() does take this new
> fault as a generic fault to be accounted, unlike VM_FAULT_RETRY.
> 
> I explicitly didn't touch hmm_vma_fault() and break_ksm() because they do
> not handle VM_FAULT_RETRY even with existing code, so I'm literally keeping
> them as-is.
> 
> Signed-off-by: Peter Xu 
...
> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index e173b6187ad5..9503a7cfaf03 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -339,6 +339,7 @@ static inline vm_fault_t do_exception(struct pt_regs 
> *regs, int access)
>   unsigned long address;
>   unsigned int flags;
>   vm_fault_t fault;
> + bool need_unlock = true;
>   bool is_write;
>  
>   tsk = current;
> @@ -433,6 +434,13 @@ static inline vm_fault_t do_exception(struct pt_regs 
> *regs, int access)
>   goto out_up;
>   goto out;
>   }
> +
> + /* The fault is fully completed (including releasing mmap lock) */
> + if (fault & VM_FAULT_COMPLETED) {
> + need_unlock = false;
> + goto out_gmap;
> + }
> +
>   if (unlikely(fault & VM_FAULT_ERROR))
>   goto out_up;
>  
> @@ -452,6 +460,7 @@ static inline vm_fault_t do_exception(struct pt_regs 
> *regs, int access)
>   mmap_read_lock(mm);
>   goto retry;
>   }
> +out_gmap:
>   if (IS_ENABLED(CONFIG_PGSTE) && gmap) {
>   address =  __gmap_link(gmap, current->thread.gmap_addr,
>  address);
> @@ -466,7 +475,8 @@ static inline vm_fault_t do_exception(struct pt_regs 
> *regs, int access)
>   }
>   fault = 0;
>  out_up:
> - mmap_read_unlock(mm);
> + if (need_unlock)
> + mmap_read_unlock(mm);
>  out:

This seems to be incorrect. __gmap_link() requires the mmap_lock to be
held. Christian, Janosch, or David, could you please check?

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


Re: [PATCH v1 2/4] memblock: allow to specify flags with memblock_add_node()

2021-09-28 Thread Heiko Carstens
On Mon, Sep 27, 2021 at 05:05:16PM +0200, David Hildenbrand wrote:
> We want to specify flags when hotplugging memory. Let's prepare to pass
> flags to memblock_add_node() by adjusting all existing users.
> 
> Note that when hotplugging memory the system is already up and running
> and we don't want to add the memory first and apply flags later: it
> should happen within one memblock call.
> 
> Signed-off-by: David Hildenbrand 
> ---
...
>  arch/s390/kernel/setup.c     | 3 ++-

For s390
Acked-by: Heiko Carstens 

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


Re: [PATCH 2/3] trace: refactor TRACE_IRQFLAGS_SUPPORT in Kconfig

2021-07-31 Thread Heiko Carstens
On Sat, Jul 31, 2021 at 02:22:32PM +0900, Masahiro Yamada wrote:
> Make architectures select TRACE_IRQFLAGS_SUPPORT instead of
> having many defines.
> 
> Signed-off-by: Masahiro Yamada 
> ---
...
>  arch/s390/Kconfig | 1 +
>  arch/s390/Kconfig.debug   | 3 ---

For s390:
Acked-by: Heiko Carstens 

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


Re: [PATCH v3 9/9] asm-generic: reverse GENERIC_{STRNCPY_FROM, STRNLEN}_USER symbols

2021-07-22 Thread Heiko Carstens
On Thu, Jul 22, 2021 at 04:01:39PM +0200, Arnd Bergmann wrote:
> On Thu, Jul 22, 2021 at 3:57 PM Johannes Berg  
> wrote:
> >
> > >
> > > The remaining architectures at the moment are: ia64, mips, parisc,
> > > s390, um and xtensa. We should probably convert these as well, but
> >
> > I'm not sure it makes sense to convert um, the implementation uses
> > strncpy(), and that should use the libc implementation, which is tuned
> > for the actual hardware that the binary is running on, so performance
> > wise that might be better.
> >
> > OTOH, maybe this is all in the noise given the huge syscall overhead in
> > um, so maybe unifying it would make more sense.
> 
> Right, makes sense. I suppose if we end up converting mips and s390,
> we should just do arch/um and the others as well for consistency, even
> if that adds some overhead.

Feel free to add the s390 patch below on top of your series. However
one question: the strncpy_from_user() prototype now comes everywhere
without the __must_check attribute. Is there any reason for that?

At least for s390 I want to keep that.

>From 03ad679909af58e1dc6864f47ce67210f0534c39 Mon Sep 17 00:00:00 2001
From: Heiko Carstens 
Date: Thu, 22 Jul 2021 21:48:18 +0200
Subject: [PATCH] s390: use generic strncpy/strnlen from_user

The s390 variant of strncpy_from_user() is slightly faster than the
generic variant, however convert to the generic variant now to follow
most if not all other architectures.

Converting to the generic variant was already considered a couple of
years ago. See commit f5c8b9601036 ("s390/uaccess: use sane length for
__strncpy_from_user()").

Signed-off-by: Heiko Carstens 
---
 arch/s390/Kconfig   |  2 --
 arch/s390/include/asm/uaccess.h | 18 ++--
 arch/s390/lib/uaccess.c | 52 -
 3 files changed, 2 insertions(+), 70 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index f4f39087cad0..a0e2130f0100 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -75,8 +75,6 @@ config S390
select ARCH_HAS_SET_MEMORY
select ARCH_HAS_STRICT_KERNEL_RWX
select ARCH_HAS_STRICT_MODULE_RWX
-   select ARCH_HAS_STRNCPY_FROM_USER
-   select ARCH_HAS_STRNLEN_USER
select ARCH_HAS_SYSCALL_WRAPPER
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARCH_HAS_VDSO_DATA
diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
index 2316f2440881..9ed9aa37e836 100644
--- a/arch/s390/include/asm/uaccess.h
+++ b/arch/s390/include/asm/uaccess.h
@@ -233,23 +233,9 @@ raw_copy_in_user(void __user *to, const void __user *from, 
unsigned long n);
 /*
  * Copy a null terminated string from userspace.
  */
+long __must_check strncpy_from_user(char *dst, const char __user *src, long 
count);
 
-long __strncpy_from_user(char *dst, const char __user *src, long count);
-
-static inline long __must_check
-strncpy_from_user(char *dst, const char __user *src, long count)
-{
-   might_fault();
-   return __strncpy_from_user(dst, src, count);
-}
-
-unsigned long __must_check __strnlen_user(const char __user *src, unsigned 
long count);
-
-static inline unsigned long strnlen_user(const char __user *src, unsigned long 
n)
-{
-   might_fault();
-   return __strnlen_user(src, n);
-}
+long __must_check strnlen_user(const char __user *src, long count);
 
 /*
  * Zero Userspace
diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c
index 7ec8b1fa0f08..94ca99bde59d 100644
--- a/arch/s390/lib/uaccess.c
+++ b/arch/s390/lib/uaccess.c
@@ -338,55 +338,3 @@ unsigned long __clear_user(void __user *to, unsigned long 
size)
return clear_user_xc(to, size);
 }
 EXPORT_SYMBOL(__clear_user);
-
-static inline unsigned long strnlen_user_srst(const char __user *src,
- unsigned long size)
-{
-   unsigned long tmp1, tmp2;
-
-   asm volatile(
-   "   lghi  0,0\n"
-   "   la%2,0(%1)\n"
-   "   la%3,0(%0,%1)\n"
-   "   slgr  %0,%0\n"
-   "   sacf  256\n"
-   "0: srst  %3,%2\n"
-   "   jo0b\n"
-   "   la%0,1(%3)\n"   /* strnlen_user results includes \0 */
-   "   slgr  %0,%1\n"
-   "1: sacf  768\n"
-   EX_TABLE(0b,1b)
-   : "+a" (size), "+a" (src), "=a" (tmp1), "=a" (tmp2)
-   :
-   : "cc", "memory", "0");
-   return size;
-}
-
-unsigned long __strnlen_user(const char __user *src, unsigned long size)
-{
-   if (unlikely(!size))
-   return 0;
-   return strnlen_user_srst(src, size);
-}
-EXPORT_SYMBOL(__str

Re: Heads up: gcc miscompiling initramfs zlib decompression code at -O3

2021-05-05 Thread Heiko Carstens
On Mon, May 03, 2021 at 10:41:45AM -0700, Linus Torvalds wrote:
> It would be lovely if somebody also took a look at some of the other
> zlib code, like inflate.c itself. But some of that code has rather
> subtle changes for things like s390 hardware support, and we have
> thihngs like our fallthrough rules etc, so it gets a bit hairier.
> 
> Which is why I did just this fairly minimal part.
> 
> Note that the commit message has a "Not-yet-signed-off-by:" from me.
> That's purely because I wanted it to be obvious that this needs a lot
> more testing - I'm not comfy with this patch unless somebody takes it
> upon themselves to actually test it under different loads (and
> different architectures).

The patch below is required on top of your patch to make it compile
for s390 as well.
Tested with kernel image decompression, and also btrfs with file
compression; both software and hardware compression.
Everything seems to work.

diff --git a/lib/zlib_dfltcc/dfltcc_inflate.c b/lib/zlib_dfltcc/dfltcc_inflate.c
index fb60b5a6a1cb..3bb3e431b79f 100644
--- a/lib/zlib_dfltcc/dfltcc_inflate.c
+++ b/lib/zlib_dfltcc/dfltcc_inflate.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: Zlib
 
+#include 
+#include "../zlib_inflate/inftrees.h"
 #include "../zlib_inflate/inflate.h"
 #include "dfltcc_util.h"
 #include "dfltcc.h"
@@ -122,7 +124,7 @@ dfltcc_inflate_action dfltcc_inflate(
 param->cvt = CVT_ADLER32;
 param->sbb = state->bits;
 param->hl = state->whave; /* Software and hardware history formats match */
-param->ho = (state->write - state->whave) & ((1 << HB_BITS) - 1);
+param->ho = (state->wnext - state->whave) & ((1 << HB_BITS) - 1);
 if (param->hl)
 param->nt = 0; /* Honor history for the first block */
 param->cv = state->check;
@@ -137,7 +139,7 @@ dfltcc_inflate_action dfltcc_inflate(
 state->last = cc == DFLTCC_CC_OK;
 state->bits = param->sbb;
 state->whave = param->hl;
-state->write = (param->ho + param->hl) & ((1 << HB_BITS) - 1);
+state->wnext = (param->ho + param->hl) & ((1 << HB_BITS) - 1);
 state->check = param->cv;
 if (cc == DFLTCC_CC_OP2_CORRUPT && param->oesc != 0) {
 /* Report an error if stream is corrupted */
diff --git a/lib/zlib_inflate/inflate.h b/lib/zlib_inflate/inflate.h
index 6ece8efd879b..52314b8b28ea 100644
--- a/lib/zlib_inflate/inflate.h
+++ b/lib/zlib_inflate/inflate.h
@@ -1,3 +1,6 @@
+#ifndef __ZLIB_INFLATE_INFLATE_H
+#define __ZLIB_INFLATE_INFLATE_H
+
 /* inflate.h -- internal inflate state definition
  * Copyright (C) 1995-2016 Mark Adler
  * For conditions of distribution and use, see copyright notice in zlib.h
@@ -123,3 +126,5 @@ struct inflate_state {
 int back;   /* bits back of last unprocessed length/lit */
 unsigned was;   /* initial length of match */
 };
+
+#endif /* __ZLIB_INFLATE_INFLATE_H */
diff --git a/lib/zlib_inflate/inftrees.h b/lib/zlib_inflate/inftrees.h
index fe4307fcfbe3..39d5d90d1258 100644
--- a/lib/zlib_inflate/inftrees.h
+++ b/lib/zlib_inflate/inftrees.h
@@ -1,3 +1,6 @@
+#ifndef __ZLIB_INFLATE_INFTREES_H
+#define __ZLIB_INFLATE_INFTREES_H
+
 /* inftrees.h -- header to use inftrees.c
  * Copyright (C) 1995-2005, 2010 Mark Adler
  * For conditions of distribution and use, see copyright notice in zlib.h
@@ -60,3 +63,5 @@ typedef enum {
 int zlib_inflate_table (codetype type, unsigned short *lens,
  unsigned codes, code **table,
  unsigned *bits, unsigned short *work);
+
+#endif /* __ZLIB_INFLATE_INFTREES_H */

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


Re: [PATCH 0/6] mm: some config cleanups

2021-03-09 Thread Heiko Carstens
On Tue, Mar 09, 2021 at 02:03:04PM +0530, Anshuman Khandual wrote:
> This series contains config cleanup patches which reduces code duplication
> across platforms and also improves maintainability. There is no functional
> change intended with this series. This has been boot tested on arm64 but
> only build tested on some other platforms.
> 
> This applies on 5.12-rc2
> 
> Cc: x...@kernel.org
> Cc: linux-i...@vger.kernel.org
> Cc: linux-s...@vger.kernel.org
> Cc: linux-snps-arc@lists.infradead.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-m...@vger.kernel.org
> Cc: linux-par...@vger.kernel.org
> Cc: linuxppc-...@lists.ozlabs.org
> Cc: linux-ri...@lists.infradead.org
> Cc: linux...@vger.kernel.org
> Cc: linux-fsde...@vger.kernel.org
> Cc: linux...@kvack.org
> Cc: linux-ker...@vger.kernel.org
> 
> Anshuman Khandual (6):
>   mm: Generalize ARCH_HAS_CACHE_LINE_SIZE
>   mm: Generalize SYS_SUPPORTS_HUGETLBFS (rename as ARCH_SUPPORTS_HUGETLBFS)
>   mm: Generalize ARCH_ENABLE_MEMORY_[HOTPLUG|HOTREMOVE]
>   mm: Drop redundant ARCH_ENABLE_[HUGEPAGE|THP]_MIGRATION
>   mm: Drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK
>   mm: Drop redundant HAVE_ARCH_TRANSPARENT_HUGEPAGE
> 
>  arch/arc/Kconfig   |  9 ++--
>  arch/arm/Kconfig   | 10 ++---
>  arch/arm64/Kconfig | 30 ++
>  arch/ia64/Kconfig  |  8 ++-
>  arch/mips/Kconfig  |  6 +-
>  arch/parisc/Kconfig|  5 +
>  arch/powerpc/Kconfig   | 11 ++
>  arch/powerpc/platforms/Kconfig.cputype | 16 +-
>  arch/riscv/Kconfig |  5 +
>  arch/s390/Kconfig  | 12 +++
>  arch/sh/Kconfig|  7 +++---
>  arch/sh/mm/Kconfig |  8 ---
>  arch/x86/Kconfig   | 29 ++---
>  fs/Kconfig |  5 -
>  mm/Kconfig     |  9 
>  15 files changed, 48 insertions(+), 122 deletions(-)

for the s390 bits:
Acked-by: Heiko Carstens 

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


Re: [PATCH 19/21] treewide: add checks for the return value of memblock_alloc*()

2019-01-18 Thread Heiko Carstens
On Wed, Jan 16, 2019 at 03:44:19PM +0200, Mike Rapoport wrote:
> Add check for the return value of memblock_alloc*() functions and call
> panic() in case of error.
> The panic message repeats the one used by panicing memblock allocators with
> adjustment of parameters to include only relevant ones.
> 
> The replacement was mostly automated with semantic patches like the one
> below with manual massaging of format strings.
> 
> @@
> expression ptr, size, align;
> @@
> ptr = memblock_alloc(size, align);
> + if (!ptr)
> + panic("%s: Failed to allocate %lu bytes align=0x%lx\n", __func__,
> size, align);
> 
> Signed-off-by: Mike Rapoport 
...
> diff --git a/arch/s390/numa/toptree.c b/arch/s390/numa/toptree.c
> index 71a608c..0118c77 100644
> --- a/arch/s390/numa/toptree.c
> +++ b/arch/s390/numa/toptree.c
> @@ -31,10 +31,14 @@ struct toptree __ref *toptree_alloc(int level, int id)
>  {
>   struct toptree *res;
> 
> - if (slab_is_available())
> + if (slab_is_available()) {
>   res = kzalloc(sizeof(*res), GFP_KERNEL);
> - else
> + } else {
>   res = memblock_alloc(sizeof(*res), 8);
> + if (!res)
> + panic("%s: Failed to allocate %zu bytes align=0x%x\n",
> +   __func__, sizeof(*res), 8);
> + }
>   if (!res)
>   return res;

Please remove this hunk, since the code _should_ be able to handle
allocation failures anyway (see end of quoted code).

Otherwise for the s390 bits:
Acked-by: Heiko Carstens 


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


Re: [PATCH 1/3] futex: remove duplicated code

2017-03-03 Thread Heiko Carstens
On Fri, Mar 03, 2017 at 01:27:10PM +0100, Jiri Slaby wrote:
> There is code duplicated over all architecture's headers for
> futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
> and comparison of the result.
> 
> Remove this duplication and leave up to the arches only the needed
> assembly which is now in arch_futex_atomic_op_inuser.
> 
> Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
> remove pointless access_ok() checks") as access_ok there returns true.
> We introduce it back to the helper for the sake of simplicity (it gets
> optimized away anyway).
> 
> Signed-off-by: Jiri Slaby <jsl...@suse.cz>
> ---
>  arch/s390/include/asm/futex.h   | 23 -
>  include/asm-generic/futex.h | 50 
> +++--
>  kernel/futex.c  | 36 ++

Looks good to me and still boots on s390. Therefore for the s390 bits:
Acked-by: Heiko Carstens <heiko.carst...@de.ibm.com>

Thanks!


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