* Re: [PATCH] cpuidle/pseries: Fixup CEDE0 latency only for POWER10 onwards
From: Vaidyanathan Srinivasan @ 2021-04-23 15:59 UTC (permalink / raw)
To: Michal Such?nek
Cc: Gautham R. Shenoy, linux-pm, Aneesh Kumar K.V, Daniel Lezcano,
Rafael J. Wysocki, joedecke, linuxppc-dev
In-Reply-To: <20210423073551.GZ6564@kitsune.suse.cz>
* Michal Such?nek <msuchanek@suse.de> [2021-04-23 09:35:51]:
> On Thu, Apr 22, 2021 at 08:37:29PM +0530, Gautham R. Shenoy wrote:
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> >
> > Commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> > CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
> > of the Extended CEDE states advertised by the platform
> >
> > On some of the POWER9 LPARs, the older firmwares advertise a very low
> > value of 2us for CEDE1 exit latency on a Dedicated LPAR. However the
> Can you be more specific about 'older firmwares'?
Hi Michal,
This is POWER9 vs POWER10 difference, not really an obsolete FW. The
key idea behind the original patch was to make the H_CEDE latency and
hence target residency come from firmware instead of being decided by
the kernel. The advantage is such that, different type of systems in
POWER10 generation can adjust this value and have an optimal H_CEDE
entry criteria which balances good single thread performance and
wakeup latency. Further we can have additional H_CEDE state to feed
into the cpuidle.
> Also while this is a performance regression on such firmwares it
> should be fixed by updating the firmware to current version.
>
> Having sub-optimal performance on obsolete firmware should not require a
> kernel workaround, should it?
When we designed and tested this change on POWER9 and POWER10 systems
the values that were set in F/w were working out fine with positive
results in all our micro benchmarks and no regression in context
switch tests. These repeatable results gave us the confidence that we
can go ahead and set the values from F/w and remove the kernel's value
for all future Linux versions.
But where we slipped is the fact that real world workload show
variations in performance and regressions in specific case because we
are favouring H_CEDE state more often than snooze loop. The root
cause is we have to send more IPIs to wakeup now because more cpus
will be in H_CEDE state than before.
This is a performance problem on POWER9 systems where we actually
expected good benefit and also proved them with micro benchmarks, but
later it turned out to have an impact for some workloads. Further the
challenge is not that regressions are severe, it is the fact that on
exact same hardware and firmware end users expect similar or better
performance for everything when updating to a newer kernel and no
regressions.
We have these setting adjusted for POWER10 in F/w and hence behaviour
will be similar when we come from old kernel on P9 to a new kernel on
P10. We did test the reverse also like new kernel on P9 should show
benefit. But as explained, the benefit came at the cost of regressing
in few cases which were discovered later.
Hence this fix is to keep exact same behaviour for POWER9 and use this
F/w driven heuristics only from POWER10.
> It's not like the kernel would crash on the affected firmware.
Correct. We do not have a functional issue, but only a performance
regression observable on certain real workloads.
This is a minor change in cpuidle's H_CEDE usage which will show up
only in certain workload patterns where we need idle CPU threads to
wakeup faster to get the job done as compared to keeping busy CPU
threads in single thread mode to get more execution slices.
This fix is primarily to ensure kernel update does not change H_CEDE
behaviour on same hardware generation there by causing performance
variation and also regression in some case.
Thanks for the questions and comments, I hope this gives additional
context for this fix.
--Vaidy
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/sstep: Add emulation support for ‘setb’ instruction
From: Segher Boessenkool @ 2021-04-23 16:57 UTC (permalink / raw)
To: Gabriel Paubert
Cc: Sathvika Vasireddy, naveen.n.rao, linuxppc-dev, Daniel Axtens
In-Reply-To: <20210423102657.GA22427@lt-gp.iram.es>
On Fri, Apr 23, 2021 at 12:26:57PM +0200, Gabriel Paubert wrote:
> On Thu, Apr 22, 2021 at 06:26:16PM -0500, Segher Boessenkool wrote:
> > > But this can be made jump free :-):
> > >
> > > int tmp = regs->ccr << ra;
> > > op->val = (tmp >> 31) | ((tmp >> 30) & 1);
> >
> > The compiler will do so automatically (or think of some better way to
> > get the same result); in source code, what matters most is readability,
> > or clarity in general (also clarity to the compiler).
>
> I just did a test (trivial code attached) and the original code always
> produces one conditional branch at -O2, at least with the cross-compiler
> I have on Debian (gcc 8.3). I have tested both -m32 and -m64. The 64 bit
> version produces an unnecessary "extsw", so I wrote the second version
> splitting the setting of the return value which gets rid of it.
That is an older compiler, and it will be out-of-service any day now.
It depends on what compiler flags you use, and what version of the ISA
you are targetting.
> The second "if" is fairly simple to optimize and the compiler does it
> properly.
Yeah.
> Of course with my suggestion the compiler does not produce any branch.
> But it needs a really good comment.
Or you could try and help improve the compiler ;-) You can do this
without writing compiler code yourself, by writing up some good
enhancement request in bugzilla.
The wider and more OoO the processors become, the more important it
becomes to have branch-free code, in situations where the branches would
not be well-predictable.
> > (Right shifts of negative numbers are implementation-defined in C,
> > fwiw -- but work like you expect in GCC).
>
> Well, I'm not worried about it, since I'd expect a compiler that does
> logical right shifts on signed valued to break so much code that it
> would be easily noticed (also in the kernel).
Yup. And it *is* defined for signed values, as long as they are
non-negative (the common case).
> > > > Also, even people who write LE have the bigger end on the left normally
> > > > (they just write some things right-to-left, and other things
> > > > left-to-right).
> > >
> > > Around 1985, I had a documentation for the the National's 32032
> > > (little-endian) processor family, and all the instruction encodings were
> > > presented with the LSB on the left and MSB on the right.
> >
> > Ouch! Did they write "regular" numbers with the least significant digit
> > on the left as well?
>
> No, they were not that sadistic!
But more inconsistent :-)
Segher
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
From: Christophe Leroy @ 2021-04-23 17:18 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Christophe Leroy, Joe Lawrence, Kees Cook, Fangrui Song, LKML,
Nicholas Piggin, clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <CAKwvOd=KP5CZ5wOrczC6qPAzN7DdFCJ_XvU6e=zvB3XpQrp_-g@mail.gmail.com>
Le 23/04/2021 à 00:44, Nick Desaulniers a écrit :
> On Wed, Sep 2, 2020 at 11:02 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 02/09/2020 à 19:41, Nick Desaulniers a écrit :
>>> On Wed, Sep 2, 2020 at 5:14 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>>>
>>>> Nick Desaulniers <ndesaulniers@google.com> writes:
>>>>> Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections")
>>>>
>>>> I think I'll just revert that for v5.9 ?
>>>
>>> SGTM; you'll probably still want these changes with some modifications
>>> at some point; vdso32 did have at least one orphaned section, and will
>>> be important for hermetic builds. Seeing crashes in supported
>>> versions of the tools ties our hands at the moment.
>>>
>>
>> Keeping the tool problem aside with binutils 2.26, do you have a way to
>> really link an elf32ppc object when building vdso32 for PPC64 ?
>
> Sorry, I'm doing a bug scrub and found
> https://github.com/ClangBuiltLinux/linux/issues/774 still open (and my
> reply to this thread still in Drafts; never sent). With my patches
> rebased:
> $ file arch/powerpc/kernel/vdso32/vdso32.so
> arch/powerpc/kernel/vdso32/vdso32.so: ELF 32-bit MSB shared object,
> PowerPC or cisco 4500, version 1 (SYSV), dynamically linked, stripped
>
> Are you still using 2.26?
>
> I'm not able to repro Nathan's reported issue from
> https://lore.kernel.org/lkml/20200902052123.GA2687902@ubuntu-n2-xlarge-x86/,
> so I'm curious if I should resend the rebased patches as v2?
One comment on your rebased patch:
> diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h
> index 8542e9bbeead..0bd06ec06aaa 100644
> --- a/arch/powerpc/include/asm/vdso.h
> +++ b/arch/powerpc/include/asm/vdso.h
> @@ -25,19 +25,7 @@ int vdso_getcpu_init(void);
>
> #else /* __ASSEMBLY__ */
>
> -#ifdef __VDSO64__
> -#define V_FUNCTION_BEGIN(name) \
> - .globl name; \
> - name: \
> -
> -#define V_FUNCTION_END(name) \
> - .size name,.-name;
> -
> -#define V_LOCAL_FUNC(name) (name)
> -#endif /* __VDSO64__ */
> -
> -#ifdef __VDSO32__
> -
> +#if defined(__VDSO32__) || defined (__VDSO64__)
You always have either __VDSO32__ or __VDSO64__ so this #if is pointless
> #define V_FUNCTION_BEGIN(name) \
> .globl name; \
> .type name,@function; \
> @@ -47,8 +35,7 @@ int vdso_getcpu_init(void);
> .size name,.-name;
>
> #define V_LOCAL_FUNC(name) (name)
> -
> -#endif /* __VDSO32__ */
> +#endif /* __VDSO{32|64}__ */
>
> #endif /* __ASSEMBLY__ */
>
Christophe
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc: Free fdt on error in elf64_load()
From: Lakshmi Ramasubramanian @ 2021-04-23 17:34 UTC (permalink / raw)
To: robh, dan.carpenter, mpe
Cc: devicetree, kbuild-all, lkp, linuxppc-dev, bauerman, dja
In-Reply-To: <20210421163610.23775-1-nramas@linux.microsoft.com>
On 4/21/21 9:36 AM, Lakshmi Ramasubramanian wrote:
Hi Dan,
> There are a few "goto out;" statements before the local variable "fdt"
> is initialized through the call to of_kexec_alloc_and_setup_fdt() in
> elf64_load(). This will result in an uninitialized "fdt" being passed
> to kvfree() in this function if there is an error before the call to
> of_kexec_alloc_and_setup_fdt().
>
> If there is any error after fdt is allocated, but before it is
> saved in the arch specific kimage struct, free the fdt.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
> arch/powerpc/kexec/elf_64.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
Please review this patch and Patch 2/2.
thanks,
-lakshmi
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 5a569bb51349..02662e72c53d 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
> initrd_len, cmdline);
> if (ret)
> - goto out;
> + goto out_free_fdt;
>
> fdt_pack(fdt);
>
> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> - goto out;
> + goto out_free_fdt;
>
> /* FDT will be freed in arch_kimage_file_post_load_cleanup */
> image->arch.fdt = fdt;
> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> if (ret)
> pr_err("Error setting up the purgatory.\n");
>
> + goto out;
> +
> +out_free_fdt:
> + kvfree(fdt);
> out:
> kfree(modified_cmdline);
> kexec_free_elf_info(&elf_info);
>
> - /*
> - * Once FDT buffer has been successfully passed to kexec_add_buffer(),
> - * the FDT buffer address is saved in image->arch.fdt. In that case,
> - * the memory cannot be freed here in case of any other error.
> - */
> - if (ret && !image->arch.fdt)
> - kvfree(fdt);
> -
> return ret ? ERR_PTR(ret) : NULL;
> }
>
>
^ permalink raw reply
* Re: [PATCH] cpuidle/pseries: Fixup CEDE0 latency only for POWER10 onwards
From: Michal Suchánek @ 2021-04-23 17:45 UTC (permalink / raw)
To: Vaidyanathan Srinivasan
Cc: Gautham R. Shenoy, linux-pm, Aneesh Kumar K.V, Daniel Lezcano,
Rafael J. Wysocki, joedecke, linuxppc-dev
In-Reply-To: <YILu6/GK+RwpskCc@drishya.in.ibm.com>
On Fri, Apr 23, 2021 at 09:29:39PM +0530, Vaidyanathan Srinivasan wrote:
> * Michal Such?nek <msuchanek@suse.de> [2021-04-23 09:35:51]:
>
> > On Thu, Apr 22, 2021 at 08:37:29PM +0530, Gautham R. Shenoy wrote:
> > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > >
> > > Commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> > > CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
> > > of the Extended CEDE states advertised by the platform
> > >
> > > On some of the POWER9 LPARs, the older firmwares advertise a very low
> > > value of 2us for CEDE1 exit latency on a Dedicated LPAR. However the
> > Can you be more specific about 'older firmwares'?
>
> Hi Michal,
>
> This is POWER9 vs POWER10 difference, not really an obsolete FW. The
> key idea behind the original patch was to make the H_CEDE latency and
> hence target residency come from firmware instead of being decided by
> the kernel. The advantage is such that, different type of systems in
> POWER10 generation can adjust this value and have an optimal H_CEDE
> entry criteria which balances good single thread performance and
> wakeup latency. Further we can have additional H_CEDE state to feed
> into the cpuidle.
So all POWER9 machines are affected by the firmware bug where firmware
reports CEDE1 exit latency of 2us and the real latency is 5us which
causes the kernel to prefer CEDE1 too much when relying on the values
supplied by the firmware. It is not about 'older firmware'.
I still think it would be preferrable to adjust the latency value
reported by the firmware to match reality over a kernel workaround.
Thanks
Michal
^ permalink raw reply
* Re: [PATCH] cpuidle/pseries: Fixup CEDE0 latency only for POWER10 onwards
From: Vaidyanathan Srinivasan @ 2021-04-23 18:29 UTC (permalink / raw)
To: Michal Such?nek
Cc: Gautham R. Shenoy, linux-pm, Aneesh Kumar K.V, Daniel Lezcano,
Rafael J. Wysocki, joedecke, linuxppc-dev
In-Reply-To: <20210423174505.GE6564@kitsune.suse.cz>
* Michal Such?nek <msuchanek@suse.de> [2021-04-23 19:45:05]:
> On Fri, Apr 23, 2021 at 09:29:39PM +0530, Vaidyanathan Srinivasan wrote:
> > * Michal Such?nek <msuchanek@suse.de> [2021-04-23 09:35:51]:
> >
> > > On Thu, Apr 22, 2021 at 08:37:29PM +0530, Gautham R. Shenoy wrote:
> > > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > > >
> > > > Commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> > > > CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
> > > > of the Extended CEDE states advertised by the platform
> > > >
> > > > On some of the POWER9 LPARs, the older firmwares advertise a very low
> > > > value of 2us for CEDE1 exit latency on a Dedicated LPAR. However the
> > > Can you be more specific about 'older firmwares'?
> >
> > Hi Michal,
> >
> > This is POWER9 vs POWER10 difference, not really an obsolete FW. The
> > key idea behind the original patch was to make the H_CEDE latency and
> > hence target residency come from firmware instead of being decided by
> > the kernel. The advantage is such that, different type of systems in
> > POWER10 generation can adjust this value and have an optimal H_CEDE
> > entry criteria which balances good single thread performance and
> > wakeup latency. Further we can have additional H_CEDE state to feed
> > into the cpuidle.
>
> So all POWER9 machines are affected by the firmware bug where firmware
> reports CEDE1 exit latency of 2us and the real latency is 5us which
> causes the kernel to prefer CEDE1 too much when relying on the values
> supplied by the firmware. It is not about 'older firmware'.
Correct. All POWER9 systems running Linux as guest LPARs will see
extra usage of CEDE idle state, but not baremetal (PowerNV).
The correct definition of the bug or miss-match in expectation is that
firmware reports wakeup latency from a core/thread wakeup timing, but
not end-to-end time from sending a wakeup event like an IPI using
H_calls and receiving the events on the target. Practically there are
few extra micro-seconds needed after deciding to wakeup a target
core/thread to getting the target to start executing instructions
within the LPAR instance.
> I still think it would be preferrable to adjust the latency value
> reported by the firmware to match reality over a kernel workaround.
Right, practically we can fix for future releases and as such we
targeted this scheme from POWER10 but expected no harm on POWER9 which
proved to be wrong.
We can possibly change this FW value for POWER9, but it is too
expensive and not practical because many release streams exist for
different platforms and further customers are at different streams as
well. We cannot force all of them to update because that blows up
co-dependency matrix.
From a user/customer's view current kernel worked fine, why is
a kernel update changing my performance :(
Looking back, we should have boxed any kernel-firmware dependent
feature like this one to a future releases only. We have all options
open for a future release like POWER10, but on a released product
stream, we have to manage with kernel changes. In this specific case
we should have been very conservative and not allow the kernel to
change behaviour on released products.
Thanks,
Vaidy
^ permalink raw reply
* Re: [PATCH] cpuidle/pseries: Fixup CEDE0 latency only for POWER10 onwards
From: Michal Suchánek @ 2021-04-23 18:42 UTC (permalink / raw)
To: Vaidyanathan Srinivasan
Cc: Gautham R. Shenoy, linux-pm, Aneesh Kumar K.V, Daniel Lezcano,
Rafael J. Wysocki, joedecke, linuxppc-dev
In-Reply-To: <YIMSCjTzcSwjQtRi@drishya.in.ibm.com>
On Fri, Apr 23, 2021 at 11:59:30PM +0530, Vaidyanathan Srinivasan wrote:
> * Michal Such?nek <msuchanek@suse.de> [2021-04-23 19:45:05]:
>
> > On Fri, Apr 23, 2021 at 09:29:39PM +0530, Vaidyanathan Srinivasan wrote:
> > > * Michal Such?nek <msuchanek@suse.de> [2021-04-23 09:35:51]:
> > >
> > > > On Thu, Apr 22, 2021 at 08:37:29PM +0530, Gautham R. Shenoy wrote:
> > > > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > > > >
> > > > > Commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> > > > > CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
> > > > > of the Extended CEDE states advertised by the platform
> > > > >
> > > > > On some of the POWER9 LPARs, the older firmwares advertise a very low
> > > > > value of 2us for CEDE1 exit latency on a Dedicated LPAR. However the
> > > > Can you be more specific about 'older firmwares'?
> > >
> > > Hi Michal,
> > >
> > > This is POWER9 vs POWER10 difference, not really an obsolete FW. The
> > > key idea behind the original patch was to make the H_CEDE latency and
> > > hence target residency come from firmware instead of being decided by
> > > the kernel. The advantage is such that, different type of systems in
> > > POWER10 generation can adjust this value and have an optimal H_CEDE
> > > entry criteria which balances good single thread performance and
> > > wakeup latency. Further we can have additional H_CEDE state to feed
> > > into the cpuidle.
> >
> > So all POWER9 machines are affected by the firmware bug where firmware
> > reports CEDE1 exit latency of 2us and the real latency is 5us which
> > causes the kernel to prefer CEDE1 too much when relying on the values
> > supplied by the firmware. It is not about 'older firmware'.
>
> Correct. All POWER9 systems running Linux as guest LPARs will see
> extra usage of CEDE idle state, but not baremetal (PowerNV).
>
> The correct definition of the bug or miss-match in expectation is that
> firmware reports wakeup latency from a core/thread wakeup timing, but
> not end-to-end time from sending a wakeup event like an IPI using
> H_calls and receiving the events on the target. Practically there are
> few extra micro-seconds needed after deciding to wakeup a target
> core/thread to getting the target to start executing instructions
> within the LPAR instance.
Thanks for the detailed explanation.
Maybe just adding a few microseconds to the reported time would be a
more reasonable workaround than using a blanket fixed value then.
>
> > I still think it would be preferrable to adjust the latency value
> > reported by the firmware to match reality over a kernel workaround.
>
> Right, practically we can fix for future releases and as such we
> targeted this scheme from POWER10 but expected no harm on POWER9 which
> proved to be wrong.
>
> We can possibly change this FW value for POWER9, but it is too
> expensive and not practical because many release streams exist for
> different platforms and further customers are at different streams as
> well. We cannot force all of them to update because that blows up
> co-dependency matrix.
From the user point of view only few firmware release streams exist but
what is packaged in such binaries might be another story.
Thanks
Michal
^ permalink raw reply
* Re: [PATCH] cpuidle/pseries: Fixup CEDE0 latency only for POWER10 onwards
From: Vaidyanathan Srinivasan @ 2021-04-24 7:37 UTC (permalink / raw)
To: Michal Such?nek
Cc: Gautham R. Shenoy, linux-pm, Aneesh Kumar K.V, Daniel Lezcano,
Rafael J. Wysocki, joedecke, linuxppc-dev
In-Reply-To: <20210423184216.GG6564@kitsune.suse.cz>
* Michal Such?nek <msuchanek@suse.de> [2021-04-23 20:42:16]:
> On Fri, Apr 23, 2021 at 11:59:30PM +0530, Vaidyanathan Srinivasan wrote:
> > * Michal Such?nek <msuchanek@suse.de> [2021-04-23 19:45:05]:
> >
> > > On Fri, Apr 23, 2021 at 09:29:39PM +0530, Vaidyanathan Srinivasan wrote:
> > > > * Michal Such?nek <msuchanek@suse.de> [2021-04-23 09:35:51]:
> > > >
> > > > > On Thu, Apr 22, 2021 at 08:37:29PM +0530, Gautham R. Shenoy wrote:
> > > > > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > > > > >
> > > > > > Commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> > > > > > CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
> > > > > > of the Extended CEDE states advertised by the platform
> > > > > >
> > > > > > On some of the POWER9 LPARs, the older firmwares advertise a very low
> > > > > > value of 2us for CEDE1 exit latency on a Dedicated LPAR. However the
> > > > > Can you be more specific about 'older firmwares'?
> > > >
> > > > Hi Michal,
> > > >
> > > > This is POWER9 vs POWER10 difference, not really an obsolete FW. The
> > > > key idea behind the original patch was to make the H_CEDE latency and
> > > > hence target residency come from firmware instead of being decided by
> > > > the kernel. The advantage is such that, different type of systems in
> > > > POWER10 generation can adjust this value and have an optimal H_CEDE
> > > > entry criteria which balances good single thread performance and
> > > > wakeup latency. Further we can have additional H_CEDE state to feed
> > > > into the cpuidle.
> > >
> > > So all POWER9 machines are affected by the firmware bug where firmware
> > > reports CEDE1 exit latency of 2us and the real latency is 5us which
> > > causes the kernel to prefer CEDE1 too much when relying on the values
> > > supplied by the firmware. It is not about 'older firmware'.
> >
> > Correct. All POWER9 systems running Linux as guest LPARs will see
> > extra usage of CEDE idle state, but not baremetal (PowerNV).
> >
> > The correct definition of the bug or miss-match in expectation is that
> > firmware reports wakeup latency from a core/thread wakeup timing, but
> > not end-to-end time from sending a wakeup event like an IPI using
> > H_calls and receiving the events on the target. Practically there are
> > few extra micro-seconds needed after deciding to wakeup a target
> > core/thread to getting the target to start executing instructions
> > within the LPAR instance.
>
> Thanks for the detailed explanation.
>
> Maybe just adding a few microseconds to the reported time would be a
> more reasonable workaround than using a blanket fixed value then.
Yes, that is an option. But that may only reduce the difference
between existing kernel and new kernel unless we make it the same
number. Further we are fixing this in P10 and hence we will have to
add "if(P9) do the compensation" and otherwise take it as is. That
would not be elegant. Given that our goal for P9 platform is to not
introduce changes in H_CEDE entry behaviour, we arrived at this
approach (this small patch) and this also makes it easy to backport to
various distro products.
Thanks,
Vaidy
^ permalink raw reply
* [PATCH] powerpc/kasan: Fix shadow start address with modules
From: Christophe Leroy @ 2021-04-24 10:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Modules are now located before kernel, KASAN area has to
be extended accordingly.
Fixes: 80edc68e0479 ("powerpc/32s: Define a MODULE area below kernel text all the time")
Fixes: 9132a2e82adc ("powerpc/8xx: Define a MODULE area below kernel text")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/kasan.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
index 7355ed05e65e..3c478e5ef24c 100644
--- a/arch/powerpc/include/asm/kasan.h
+++ b/arch/powerpc/include/asm/kasan.h
@@ -19,7 +19,7 @@
#define KASAN_SHADOW_SCALE_SHIFT 3
-#if defined(CONFIG_PPC_BOOK3S_32) && defined(CONFIG_MODULES) && defined(CONFIG_STRICT_KERNEL_RWX)
+#ifdef CONFIG_MODULES
#define KASAN_KERN_START ALIGN_DOWN(PAGE_OFFSET - SZ_256M, SZ_256M)
#else
#define KASAN_KERN_START PAGE_OFFSET
--
2.25.0
^ permalink raw reply related
* [PATCH] kbuild: replace LANG=C with LC_ALL=C
From: Masahiro Yamada @ 2021-04-24 11:48 UTC (permalink / raw)
To: linux-kbuild
Cc: linuxppc-dev, Masahiro Yamada, Mat Martineau, Matthias Maennich,
linux-kernel, mptcp, Paul Mackerras, linux-kselftest,
Matthieu Baerts, Jakub Kicinski, netdev, Shuah Khan,
David S. Miller
LANG gives a weak default to each LC_* in case it is not explicitly
defined. LC_ALL, if set, overrides all other LC_* variables.
LANG < LC_CTYPE, LC_COLLATE, LC_MONETARY, LC_NUMERIC, ... < LC_ALL
This is why documentation such as [1] suggests to set LC_ALL in build
scripts to get the deterministic result.
LANG=C is not strong enough to override LC_* that may be set by end
users.
[1]: https://reproducible-builds.org/docs/locales/
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
arch/powerpc/boot/wrapper | 2 +-
scripts/nsdeps | 2 +-
scripts/recordmcount.pl | 2 +-
scripts/setlocalversion | 2 +-
scripts/tags.sh | 2 +-
tools/testing/selftests/net/mptcp/mptcp_connect.sh | 2 +-
usr/gen_initramfs.sh | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index 41fa0a8715e3..cdb796b76e2e 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -191,7 +191,7 @@ if [ -z "$kernel" ]; then
kernel=vmlinux
fi
-LANG=C elfformat="`${CROSS}objdump -p "$kernel" | grep 'file format' | awk '{print $4}'`"
+LC_ALL=C elfformat="`${CROSS}objdump -p "$kernel" | grep 'file format' | awk '{print $4}'`"
case "$elfformat" in
elf64-powerpcle) format=elf64lppc ;;
elf64-powerpc) format=elf32ppc ;;
diff --git a/scripts/nsdeps b/scripts/nsdeps
index e8ce2a4d704a..04c4b96e95ec 100644
--- a/scripts/nsdeps
+++ b/scripts/nsdeps
@@ -44,7 +44,7 @@ generate_deps() {
for source_file in $mod_source_files; do
sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
offset=$(wc -l ${source_file}.tmp | awk '{print $1;}')
- cat $source_file | grep MODULE_IMPORT_NS | LANG=C sort -u >> ${source_file}.tmp
+ cat $source_file | grep MODULE_IMPORT_NS | LC_ALL=C sort -u >> ${source_file}.tmp
tail -n +$((offset +1)) ${source_file} | grep -v MODULE_IMPORT_NS >> ${source_file}.tmp
if ! diff -q ${source_file} ${source_file}.tmp; then
mv ${source_file}.tmp ${source_file}
diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 867860ea57da..0a7fc9507d6f 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -497,7 +497,7 @@ sub update_funcs
#
# Step 2: find the sections and mcount call sites
#
-open(IN, "LANG=C $objdump -hdr $inputfile|") || die "error running $objdump";
+open(IN, "LC_ALL=C $objdump -hdr $inputfile|") || die "error running $objdump";
my $text;
diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index bb709eda96cd..db941f6d9591 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -126,7 +126,7 @@ scm_version()
fi
# Check for svn and a svn repo.
- if rev=$(LANG= LC_ALL= LC_MESSAGES=C svn info 2>/dev/null | grep '^Last Changed Rev'); then
+ if rev=$(LC_ALL=C svn info 2>/dev/null | grep '^Last Changed Rev'); then
rev=$(echo $rev | awk '{print $NF}')
printf -- '-svn%s' "$rev"
diff --git a/scripts/tags.sh b/scripts/tags.sh
index fd96734deff1..db8ba411860a 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -326,5 +326,5 @@ esac
# Remove structure forward declarations.
if [ -n "$remove_structs" ]; then
- LANG=C sed -i -e '/^\([a-zA-Z_][a-zA-Z0-9_]*\)\t.*\t\/\^struct \1;.*\$\/;"\tx$/d' $1
+ LC_ALL=C sed -i -e '/^\([a-zA-Z_][a-zA-Z0-9_]*\)\t.*\t\/\^struct \1;.*\$\/;"\tx$/d' $1
fi
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 10a030b53b23..1d2a6e7b877c 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -273,7 +273,7 @@ check_mptcp_disabled()
ip netns exec ${disabled_ns} sysctl -q net.mptcp.enabled=0
local err=0
- LANG=C ip netns exec ${disabled_ns} ./mptcp_connect -t $timeout -p 10000 -s MPTCP 127.0.0.1 < "$cin" 2>&1 | \
+ LC_ALL=C ip netns exec ${disabled_ns} ./mptcp_connect -t $timeout -p 10000 -s MPTCP 127.0.0.1 < "$cin" 2>&1 | \
grep -q "^socket: Protocol not available$" && err=1
ip netns delete ${disabled_ns}
diff --git a/usr/gen_initramfs.sh b/usr/gen_initramfs.sh
index 8ae831657e5d..63476bb70b41 100755
--- a/usr/gen_initramfs.sh
+++ b/usr/gen_initramfs.sh
@@ -147,7 +147,7 @@ dir_filelist() {
header "$1"
srcdir=$(echo "$1" | sed -e 's://*:/:g')
- dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n" | LANG=C sort)
+ dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n" | LC_ALL=C sort)
# If $dirlist is only one line, then the directory is empty
if [ "$(echo "${dirlist}" | wc -l)" -gt 1 ]; then
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 2/2] powerpc: Print esr register when hitting Program Interrupt
From: Xiongwei Song @ 2021-04-24 13:51 UTC (permalink / raw)
To: Christophe Leroy
Cc: ravi.bangoria, mikey, Xiongwei Song, 0x7f454c46,
Linux Kernel Mailing List, npiggin, Paul Mackerras, aneesh.kumar,
PowerPC
In-Reply-To: <ac6cf0ae-7565-180e-03b2-5e72f89a823a@csgroup.eu>
On Thu, Apr 22, 2021 at 11:29 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 22/04/2021 à 17:10, Xiongwei Song a écrit :
> > From: Xiongwei Song <sxwjean@gmail.com>
> >
> > The esr register has the details of Program Interrupt on BookE/4xx cpus,
> > printing its value is helpful.
> >
> > Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
> > ---
> > arch/powerpc/kernel/process.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index 5c3830837f3a..664aecf8ee2e 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -1459,6 +1459,7 @@ static bool interrupt_detail_printable(int trap)
> > case INTERRUPT_MACHINE_CHECK:
> > case INTERRUPT_DATA_STORAGE:
> > case INTERRUPT_ALIGNMENT:
> > + case INTERRUPT_PROGRAM:
>
> With this, it will also print the DSISR on 8xx/6xx so it will print garbage.
>
> 8xx/6xx provide the information in SRR1. If you want to proceed, you have to do the same as in ISI:
> Copy the content of SRR1 into regs->dsisr in the assembly handler in head_book3s_32.S and in the
> instruction TLB error handler in head_8xx.S
Good point.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc: Print esr register when hitting Program Interrupt
From: Xiongwei Song @ 2021-04-24 14:21 UTC (permalink / raw)
To: Christophe Leroy
Cc: ravi.bangoria, mikey, Xiongwei Song, 0x7f454c46,
Linux Kernel Mailing List, npiggin, Paul Mackerras, aneesh.kumar,
PowerPC
In-Reply-To: <7b3ad970-707d-ffcd-e4a3-0b6e622db403@csgroup.eu>
On Fri, Apr 23, 2021 at 12:50 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 22/04/2021 à 17:29, Christophe Leroy a écrit :
> >
> >
> > Le 22/04/2021 à 17:10, Xiongwei Song a écrit :
> >> From: Xiongwei Song <sxwjean@gmail.com>
> >>
> >> The esr register has the details of Program Interrupt on BookE/4xx cpus,
> >> printing its value is helpful.
> >>
> >> Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
> >> ---
> >> arch/powerpc/kernel/process.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> >> index 5c3830837f3a..664aecf8ee2e 100644
> >> --- a/arch/powerpc/kernel/process.c
> >> +++ b/arch/powerpc/kernel/process.c
> >> @@ -1459,6 +1459,7 @@ static bool interrupt_detail_printable(int trap)
> >> case INTERRUPT_MACHINE_CHECK:
> >> case INTERRUPT_DATA_STORAGE:
> >> case INTERRUPT_ALIGNMENT:
> >> + case INTERRUPT_PROGRAM:
> >
> > With this, it will also print the DSISR on 8xx/6xx so it will print garbage.
> >
> > 8xx/6xx provide the information in SRR1. If you want to proceed, you have to do the same as in ISI:
> > Copy the content of SRR1 into regs->dsisr in the assembly handler in head_book3s_32.S and in the
> > instruction TLB error handler in head_8xx.S
>
> In fact, we already have get_reason() called by do_program_check() to retrieve the reason of the
> program check exception. Maybe it could be used to print usefull information instead of starting
> doing almost the same is another way.
Yes, there is the get_reason() function. But if the program interrupt
is triggered in kernel mode,
the reason can be lost , see the code below:
335 static bool exception_common(int signr, struct pt_regs *regs, int code,
336 unsigned long addr)
337 {
338 if (!user_mode(regs)) {
339 die("Exception in kernel mode", regs, signr);
340 return false;
341 }
The third parameter(int code) of exception_common is to pass the
reason, when in kernel
mode, the "code" parameter is lost, hence I append INTERRUPT_PROGRAM here.
This is for __show_regs(), so just printing the content of the
register is fine I think.
>
> Or we do as I suggested above, and we remove that get_reason() stuff. But get_reason() is also used
> by the alignment exception. So that doesn't look easy.
>
> I don't know what the best approach is.
Is it acceptable to print the interrupt reason before invoking die()
in exception_common()?
Regards,
Xiongwei
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: Make the code in __show_regs nice-looking
From: Xiongwei Song @ 2021-04-24 14:36 UTC (permalink / raw)
To: Christophe Leroy
Cc: ravi.bangoria, mikey, Xiongwei Song, 0x7f454c46,
Linux Kernel Mailing List, npiggin, Paul Mackerras, aneesh.kumar,
PowerPC
In-Reply-To: <15aac264-8626-ad15-7301-044c622d7f60@csgroup.eu>
On Thu, Apr 22, 2021 at 11:27 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 22/04/2021 à 17:10, Xiongwei Song a écrit :
> > From: Xiongwei Song <sxwjean@gmail.com>
> >
> > Create a new function named interrupt_detail_printable to judge which
> > interrupts can print esr/dsisr register.
>
> What is the benefit of that function ? It may be interesting if the test was done at several places,
> but as it is done at only one place, I don't thing it is an improvement.
>
> Until know, you new immediately what was the traps that would print it. Now you have to go and look
> into a sub-function.
How about replace if statement with switch statement directly, like
the changes below:
@@ -1467,13 +1481,17 @@ static void __show_regs(struct pt_regs *regs)
trap = TRAP(regs);
if (!trap_is_syscall(regs) && cpu_has_feature(CPU_FTR_CFAR))
pr_cont("CFAR: "REG" ", regs->orig_gpr3);
- if (trap == INTERRUPT_MACHINE_CHECK ||
- trap == INTERRUPT_DATA_STORAGE ||
- trap == INTERRUPT_ALIGNMENT) {
+ switch(trap){
+ case INTERRUPT_MACHINE_CHECK:
+ case INTERRUPT_DATA_STORAGE:
+ case INTERRUPT_ALIGNMENT:
if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar,
regs->dsisr);
else
pr_cont("DAR: "REG" DSISR: %08lx ", regs->dar,
regs->dsisr);
+ break;
+ default:
+ break;
}
Thanks,
Xiongwei
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/sstep: Add emulation support for ‘setb’ instruction
From: Daniel Axtens @ 2021-04-24 16:13 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Sathvika Vasireddy, naveen.n.rao, linuxppc-dev
In-Reply-To: <20210422191334.GE27473@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> Hi!
>
> On Fri, Apr 16, 2021 at 05:44:52PM +1000, Daniel Axtens wrote:
>> Sathvika Vasireddy <sathvika@linux.vnet.ibm.com> writes:
>> Ok, if I've understood correctly...
>>
>> > + ra = ra & ~0x3;
>>
>> This masks off the bits of RA that are not part of BTF:
>>
>> ra is in [0, 31] which is [0b00000, 0b11111]
>> Then ~0x3 = ~0b00011
>> ra = ra & 0b11100
>>
>> This gives us then,
>> ra = btf << 2; or
>> btf = ra >> 2;
>
> Yes. In effect, you want the offset in bits of the CR field, which is
> just fine like this. But a comment would not hurt.
>
>> Let's then check to see if your calculations read the right fields.
>>
>> > + if ((regs->ccr) & (1 << (31 - ra)))
>> > + op->val = -1;
>> > + else if ((regs->ccr) & (1 << (30 - ra)))
>> > + op->val = 1;
>> > + else
>> > + op->val = 0;
>
> It imo is clearer if written
>
> if ((regs->ccr << ra) & 0x80000000)
> op->val = -1;
> else if ((regs->ccr << ra) & 0x40000000)
> op->val = 1;
> else
> op->val = 0;
>
> but I guess not everyone agrees :-)
>
>> CR field: 7 6 5 4 3 2 1 0
>> bit: 0123 0123 0123 0123 0123 0123 0123 0123
>> normal bit #: 0.....................................31
>> ibm bit #: 31.....................................0
>
> The bit numbers in CR fields are *always* numbered left-to-right. I
> have never seen anyone use LE for it, anyway.
>
> Also, even people who write LE have the bigger end on the left normally
> (they just write some things right-to-left, and other things
> left-to-right).
Sorry, the numbers in the CR fields weren't meant to be especially
meaningful, I was just trying to convince myself that we referenced the
same bits doing the ISA way vs the way this code did it.
Kind regards,
Daniel
>
>> Checkpatch does have one complaint:
>>
>> CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'regs->ccr'
>> #30: FILE: arch/powerpc/lib/sstep.c:1971:
>> + if ((regs->ccr) & (1 << (31 - ra)))
>>
>> I don't really mind the parenteses: I think you are safe to ignore
>> checkpatch here unless someone else complains :)
>
> I find them annoying. If there are too many parentheses, it is hard to
> see at a glance what groups where. Also, a suspicious reader might
> think there is something special going on (with macros for example).
>
> This is simple code of course, but :-)
>
>> If you do end up respinning the patch, I think it would be good to make
>> the maths a bit clearer. I think it works because a left shift of 2 is
>> the same as multiplying by 4, but it would be easier to follow if you
>> used a temporary variable for btf.
>
> It is very simple. The BFA instruction field is closely related to the
> BI instruction field, which is 5 bits, and selects one of the 32 bits in
> the CR. If you have "BFA00 BFA01 BFA10 BFA11", that gives the bit
> numbers of all four bits in the selected CR field. So the "& ~3" does
> all you need. It is quite pretty :-)
>
>
> Segher
^ permalink raw reply
* Re: [PATCH] cpuidle/pseries: Fixup CEDE0 latency only for POWER10 onwards
From: Michal Suchánek @ 2021-04-25 11:07 UTC (permalink / raw)
To: Vaidyanathan Srinivasan
Cc: Gautham R. Shenoy, linux-pm, Aneesh Kumar K.V, Daniel Lezcano,
Rafael J. Wysocki, joedecke, linuxppc-dev
In-Reply-To: <YIPKrIb+tY39taZv@drishya.in.ibm.com>
On Sat, Apr 24, 2021 at 01:07:16PM +0530, Vaidyanathan Srinivasan wrote:
> * Michal Such?nek <msuchanek@suse.de> [2021-04-23 20:42:16]:
>
> > On Fri, Apr 23, 2021 at 11:59:30PM +0530, Vaidyanathan Srinivasan wrote:
> > > * Michal Such?nek <msuchanek@suse.de> [2021-04-23 19:45:05]:
> > >
> > > > On Fri, Apr 23, 2021 at 09:29:39PM +0530, Vaidyanathan Srinivasan wrote:
> > > > > * Michal Such?nek <msuchanek@suse.de> [2021-04-23 09:35:51]:
> > > > >
> > > > > > On Thu, Apr 22, 2021 at 08:37:29PM +0530, Gautham R. Shenoy wrote:
> > > > > > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > > > > > >
> > > > > > > Commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
> > > > > > > CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
> > > > > > > of the Extended CEDE states advertised by the platform
> > > > > > >
> > > > > > > On some of the POWER9 LPARs, the older firmwares advertise a very low
> > > > > > > value of 2us for CEDE1 exit latency on a Dedicated LPAR. However the
> > > > > > Can you be more specific about 'older firmwares'?
> > > > >
> > > > > Hi Michal,
> > > > >
> > > > > This is POWER9 vs POWER10 difference, not really an obsolete FW. The
> > > > > key idea behind the original patch was to make the H_CEDE latency and
> > > > > hence target residency come from firmware instead of being decided by
> > > > > the kernel. The advantage is such that, different type of systems in
> > > > > POWER10 generation can adjust this value and have an optimal H_CEDE
> > > > > entry criteria which balances good single thread performance and
> > > > > wakeup latency. Further we can have additional H_CEDE state to feed
> > > > > into the cpuidle.
> > > >
> > > > So all POWER9 machines are affected by the firmware bug where firmware
> > > > reports CEDE1 exit latency of 2us and the real latency is 5us which
> > > > causes the kernel to prefer CEDE1 too much when relying on the values
> > > > supplied by the firmware. It is not about 'older firmware'.
> > >
> > > Correct. All POWER9 systems running Linux as guest LPARs will see
> > > extra usage of CEDE idle state, but not baremetal (PowerNV).
> > >
> > > The correct definition of the bug or miss-match in expectation is that
> > > firmware reports wakeup latency from a core/thread wakeup timing, but
> > > not end-to-end time from sending a wakeup event like an IPI using
> > > H_calls and receiving the events on the target. Practically there are
> > > few extra micro-seconds needed after deciding to wakeup a target
> > > core/thread to getting the target to start executing instructions
> > > within the LPAR instance.
> >
> > Thanks for the detailed explanation.
> >
> > Maybe just adding a few microseconds to the reported time would be a
> > more reasonable workaround than using a blanket fixed value then.
>
> Yes, that is an option. But that may only reduce the difference
> between existing kernel and new kernel unless we make it the same
> number. Further we are fixing this in P10 and hence we will have to
> add "if(P9) do the compensation" and otherwise take it as is. That
> would not be elegant. Given that our goal for P9 platform is to not
> introduce changes in H_CEDE entry behaviour, we arrived at this
> approach (this small patch) and this also makes it easy to backport to
> various distro products.
I don't see how this is more elegent.
The current patch is
if(p9)
use fixed value
the suggested patch is
if(p9)
apply compensation
That is either will add one branch for the affected platform.
But I understand if you do not have confidence that the compensation is
the same in all cases and do not have the opportunity to measure it it
may be simpler to apply one very conservative adjustment.
Thanks
Michal
^ permalink raw reply
* Re: [PATCH] kbuild: replace LANG=C with LC_ALL=C
From: Michael Ellerman @ 2021-04-25 11:13 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
Cc: linuxppc-dev, Masahiro Yamada, Mat Martineau, Matthias Maennich,
linux-kernel, netdev, Paul Mackerras, mptcp, linux-kselftest,
Matthieu Baerts, Jakub Kicinski, Shuah Khan, David S. Miller
In-Reply-To: <20210424114841.394239-1-masahiroy@kernel.org>
Masahiro Yamada <masahiroy@kernel.org> writes:
> LANG gives a weak default to each LC_* in case it is not explicitly
> defined. LC_ALL, if set, overrides all other LC_* variables.
>
> LANG < LC_CTYPE, LC_COLLATE, LC_MONETARY, LC_NUMERIC, ... < LC_ALL
>
> This is why documentation such as [1] suggests to set LC_ALL in build
> scripts to get the deterministic result.
>
> LANG=C is not strong enough to override LC_* that may be set by end
> users.
>
> [1]: https://reproducible-builds.org/docs/locales/
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> arch/powerpc/boot/wrapper | 2 +-
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
> scripts/nsdeps | 2 +-
> scripts/recordmcount.pl | 2 +-
> scripts/setlocalversion | 2 +-
> scripts/tags.sh | 2 +-
> tools/testing/selftests/net/mptcp/mptcp_connect.sh | 2 +-
> usr/gen_initramfs.sh | 2 +-
> 7 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
> index 41fa0a8715e3..cdb796b76e2e 100755
> --- a/arch/powerpc/boot/wrapper
> +++ b/arch/powerpc/boot/wrapper
> @@ -191,7 +191,7 @@ if [ -z "$kernel" ]; then
> kernel=vmlinux
> fi
>
> -LANG=C elfformat="`${CROSS}objdump -p "$kernel" | grep 'file format' | awk '{print $4}'`"
> +LC_ALL=C elfformat="`${CROSS}objdump -p "$kernel" | grep 'file format' | awk '{print $4}'`"
> case "$elfformat" in
> elf64-powerpcle) format=elf64lppc ;;
> elf64-powerpc) format=elf32ppc ;;
> diff --git a/scripts/nsdeps b/scripts/nsdeps
> index e8ce2a4d704a..04c4b96e95ec 100644
> --- a/scripts/nsdeps
> +++ b/scripts/nsdeps
> @@ -44,7 +44,7 @@ generate_deps() {
> for source_file in $mod_source_files; do
> sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
> offset=$(wc -l ${source_file}.tmp | awk '{print $1;}')
> - cat $source_file | grep MODULE_IMPORT_NS | LANG=C sort -u >> ${source_file}.tmp
> + cat $source_file | grep MODULE_IMPORT_NS | LC_ALL=C sort -u >> ${source_file}.tmp
> tail -n +$((offset +1)) ${source_file} | grep -v MODULE_IMPORT_NS >> ${source_file}.tmp
> if ! diff -q ${source_file} ${source_file}.tmp; then
> mv ${source_file}.tmp ${source_file}
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 867860ea57da..0a7fc9507d6f 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -497,7 +497,7 @@ sub update_funcs
> #
> # Step 2: find the sections and mcount call sites
> #
> -open(IN, "LANG=C $objdump -hdr $inputfile|") || die "error running $objdump";
> +open(IN, "LC_ALL=C $objdump -hdr $inputfile|") || die "error running $objdump";
>
> my $text;
>
> diff --git a/scripts/setlocalversion b/scripts/setlocalversion
> index bb709eda96cd..db941f6d9591 100755
> --- a/scripts/setlocalversion
> +++ b/scripts/setlocalversion
> @@ -126,7 +126,7 @@ scm_version()
> fi
>
> # Check for svn and a svn repo.
> - if rev=$(LANG= LC_ALL= LC_MESSAGES=C svn info 2>/dev/null | grep '^Last Changed Rev'); then
> + if rev=$(LC_ALL=C svn info 2>/dev/null | grep '^Last Changed Rev'); then
> rev=$(echo $rev | awk '{print $NF}')
> printf -- '-svn%s' "$rev"
>
> diff --git a/scripts/tags.sh b/scripts/tags.sh
> index fd96734deff1..db8ba411860a 100755
> --- a/scripts/tags.sh
> +++ b/scripts/tags.sh
> @@ -326,5 +326,5 @@ esac
>
> # Remove structure forward declarations.
> if [ -n "$remove_structs" ]; then
> - LANG=C sed -i -e '/^\([a-zA-Z_][a-zA-Z0-9_]*\)\t.*\t\/\^struct \1;.*\$\/;"\tx$/d' $1
> + LC_ALL=C sed -i -e '/^\([a-zA-Z_][a-zA-Z0-9_]*\)\t.*\t\/\^struct \1;.*\$\/;"\tx$/d' $1
> fi
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index 10a030b53b23..1d2a6e7b877c 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> @@ -273,7 +273,7 @@ check_mptcp_disabled()
> ip netns exec ${disabled_ns} sysctl -q net.mptcp.enabled=0
>
> local err=0
> - LANG=C ip netns exec ${disabled_ns} ./mptcp_connect -t $timeout -p 10000 -s MPTCP 127.0.0.1 < "$cin" 2>&1 | \
> + LC_ALL=C ip netns exec ${disabled_ns} ./mptcp_connect -t $timeout -p 10000 -s MPTCP 127.0.0.1 < "$cin" 2>&1 | \
> grep -q "^socket: Protocol not available$" && err=1
> ip netns delete ${disabled_ns}
>
> diff --git a/usr/gen_initramfs.sh b/usr/gen_initramfs.sh
> index 8ae831657e5d..63476bb70b41 100755
> --- a/usr/gen_initramfs.sh
> +++ b/usr/gen_initramfs.sh
> @@ -147,7 +147,7 @@ dir_filelist() {
> header "$1"
>
> srcdir=$(echo "$1" | sed -e 's://*:/:g')
> - dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n" | LANG=C sort)
> + dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n" | LC_ALL=C sort)
>
> # If $dirlist is only one line, then the directory is empty
> if [ "$(echo "${dirlist}" | wc -l)" -gt 1 ]; then
> --
> 2.27.0
^ permalink raw reply
* [PATCH] powerpc/kvm: Fix build error when PPC_MEM_KEYS/PPC_PSERIES=n
From: Michael Ellerman @ 2021-04-25 11:58 UTC (permalink / raw)
To: linuxppc-dev
lkp reported a randconfig failure:
In file included from arch/powerpc/include/asm/book3s/64/pkeys.h:6,
from arch/powerpc/kvm/book3s_64_mmu_host.c:15:
arch/powerpc/include/asm/book3s/64/hash-pkey.h: In function 'hash__vmflag_to_pte_pkey_bits':
>> arch/powerpc/include/asm/book3s/64/hash-pkey.h:10:23: error: 'VM_PKEY_BIT0' undeclared
10 | return (((vm_flags & VM_PKEY_BIT0) ? H_PTE_PKEY_BIT0 : 0x0UL) |
| ^~~~~~~~~~~~
We added the include of book3s/64/pkeys.h for pte_to_hpte_pkey_bits(),
but that header on its own should only be included when PPC_MEM_KEYS=y.
Instead include linux/pkeys.h, which brings in the right definitions
when PPC_MEM_KEYS=y and also provides empty stubs when PPC_MEM_KEYS=n.
Fixes: e4e8bc1df691 ("powerpc/kvm: Fix PR KVM with KUAP/MEM_KEYS enabled")
Cc: stable@vger.kernel.org # v5.11+
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kvm/book3s_64_mmu_host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_host.c b/arch/powerpc/kvm/book3s_64_mmu_host.c
index 5ac66be1cb3c..c3e31fef0be1 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_host.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_host.c
@@ -8,11 +8,11 @@
*/
#include <linux/kvm_host.h>
+#include <linux/pkeys.h>
#include <asm/kvm_ppc.h>
#include <asm/kvm_book3s.h>
#include <asm/book3s/64/mmu-hash.h>
-#include <asm/book3s/64/pkeys.h>
#include <asm/machdep.h>
#include <asm/mmu_context.h>
#include <asm/hw_irq.h>
--
2.25.1
^ permalink raw reply related
* PPC32: Boot regression on Nintendo Wii, after create_branch rework in 5.8
From: Jonathan Neuschäfer @ 2021-04-25 15:40 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Emmanuel Gil Peyrot, Jordan Niethe, Alistair Popple
[-- Attachment #1: Type: text/plain, Size: 1598 bytes --]
Hi,
I recently booted my Wii again, and I noticed a regression at boot time.
Output stops after the "Finalizing device tree... flat tree at 0xXXXXXX"
message. I bisected it to this commit in the 5.8 development cycle:
commit 7c95d8893fb55869882c9f68f4c94840dc43f18f
Author: Jordan Niethe <jniethe5@gmail.com>
Date: Wed May 6 13:40:25 2020 +1000
powerpc: Change calling convention for create_branch() et. al.
create_branch(), create_cond_branch() and translate_branch() return the
instruction that they create, or return 0 to signal an error. Separate
these concerns in preparation for an instruction type that is not just
an unsigned int. Fill the created instruction to a pointer passed as
the first parameter to the function and use a non-zero return value to
signify an error.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Alistair Popple <alistair@popple.id.au>
Link: https://lore.kernel.org/r/20200506034050.24806-6-jniethe5@gmail.com
arch/powerpc/include/asm/code-patching.h | 12 +--
arch/powerpc/kernel/optprobes.c | 24 +++---
arch/powerpc/kernel/setup_32.c | 4 +-
arch/powerpc/kernel/trace/ftrace.c | 24 +++---
arch/powerpc/lib/code-patching.c | 134 ++++++++++++++++++-------------
arch/powerpc/lib/feature-fixups.c | 5 +-
6 files changed, 119 insertions(+), 84 deletions(-)
Do you have any hints on how to debug and/or fix this issue?
Best regards,
Jonathan Neuschäfer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* linux-next: boot failure in today's linux-next
From: Stephen Rothwell @ 2021-04-26 6:36 UTC (permalink / raw)
To: Linux Next Mailing List
Cc: Jens Axboe, Andrew Morton, PowerPC, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 5197 bytes --]
Hi all,
Today's linux-next build (ipowerpc_pseries_le_defconfig)
failed its qemu boot tests like this:
[ 1.833361][ T1] ibmvscsi 71000003: SRP_VERSION: 16.a
[ 1.834439][ T1] ibmvscsi 71000003: Maximum ID: 64 Maximum LUN: 32 Maximum Channel: 3
[ 1.834683][ T1] scsi host0: IBM POWER Virtual SCSI Adapter 1.5.9
[ 1.842605][ C0] ibmvscsi 71000003: partner initialization complete
[ 1.844979][ C0] ibmvscsi 71000003: host srp version: 16.a, host partition qemu (0), OS 2, max io 2097152
[ 1.845502][ C0] ibmvscsi 71000003: sent SRP login
[ 1.845853][ C0] ibmvscsi 71000003: SRP_LOGIN succeeded
[ 1.851447][ T1] BUG: Kernel NULL pointer dereference on write at 0x00000390
[ 1.851577][ T1] Faulting instruction address: 0xc00000000070386c
[ 1.852171][ T1] Oops: Kernel access of bad area, sig: 11 [#1]
[ 1.852324][ T1] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
[ 1.852689][ T1] Modules linked in:
[ 1.853136][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0 #2
[ 1.853555][ T1] NIP: c00000000070386c LR: c000000000703a6c CTR: 0000000000000000
[ 1.853679][ T1] REGS: c0000000063a2f40 TRAP: 0380 Not tainted (5.12.0)
[ 1.853870][ T1] MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44002240 XER: 00000000
[ 1.854305][ T1] CFAR: c000000000703a68 IRQMASK: 0
[ 1.854305][ T1] GPR00: c000000000703a6c c0000000063a31e0 c00000000146b200 c0000000080ca800
[ 1.854305][ T1] GPR04: c000000006067380 c00c000000020180 0000000000000024 0000000000008500
[ 1.854305][ T1] GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 1.854305][ T1] GPR12: 0000000000002000 c000000001640000 c000000008068508 0000000000000020
[ 1.854305][ T1] GPR16: 0000000000000000 0000000000000024 c000000000f85f78 c000000000f0d998
[ 1.854305][ T1] GPR20: c0000000013b59e0 0000000000000003 c0000000063a340c 0000000000000001
[ 1.854305][ T1] GPR24: 0000000000000000 c0000000084a3000 c0000000080ca800 c00c000000020180
[ 1.854305][ T1] GPR28: 0000000000008500 c0000000080ca800 0000000000000024 c000000006067380
[ 1.855486][ T1] NIP [c00000000070386c] bio_add_hw_page+0x7c/0x240
[ 1.856357][ T1] LR [c000000000703a6c] bio_add_pc_page+0x3c/0x70
[ 1.856723][ T1] Call Trace:
[ 1.856890][ T1] [c0000000063a31e0] [0000000000000c00] 0xc00 (unreliable)
[ 1.857390][ T1] [c0000000063a3230] [c00000000070105c] bio_kmalloc+0x3c/0xd0
[ 1.857514][ T1] [c0000000063a3260] [c000000000713014] blk_rq_map_kern+0x164/0x4a0
[ 1.857630][ T1] [c0000000063a32d0] [c0000000008e17dc] __scsi_execute+0x1cc/0x270
[ 1.857746][ T1] [c0000000063a3350] [c0000000008e7bf0] scsi_probe_and_add_lun+0x250/0xd90
[ 1.857887][ T1] [c0000000063a34c0] [c0000000008e921c] __scsi_scan_target+0x17c/0x630
[ 1.858007][ T1] [c0000000063a35d0] [c0000000008e9900] scsi_scan_channel+0x90/0xe0
[ 1.858133][ T1] [c0000000063a3620] [c0000000008e9ba8] scsi_scan_host_selected+0x138/0x1a0
[ 1.858258][ T1] [c0000000063a3670] [c0000000008e9fec] scsi_scan_host+0x2dc/0x320
[ 1.858367][ T1] [c0000000063a3710] [c00000000091b2a0] ibmvscsi_probe+0xa70/0xa80
[ 1.858487][ T1] [c0000000063a3800] [c0000000000eb8ac] vio_bus_probe+0x9c/0x460
[ 1.858616][ T1] [c0000000063a38a0] [c0000000008979bc] really_probe+0x12c/0x6b0
[ 1.858749][ T1] [c0000000063a3950] [c000000000897fd4] driver_probe_device+0x94/0x130
[ 1.858874][ T1] [c0000000063a3980] [c00000000089896c] device_driver_attach+0x11c/0x130
[ 1.858999][ T1] [c0000000063a39c0] [c000000000898a38] __driver_attach+0xb8/0x1a0
[ 1.859123][ T1] [c0000000063a3a10] [c0000000008941a8] bus_for_each_dev+0xa8/0x130
[ 1.859257][ T1] [c0000000063a3a70] [c000000000896ef4] driver_attach+0x34/0x50
[ 1.859381][ T1] [c0000000063a3a90] [c000000000896510] bus_add_driver+0x170/0x2b0
[ 1.859503][ T1] [c0000000063a3b20] [c000000000899b04] driver_register+0xb4/0x1c0
[ 1.859626][ T1] [c0000000063a3b90] [c0000000000ea808] __vio_register_driver+0x68/0x90
[ 1.859754][ T1] [c0000000063a3bb0] [c0000000010cee74] ibmvscsi_module_init+0xa4/0xdc
[ 1.859931][ T1] [c0000000063a3bf0] [c000000000012190] do_one_initcall+0x60/0x2c0
[ 1.860071][ T1] [c0000000063a3cc0] [c0000000010846e4] kernel_init_freeable+0x300/0x3a0
[ 1.860207][ T1] [c0000000063a3da0] [c000000000012764] kernel_init+0x2c/0x168
[ 1.860336][ T1] [c0000000063a3e10] [c00000000000d5ec] ret_from_kernel_thread+0x5c/0x70
[ 1.860690][ T1] Instruction dump:
[ 1.861072][ T1] fba10038 7cbb2b78 7c7d1b78 7cfc3b78 a1440048 2c2a0000 4082008c a13f004a
[ 1.861328][ T1] 7c095040 40810110 e93f0008 811f0028 <e9290390> e9290050 812903d8 7d3e4850
[ 1.863000][ T1] ---[ end trace c49ca2d91ee47d7f ]---
[ 1.879456][ T1]
[ 2.880941][ T1] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
I don't know what caused this, but it is some change since Friday.
I have left it like this.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: linux-next: boot failure in today's linux-next
From: Stephen Rothwell @ 2021-04-26 7:43 UTC (permalink / raw)
To: Jens Axboe
Cc: Linux Kernel Mailing List, Linus, Linux Next Mailing List,
Changheun Lee, Andrew Morton, PowerPC
In-Reply-To: <20210426163606.36a97253@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 5602 bytes --]
Hi all,
On Mon, 26 Apr 2021 16:36:06 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next build (ipowerpc_pseries_le_defconfig)
> failed its qemu boot tests like this:
>
> [ 1.833361][ T1] ibmvscsi 71000003: SRP_VERSION: 16.a
> [ 1.834439][ T1] ibmvscsi 71000003: Maximum ID: 64 Maximum LUN: 32 Maximum Channel: 3
> [ 1.834683][ T1] scsi host0: IBM POWER Virtual SCSI Adapter 1.5.9
> [ 1.842605][ C0] ibmvscsi 71000003: partner initialization complete
> [ 1.844979][ C0] ibmvscsi 71000003: host srp version: 16.a, host partition qemu (0), OS 2, max io 2097152
> [ 1.845502][ C0] ibmvscsi 71000003: sent SRP login
> [ 1.845853][ C0] ibmvscsi 71000003: SRP_LOGIN succeeded
> [ 1.851447][ T1] BUG: Kernel NULL pointer dereference on write at 0x00000390
> [ 1.851577][ T1] Faulting instruction address: 0xc00000000070386c
> [ 1.852171][ T1] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 1.852324][ T1] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> [ 1.852689][ T1] Modules linked in:
> [ 1.853136][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0 #2
> [ 1.853555][ T1] NIP: c00000000070386c LR: c000000000703a6c CTR: 0000000000000000
> [ 1.853679][ T1] REGS: c0000000063a2f40 TRAP: 0380 Not tainted (5.12.0)
> [ 1.853870][ T1] MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44002240 XER: 00000000
> [ 1.854305][ T1] CFAR: c000000000703a68 IRQMASK: 0
> [ 1.854305][ T1] GPR00: c000000000703a6c c0000000063a31e0 c00000000146b200 c0000000080ca800
> [ 1.854305][ T1] GPR04: c000000006067380 c00c000000020180 0000000000000024 0000000000008500
> [ 1.854305][ T1] GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 1.854305][ T1] GPR12: 0000000000002000 c000000001640000 c000000008068508 0000000000000020
> [ 1.854305][ T1] GPR16: 0000000000000000 0000000000000024 c000000000f85f78 c000000000f0d998
> [ 1.854305][ T1] GPR20: c0000000013b59e0 0000000000000003 c0000000063a340c 0000000000000001
> [ 1.854305][ T1] GPR24: 0000000000000000 c0000000084a3000 c0000000080ca800 c00c000000020180
> [ 1.854305][ T1] GPR28: 0000000000008500 c0000000080ca800 0000000000000024 c000000006067380
> [ 1.855486][ T1] NIP [c00000000070386c] bio_add_hw_page+0x7c/0x240
> [ 1.856357][ T1] LR [c000000000703a6c] bio_add_pc_page+0x3c/0x70
> [ 1.856723][ T1] Call Trace:
> [ 1.856890][ T1] [c0000000063a31e0] [0000000000000c00] 0xc00 (unreliable)
> [ 1.857390][ T1] [c0000000063a3230] [c00000000070105c] bio_kmalloc+0x3c/0xd0
> [ 1.857514][ T1] [c0000000063a3260] [c000000000713014] blk_rq_map_kern+0x164/0x4a0
> [ 1.857630][ T1] [c0000000063a32d0] [c0000000008e17dc] __scsi_execute+0x1cc/0x270
> [ 1.857746][ T1] [c0000000063a3350] [c0000000008e7bf0] scsi_probe_and_add_lun+0x250/0xd90
> [ 1.857887][ T1] [c0000000063a34c0] [c0000000008e921c] __scsi_scan_target+0x17c/0x630
> [ 1.858007][ T1] [c0000000063a35d0] [c0000000008e9900] scsi_scan_channel+0x90/0xe0
> [ 1.858133][ T1] [c0000000063a3620] [c0000000008e9ba8] scsi_scan_host_selected+0x138/0x1a0
> [ 1.858258][ T1] [c0000000063a3670] [c0000000008e9fec] scsi_scan_host+0x2dc/0x320
> [ 1.858367][ T1] [c0000000063a3710] [c00000000091b2a0] ibmvscsi_probe+0xa70/0xa80
> [ 1.858487][ T1] [c0000000063a3800] [c0000000000eb8ac] vio_bus_probe+0x9c/0x460
> [ 1.858616][ T1] [c0000000063a38a0] [c0000000008979bc] really_probe+0x12c/0x6b0
> [ 1.858749][ T1] [c0000000063a3950] [c000000000897fd4] driver_probe_device+0x94/0x130
> [ 1.858874][ T1] [c0000000063a3980] [c00000000089896c] device_driver_attach+0x11c/0x130
> [ 1.858999][ T1] [c0000000063a39c0] [c000000000898a38] __driver_attach+0xb8/0x1a0
> [ 1.859123][ T1] [c0000000063a3a10] [c0000000008941a8] bus_for_each_dev+0xa8/0x130
> [ 1.859257][ T1] [c0000000063a3a70] [c000000000896ef4] driver_attach+0x34/0x50
> [ 1.859381][ T1] [c0000000063a3a90] [c000000000896510] bus_add_driver+0x170/0x2b0
> [ 1.859503][ T1] [c0000000063a3b20] [c000000000899b04] driver_register+0xb4/0x1c0
> [ 1.859626][ T1] [c0000000063a3b90] [c0000000000ea808] __vio_register_driver+0x68/0x90
> [ 1.859754][ T1] [c0000000063a3bb0] [c0000000010cee74] ibmvscsi_module_init+0xa4/0xdc
> [ 1.859931][ T1] [c0000000063a3bf0] [c000000000012190] do_one_initcall+0x60/0x2c0
> [ 1.860071][ T1] [c0000000063a3cc0] [c0000000010846e4] kernel_init_freeable+0x300/0x3a0
> [ 1.860207][ T1] [c0000000063a3da0] [c000000000012764] kernel_init+0x2c/0x168
> [ 1.860336][ T1] [c0000000063a3e10] [c00000000000d5ec] ret_from_kernel_thread+0x5c/0x70
> [ 1.860690][ T1] Instruction dump:
> [ 1.861072][ T1] fba10038 7cbb2b78 7c7d1b78 7cfc3b78 a1440048 2c2a0000 4082008c a13f004a
> [ 1.861328][ T1] 7c095040 40810110 e93f0008 811f0028 <e9290390> e9290050 812903d8 7d3e4850
> [ 1.863000][ T1] ---[ end trace c49ca2d91ee47d7f ]---
> [ 1.879456][ T1]
> [ 2.880941][ T1] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> I don't know what caused this, but it is some change since Friday.
>
> I have left it like this.
Bisections leads to commit
42fb54fbc707 ("bio: limit bio max size")
from the block tree. Reverting that commit on top of today's
linux-next allows to the boot to work again.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] ASoC: fsl_spdif: add support for enabling raw capture mode
From: Shengjiu Wang @ 2021-04-26 8:24 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
From: Viorel Suman <viorel.suman@nxp.com>
Since i.MX8MM SPDIF interface is able to capture raw data.
Add support in SPDIF driver for this functionality.
Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_spdif.c | 67 +++++++++++++++++++++++++++++++++++++++
sound/soc/fsl/fsl_spdif.h | 1 +
2 files changed, 68 insertions(+)
diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index c631de325a6e..2a76714eb8e6 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -49,6 +49,7 @@ static u8 srpc_dpll_locked[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0xa, 0xb };
* @imx: for imx platform
* @shared_root_clock: flag of sharing a clock source with others;
* so the driver shouldn't set root clock rate
+ * @raw_capture_mode: if raw capture mode support
* @interrupts: interrupt number
* @tx_burst: tx maxburst size
* @rx_burst: rx maxburst size
@@ -57,6 +58,7 @@ static u8 srpc_dpll_locked[] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0xa, 0xb };
struct fsl_spdif_soc_data {
bool imx;
bool shared_root_clock;
+ bool raw_capture_mode;
u32 interrupts;
u32 tx_burst;
u32 rx_burst;
@@ -136,6 +138,7 @@ struct fsl_spdif_priv {
static struct fsl_spdif_soc_data fsl_spdif_vf610 = {
.imx = false,
.shared_root_clock = false,
+ .raw_capture_mode = false,
.interrupts = 1,
.tx_burst = FSL_SPDIF_TXFIFO_WML,
.rx_burst = FSL_SPDIF_RXFIFO_WML,
@@ -145,6 +148,7 @@ static struct fsl_spdif_soc_data fsl_spdif_vf610 = {
static struct fsl_spdif_soc_data fsl_spdif_imx35 = {
.imx = true,
.shared_root_clock = false,
+ .raw_capture_mode = false,
.interrupts = 1,
.tx_burst = FSL_SPDIF_TXFIFO_WML,
.rx_burst = FSL_SPDIF_RXFIFO_WML,
@@ -154,6 +158,7 @@ static struct fsl_spdif_soc_data fsl_spdif_imx35 = {
static struct fsl_spdif_soc_data fsl_spdif_imx6sx = {
.imx = true,
.shared_root_clock = true,
+ .raw_capture_mode = false,
.interrupts = 1,
.tx_burst = FSL_SPDIF_TXFIFO_WML,
.rx_burst = FSL_SPDIF_RXFIFO_WML,
@@ -164,12 +169,23 @@ static struct fsl_spdif_soc_data fsl_spdif_imx6sx = {
static struct fsl_spdif_soc_data fsl_spdif_imx8qm = {
.imx = true,
.shared_root_clock = true,
+ .raw_capture_mode = false,
.interrupts = 2,
.tx_burst = 2, /* Applied for EDMA */
.rx_burst = 2, /* Applied for EDMA */
.tx_formats = SNDRV_PCM_FMTBIT_S24_LE, /* Applied for EDMA */
};
+static struct fsl_spdif_soc_data fsl_spdif_imx8mm = {
+ .imx = true,
+ .shared_root_clock = false,
+ .raw_capture_mode = true,
+ .interrupts = 1,
+ .tx_burst = FSL_SPDIF_TXFIFO_WML,
+ .rx_burst = FSL_SPDIF_RXFIFO_WML,
+ .tx_formats = FSL_SPDIF_FORMATS_PLAYBACK,
+};
+
/* Check if clk is a root clock that does not share clock source with others */
static inline bool fsl_spdif_can_set_clk_rate(struct fsl_spdif_priv *spdif, int clk)
{
@@ -846,6 +862,39 @@ static int fsl_spdif_tx_vbit_put(struct snd_kcontrol *kcontrol,
return 0;
}
+static int fsl_spdif_rx_rcm_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_dai *cpu_dai = snd_kcontrol_chip(kcontrol);
+ struct fsl_spdif_priv *spdif_priv = snd_soc_dai_get_drvdata(cpu_dai);
+ struct regmap *regmap = spdif_priv->regmap;
+ u32 val;
+
+ regmap_read(regmap, REG_SPDIF_SCR, &val);
+ val = (val & SCR_RAW_CAPTURE_MODE) ? 1 : 0;
+ ucontrol->value.integer.value[0] = val;
+
+ return 0;
+}
+
+static int fsl_spdif_rx_rcm_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_dai *cpu_dai = snd_kcontrol_chip(kcontrol);
+ struct fsl_spdif_priv *spdif_priv = snd_soc_dai_get_drvdata(cpu_dai);
+ struct regmap *regmap = spdif_priv->regmap;
+ u32 val = (ucontrol->value.integer.value[0] ? SCR_RAW_CAPTURE_MODE : 0);
+
+ if (val)
+ cpu_dai->driver->capture.formats |= SNDRV_PCM_FMTBIT_S32_LE;
+ else
+ cpu_dai->driver->capture.formats &= ~SNDRV_PCM_FMTBIT_S32_LE;
+
+ regmap_update_bits(regmap, REG_SPDIF_SCR, SCR_RAW_CAPTURE_MODE, val);
+
+ return 0;
+}
+
/* DPLL lock information */
static int fsl_spdif_rxrate_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)
@@ -1029,6 +1078,19 @@ static struct snd_kcontrol_new fsl_spdif_ctrls[] = {
},
};
+static struct snd_kcontrol_new fsl_spdif_ctrls_rcm[] = {
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+ .name = "IEC958 Raw Capture Mode",
+ .access = SNDRV_CTL_ELEM_ACCESS_READ |
+ SNDRV_CTL_ELEM_ACCESS_WRITE |
+ SNDRV_CTL_ELEM_ACCESS_VOLATILE,
+ .info = snd_ctl_boolean_mono_info,
+ .get = fsl_spdif_rx_rcm_get,
+ .put = fsl_spdif_rx_rcm_put,
+ },
+};
+
static int fsl_spdif_dai_probe(struct snd_soc_dai *dai)
{
struct fsl_spdif_priv *spdif_private = snd_soc_dai_get_drvdata(dai);
@@ -1038,6 +1100,10 @@ static int fsl_spdif_dai_probe(struct snd_soc_dai *dai)
snd_soc_add_dai_controls(dai, fsl_spdif_ctrls, ARRAY_SIZE(fsl_spdif_ctrls));
+ if (spdif_private->soc->raw_capture_mode)
+ snd_soc_add_dai_controls(dai, fsl_spdif_ctrls_rcm,
+ ARRAY_SIZE(fsl_spdif_ctrls_rcm));
+
/*Clear the val bit for Tx*/
regmap_update_bits(spdif_private->regmap, REG_SPDIF_SCR,
SCR_VAL_MASK, SCR_VAL_CLEAR);
@@ -1476,6 +1542,7 @@ static const struct of_device_id fsl_spdif_dt_ids[] = {
{ .compatible = "fsl,vf610-spdif", .data = &fsl_spdif_vf610, },
{ .compatible = "fsl,imx6sx-spdif", .data = &fsl_spdif_imx6sx, },
{ .compatible = "fsl,imx8qm-spdif", .data = &fsl_spdif_imx8qm, },
+ { .compatible = "fsl,imx8mm-spdif", .data = &fsl_spdif_imx8mm, },
{}
};
MODULE_DEVICE_TABLE(of, fsl_spdif_dt_ids);
diff --git a/sound/soc/fsl/fsl_spdif.h b/sound/soc/fsl/fsl_spdif.h
index d5f1dfd58740..bff8290e71f2 100644
--- a/sound/soc/fsl/fsl_spdif.h
+++ b/sound/soc/fsl/fsl_spdif.h
@@ -63,6 +63,7 @@
#define SCR_TXFIFO_FSEL_IF4 (0x1 << SCR_TXFIFO_FSEL_OFFSET)
#define SCR_TXFIFO_FSEL_IF8 (0x2 << SCR_TXFIFO_FSEL_OFFSET)
#define SCR_TXFIFO_FSEL_IF12 (0x3 << SCR_TXFIFO_FSEL_OFFSET)
+#define SCR_RAW_CAPTURE_MODE BIT(14)
#define SCR_LOW_POWER (1 << 13)
#define SCR_SOFT_RESET (1 << 12)
#define SCR_TXFIFO_CTRL_OFFSET 10
--
2.27.0
^ permalink raw reply related
* Guest entry/exit performance work and observations
From: Nicholas Piggin @ 2021-04-26 11:27 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev
I'm looking at KVM HV P9 path guest exit/entry performance with the Cify
patches, plus some further work to see what we can do.
Measurement is done in the guest making a "NULL hcall" and return back
to a non-nested guest. Two cases considered: First, returning to guest
at the "try_real_mode" hcall handler. Second, returning back to guest
after going around a loop in kvmppc_vcpu_run_hv (i.e., exit into full
host kernel context, but not to host usermode).
The real-mode test is a proxy for real mode hcall and other interrupt
handlers, and the full exit is a proxy for virtual mode hcalls and
interrupt handlers.
The test was done with powernv_defconfig, radix guest and radix host on
a POWER9 with meltdown mitigations disabled. A minor hack was made
just to get the immediate return / NULL hcall behaviour to measure
performance.
* Upstream try_real_mode return - 509 cycles
* Upstream virt NULL hcall - 9587 cycles
* KVM Cify virt NULL hcall - 9333 cycles
* KVM Cify+opt virt NULL hcall - 5754 cycles (167% faster than upstream,
or 60% the cycles required)
The KVM Cify series (which you have already seen) plus the further
optimisations patch series is here:
https://github.com/npiggin/linux/tree/kvm-in-c-new
Some of the important / major further optimisation patches have
individual cycle time improvement contribution annotated. In many cases
things are inter-dependent, e.g., patch A might improve 100 cycles and
B 50 cycles but A+B might be 250 due to together avoiding an SPR stall.
So take the individual numbers with a grain of salt, and the cumulative
result above is most important.
In summary the Cify series does not hurt performance of entry/exit,
which is good. It actually helps a bit, I'm not sure exactly where.
And we can make quite a lot more improvement with this series.
HOWEVER! The Cify series removes the very fast real mode hcall and
interrupt handlers (except some things like machine check). So any real
mode handler will be handled as a virt mode handler on P9 after Cify.
Now I have some further patches in progress that should shave about 1000
more cycles more from the full exit, but beyond that it gets pretty
tough to improve. That still leaves it an order of magnitude slower.
Now I did say this doesn't matter so much with a P9/radix/xive guest
which is true, except possibly for TCE hcalls that Alexey brought to my
attention (any other important cases?). So we will have to think about
that.
Alexey did say that the real mode TCE hcalls were added for P8, and
were less important for P9, but it is something to keep an eye on. We
might end up adding a faster handler back, but I would much prefer if
wasn't entirely run in guest context as they do today (maybe switch
MMU context, TB, and a few other important SPRs, and enable translation
so it can run practically as host kernel context). But I think we should
wait and see, and add the complexity only if it comes up as a problem.
The other thing is the P9 path now implements the P9 hash guest support
after the Cify series. Hash does a lot more exits due to translation
hcalls and interrupts. I did do some basic measurements (e.g., kernel
compile) and couldn't see a significant slowdown. But in any case I
think the P9 hash code is not important to micro optimise, it was only
done to simplify code and remove asm, so I would rather not add
complexity for that.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] kbuild: replace LANG=C with LC_ALL=C
From: Matthias Maennich @ 2021-04-26 10:17 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linuxppc-dev, linux-kbuild, Mat Martineau, linux-kernel, mptcp,
Paul Mackerras, linux-kselftest, Jakub Kicinski, Matthieu Baerts,
netdev, Shuah Khan, David S. Miller
In-Reply-To: <20210424114841.394239-1-masahiroy@kernel.org>
On Sat, Apr 24, 2021 at 08:48:41PM +0900, Masahiro Yamada wrote:
>LANG gives a weak default to each LC_* in case it is not explicitly
>defined. LC_ALL, if set, overrides all other LC_* variables.
>
> LANG < LC_CTYPE, LC_COLLATE, LC_MONETARY, LC_NUMERIC, ... < LC_ALL
>
>This is why documentation such as [1] suggests to set LC_ALL in build
>scripts to get the deterministic result.
>
>LANG=C is not strong enough to override LC_* that may be set by end
>users.
>
>[1]: https://reproducible-builds.org/docs/locales/
>
>Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Matthias Maennich <maennich@google.com>
Cheers,
Matthias
>---
>
> arch/powerpc/boot/wrapper | 2 +-
> scripts/nsdeps | 2 +-
> scripts/recordmcount.pl | 2 +-
> scripts/setlocalversion | 2 +-
> scripts/tags.sh | 2 +-
> tools/testing/selftests/net/mptcp/mptcp_connect.sh | 2 +-
> usr/gen_initramfs.sh | 2 +-
> 7 files changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
>index 41fa0a8715e3..cdb796b76e2e 100755
>--- a/arch/powerpc/boot/wrapper
>+++ b/arch/powerpc/boot/wrapper
>@@ -191,7 +191,7 @@ if [ -z "$kernel" ]; then
> kernel=vmlinux
> fi
>
>-LANG=C elfformat="`${CROSS}objdump -p "$kernel" | grep 'file format' | awk '{print $4}'`"
>+LC_ALL=C elfformat="`${CROSS}objdump -p "$kernel" | grep 'file format' | awk '{print $4}'`"
> case "$elfformat" in
> elf64-powerpcle) format=elf64lppc ;;
> elf64-powerpc) format=elf32ppc ;;
>diff --git a/scripts/nsdeps b/scripts/nsdeps
>index e8ce2a4d704a..04c4b96e95ec 100644
>--- a/scripts/nsdeps
>+++ b/scripts/nsdeps
>@@ -44,7 +44,7 @@ generate_deps() {
> for source_file in $mod_source_files; do
> sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
> offset=$(wc -l ${source_file}.tmp | awk '{print $1;}')
>- cat $source_file | grep MODULE_IMPORT_NS | LANG=C sort -u >> ${source_file}.tmp
>+ cat $source_file | grep MODULE_IMPORT_NS | LC_ALL=C sort -u >> ${source_file}.tmp
> tail -n +$((offset +1)) ${source_file} | grep -v MODULE_IMPORT_NS >> ${source_file}.tmp
> if ! diff -q ${source_file} ${source_file}.tmp; then
> mv ${source_file}.tmp ${source_file}
>diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
>index 867860ea57da..0a7fc9507d6f 100755
>--- a/scripts/recordmcount.pl
>+++ b/scripts/recordmcount.pl
>@@ -497,7 +497,7 @@ sub update_funcs
> #
> # Step 2: find the sections and mcount call sites
> #
>-open(IN, "LANG=C $objdump -hdr $inputfile|") || die "error running $objdump";
>+open(IN, "LC_ALL=C $objdump -hdr $inputfile|") || die "error running $objdump";
>
> my $text;
>
>diff --git a/scripts/setlocalversion b/scripts/setlocalversion
>index bb709eda96cd..db941f6d9591 100755
>--- a/scripts/setlocalversion
>+++ b/scripts/setlocalversion
>@@ -126,7 +126,7 @@ scm_version()
> fi
>
> # Check for svn and a svn repo.
>- if rev=$(LANG= LC_ALL= LC_MESSAGES=C svn info 2>/dev/null | grep '^Last Changed Rev'); then
>+ if rev=$(LC_ALL=C svn info 2>/dev/null | grep '^Last Changed Rev'); then
> rev=$(echo $rev | awk '{print $NF}')
> printf -- '-svn%s' "$rev"
>
>diff --git a/scripts/tags.sh b/scripts/tags.sh
>index fd96734deff1..db8ba411860a 100755
>--- a/scripts/tags.sh
>+++ b/scripts/tags.sh
>@@ -326,5 +326,5 @@ esac
>
> # Remove structure forward declarations.
> if [ -n "$remove_structs" ]; then
>- LANG=C sed -i -e '/^\([a-zA-Z_][a-zA-Z0-9_]*\)\t.*\t\/\^struct \1;.*\$\/;"\tx$/d' $1
>+ LC_ALL=C sed -i -e '/^\([a-zA-Z_][a-zA-Z0-9_]*\)\t.*\t\/\^struct \1;.*\$\/;"\tx$/d' $1
> fi
>diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>index 10a030b53b23..1d2a6e7b877c 100755
>--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>@@ -273,7 +273,7 @@ check_mptcp_disabled()
> ip netns exec ${disabled_ns} sysctl -q net.mptcp.enabled=0
>
> local err=0
>- LANG=C ip netns exec ${disabled_ns} ./mptcp_connect -t $timeout -p 10000 -s MPTCP 127.0.0.1 < "$cin" 2>&1 | \
>+ LC_ALL=C ip netns exec ${disabled_ns} ./mptcp_connect -t $timeout -p 10000 -s MPTCP 127.0.0.1 < "$cin" 2>&1 | \
> grep -q "^socket: Protocol not available$" && err=1
> ip netns delete ${disabled_ns}
>
>diff --git a/usr/gen_initramfs.sh b/usr/gen_initramfs.sh
>index 8ae831657e5d..63476bb70b41 100755
>--- a/usr/gen_initramfs.sh
>+++ b/usr/gen_initramfs.sh
>@@ -147,7 +147,7 @@ dir_filelist() {
> header "$1"
>
> srcdir=$(echo "$1" | sed -e 's://*:/:g')
>- dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n" | LANG=C sort)
>+ dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n" | LC_ALL=C sort)
>
> # If $dirlist is only one line, then the directory is empty
> if [ "$(echo "${dirlist}" | wc -l)" -gt 1 ]; then
>--
>2.27.0
>
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] of: net: fix of_get_mac_addr_nvmem() for non-platform devices
From: Michael Walle @ 2021-04-26 10:54 UTC (permalink / raw)
To: Rob Herring
Cc: Andrew Lunn, Paul Mackerras, Rafał Miłecki,
Nobuhiro Iwamatsu, moderated list:ARM/STM32 ARCHITECTURE,
Jerome Brunet, Neil Armstrong, Michal Simek, Jose Abreu,
NXP Linux Team, Mark Lee, Hauke Mehrtens, Sascha Hauer,
Lorenzo Bianconi, linux-omap, Greg Kroah-Hartman, linux-wireless,
linux-kernel, Pengutronix Kernel Team, Vladimir Oltean,
Claudiu Beznea, Jérôme Pouiller, Kunihiko Hayashi,
Chris Snook, Frank Rowand, Gregory Clement, Madalin Bucur,
Martin Blumenstingl, Murali Karicheri, Yisen Zhuang,
Alexandre Torgue, Wingman Kwok, Sean Wang, Maxime Ripard,
Claudiu Manoil, open list:ARM/Amlogic Meson..., Kalle Valo,
Mirko Lindner, Fugang Duan, Bryan Whitehead,
QCA ath9k Development, Microchip Linux Driver Support,
Taras Chornyi, Maxime Coquelin, Kevin Hilman, Heiner Kallweit,
Andreas Larsson, Giuseppe Cavallaro, Fabio Estevam,
Stanislaw Gruszka, Florian Fainelli, linux-staging, Chen-Yu Tsai,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, linux-arm-kernel,
Grygorii Strashko, Byungho An, Radhey Shyam Pandey,
Vladimir Zapolskiy, John Crispin, Salil Mehta, Sergei Shtylyov,
linux-oxnas, Shawn Guo, David S . Miller, Helmut Schaa,
Thomas Petazzoni, open list:MEDIA DRIVERS FOR RENESAS - FCP,
Ryder Lee, Russell King, Vadym Kochan, Jakub Kicinski,
Vivien Didelot, Sunil Goutham, Sebastian Hesselbarth, devicetree,
moderated list:ARM/Mediatek SoC support, Matthias Brugger,
Jernej Skrabec, netdev, Nicolas Ferre, Li Yang, Stephen Hemminger,
Vinod Koul, Joyce Ooi, linuxppc-dev, Felix Fietkau
In-Reply-To: <CAL_JsqLrx6nFZrKiEtm2a1vDvQGG+FkpGtJCG2osM8hhGo3P=Q@mail.gmail.com>
Am 2021-04-16 17:19, schrieb Rob Herring:
> On Fri, Apr 16, 2021 at 2:30 AM Michael Walle <michael@walle.cc> wrote:
>>
>> Am 2021-04-16 05:24, schrieb Benjamin Herrenschmidt:
>> > On Mon, 2021-04-12 at 19:47 +0200, Michael Walle wrote:
>> >>
>> >> /**
>> >> * of_get_phy_mode - Get phy mode for given device_node
>> >> @@ -59,15 +60,39 @@ static int of_get_mac_addr(struct device_node *np,
>> >> const char *name, u8 *addr)
>> >> static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
>> >> {
>> >> struct platform_device *pdev = of_find_device_by_node(np);
>> >> + struct nvmem_cell *cell;
>> >> + const void *mac;
>> >> + size_t len;
>> >> int ret;
>> >>
>> >> - if (!pdev)
>> >> - return -ENODEV;
>> >> + /* Try lookup by device first, there might be a
>> >> nvmem_cell_lookup
>> >> + * associated with a given device.
>> >> + */
>> >> + if (pdev) {
>> >> + ret = nvmem_get_mac_address(&pdev->dev, addr);
>> >> + put_device(&pdev->dev);
>> >> + return ret;
>> >> + }
>> >> +
>> >
>> > This smells like the wrong band aid :)
>> >
>> > Any struct device can contain an OF node pointer these days.
>>
>> But not all nodes might have an associated device, see DSA for
>> example.
>
> I believe what Ben is saying and what I said earlier is going from dev
> -> OF node is right and OF node -> dev is wrong. If you only have an
> OF node, then use an of_* function.
>
>> And as the name suggests of_get_mac_address() operates on a node. So
>> if a driver calls of_get_mac_address() it should work on the node.
>> What
>> is wrong IMHO, is that the ethernet drivers where the corresponding
>> board
>> has a nvmem_cell_lookup registered is calling
>> of_get_mac_address(node).
>> It should rather call eth_get_mac_address(dev) in the first place.
>>
>> One would need to figure out if there is an actual device (with an
>> assiciated of_node), then call eth_get_mac_address(dev) and if there
>> isn't a device call of_get_mac_address(node).
>
> Yes, I think we're all in agreement.
>
>> But I don't know if that is easy to figure out. Well, one could start
>> with just the device where nvmem_cell_lookup is used. Then we could
>> drop the workaround above.
>
> Start with the ones just passing dev.of_node directly:
>
> $ git grep 'of_get_mac_address(.*of_node)'
[..]
Before I'll try to come up with a patch for this, I'd like to get
your opinion on it.
(1) replacing of_get_mac_address(node) with eth_get_mac_address(dev)
might sometimes lead to confusing comments like in
drivers/net/ethernet/allwinner/sun4i-emac.c:
/* Read MAC-address from DT */
ret = of_get_mac_address(np, ndev->dev_addr);
Do we live with that or should the new name somehow reflect that
it is taken from the device tree.
(2) What do you think of eth_get_mac_address(ndev). That is, the
second argument is missing and ndev->dev_addr is used.
I'm unsure about it. We'd still need a second function for drivers
which don't write ndev->dev_addr directly, but have some custom
logic in between. OTOH it would be like eth_hw_addr_random(ndev).
-michael
^ permalink raw reply
* Re: linux-next: boot failure in today's linux-next
From: Jens Axboe @ 2021-04-26 12:46 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Linux Kernel Mailing List, Linus, Linux Next Mailing List,
Changheun Lee, Andrew Morton, PowerPC
In-Reply-To: <20210426174310.6f03345b@canb.auug.org.au>
On 4/26/21 1:43 AM, Stephen Rothwell wrote:
> Hi all,
>
> On Mon, 26 Apr 2021 16:36:06 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> Today's linux-next build (ipowerpc_pseries_le_defconfig)
>> failed its qemu boot tests like this:
>>
>> [ 1.833361][ T1] ibmvscsi 71000003: SRP_VERSION: 16.a
>> [ 1.834439][ T1] ibmvscsi 71000003: Maximum ID: 64 Maximum LUN: 32 Maximum Channel: 3
>> [ 1.834683][ T1] scsi host0: IBM POWER Virtual SCSI Adapter 1.5.9
>> [ 1.842605][ C0] ibmvscsi 71000003: partner initialization complete
>> [ 1.844979][ C0] ibmvscsi 71000003: host srp version: 16.a, host partition qemu (0), OS 2, max io 2097152
>> [ 1.845502][ C0] ibmvscsi 71000003: sent SRP login
>> [ 1.845853][ C0] ibmvscsi 71000003: SRP_LOGIN succeeded
>> [ 1.851447][ T1] BUG: Kernel NULL pointer dereference on write at 0x00000390
>> [ 1.851577][ T1] Faulting instruction address: 0xc00000000070386c
>> [ 1.852171][ T1] Oops: Kernel access of bad area, sig: 11 [#1]
>> [ 1.852324][ T1] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
>> [ 1.852689][ T1] Modules linked in:
>> [ 1.853136][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0 #2
>> [ 1.853555][ T1] NIP: c00000000070386c LR: c000000000703a6c CTR: 0000000000000000
>> [ 1.853679][ T1] REGS: c0000000063a2f40 TRAP: 0380 Not tainted (5.12.0)
>> [ 1.853870][ T1] MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44002240 XER: 00000000
>> [ 1.854305][ T1] CFAR: c000000000703a68 IRQMASK: 0
>> [ 1.854305][ T1] GPR00: c000000000703a6c c0000000063a31e0 c00000000146b200 c0000000080ca800
>> [ 1.854305][ T1] GPR04: c000000006067380 c00c000000020180 0000000000000024 0000000000008500
>> [ 1.854305][ T1] GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> [ 1.854305][ T1] GPR12: 0000000000002000 c000000001640000 c000000008068508 0000000000000020
>> [ 1.854305][ T1] GPR16: 0000000000000000 0000000000000024 c000000000f85f78 c000000000f0d998
>> [ 1.854305][ T1] GPR20: c0000000013b59e0 0000000000000003 c0000000063a340c 0000000000000001
>> [ 1.854305][ T1] GPR24: 0000000000000000 c0000000084a3000 c0000000080ca800 c00c000000020180
>> [ 1.854305][ T1] GPR28: 0000000000008500 c0000000080ca800 0000000000000024 c000000006067380
>> [ 1.855486][ T1] NIP [c00000000070386c] bio_add_hw_page+0x7c/0x240
>> [ 1.856357][ T1] LR [c000000000703a6c] bio_add_pc_page+0x3c/0x70
>> [ 1.856723][ T1] Call Trace:
>> [ 1.856890][ T1] [c0000000063a31e0] [0000000000000c00] 0xc00 (unreliable)
>> [ 1.857390][ T1] [c0000000063a3230] [c00000000070105c] bio_kmalloc+0x3c/0xd0
>> [ 1.857514][ T1] [c0000000063a3260] [c000000000713014] blk_rq_map_kern+0x164/0x4a0
>> [ 1.857630][ T1] [c0000000063a32d0] [c0000000008e17dc] __scsi_execute+0x1cc/0x270
>> [ 1.857746][ T1] [c0000000063a3350] [c0000000008e7bf0] scsi_probe_and_add_lun+0x250/0xd90
>> [ 1.857887][ T1] [c0000000063a34c0] [c0000000008e921c] __scsi_scan_target+0x17c/0x630
>> [ 1.858007][ T1] [c0000000063a35d0] [c0000000008e9900] scsi_scan_channel+0x90/0xe0
>> [ 1.858133][ T1] [c0000000063a3620] [c0000000008e9ba8] scsi_scan_host_selected+0x138/0x1a0
>> [ 1.858258][ T1] [c0000000063a3670] [c0000000008e9fec] scsi_scan_host+0x2dc/0x320
>> [ 1.858367][ T1] [c0000000063a3710] [c00000000091b2a0] ibmvscsi_probe+0xa70/0xa80
>> [ 1.858487][ T1] [c0000000063a3800] [c0000000000eb8ac] vio_bus_probe+0x9c/0x460
>> [ 1.858616][ T1] [c0000000063a38a0] [c0000000008979bc] really_probe+0x12c/0x6b0
>> [ 1.858749][ T1] [c0000000063a3950] [c000000000897fd4] driver_probe_device+0x94/0x130
>> [ 1.858874][ T1] [c0000000063a3980] [c00000000089896c] device_driver_attach+0x11c/0x130
>> [ 1.858999][ T1] [c0000000063a39c0] [c000000000898a38] __driver_attach+0xb8/0x1a0
>> [ 1.859123][ T1] [c0000000063a3a10] [c0000000008941a8] bus_for_each_dev+0xa8/0x130
>> [ 1.859257][ T1] [c0000000063a3a70] [c000000000896ef4] driver_attach+0x34/0x50
>> [ 1.859381][ T1] [c0000000063a3a90] [c000000000896510] bus_add_driver+0x170/0x2b0
>> [ 1.859503][ T1] [c0000000063a3b20] [c000000000899b04] driver_register+0xb4/0x1c0
>> [ 1.859626][ T1] [c0000000063a3b90] [c0000000000ea808] __vio_register_driver+0x68/0x90
>> [ 1.859754][ T1] [c0000000063a3bb0] [c0000000010cee74] ibmvscsi_module_init+0xa4/0xdc
>> [ 1.859931][ T1] [c0000000063a3bf0] [c000000000012190] do_one_initcall+0x60/0x2c0
>> [ 1.860071][ T1] [c0000000063a3cc0] [c0000000010846e4] kernel_init_freeable+0x300/0x3a0
>> [ 1.860207][ T1] [c0000000063a3da0] [c000000000012764] kernel_init+0x2c/0x168
>> [ 1.860336][ T1] [c0000000063a3e10] [c00000000000d5ec] ret_from_kernel_thread+0x5c/0x70
>> [ 1.860690][ T1] Instruction dump:
>> [ 1.861072][ T1] fba10038 7cbb2b78 7c7d1b78 7cfc3b78 a1440048 2c2a0000 4082008c a13f004a
>> [ 1.861328][ T1] 7c095040 40810110 e93f0008 811f0028 <e9290390> e9290050 812903d8 7d3e4850
>> [ 1.863000][ T1] ---[ end trace c49ca2d91ee47d7f ]---
>> [ 1.879456][ T1]
>> [ 2.880941][ T1] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>>
>> I don't know what caused this, but it is some change since Friday.
>>
>> I have left it like this.
>
> Bisections leads to commit
>
> 42fb54fbc707 ("bio: limit bio max size")
>
> from the block tree. Reverting that commit on top of today's
> linux-next allows to the boot to work again.
The patch has been dropped, thanks Stephen.
--
Jens Axboe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox