* Re: [PATCH v3] PCI: Move pci_dev_is/assign_added() to pci.h
From: Bjorn Helgaas @ 2021-08-25 19:04 UTC (permalink / raw)
To: Niklas Schnelle
Cc: linux-arch, linux-s390, linux-kernel, Paul Mackerras, linux-pci,
Bjorn Helgaas, linuxppc-dev
In-Reply-To: <7595397d6c32ae8745201085956696866cc400b6.camel@linux.ibm.com>
On Mon, Aug 23, 2021 at 12:53:39PM +0200, Niklas Schnelle wrote:
> On Fri, 2021-08-20 at 17:37 -0500, Bjorn Helgaas wrote:
> > On Tue, Jul 20, 2021 at 05:01:45PM +0200, Niklas Schnelle wrote:
> > > The helper function pci_dev_is_added() from drivers/pci/pci.h is used in
> > > PCI arch code of both s390 and powerpc leading to awkward relative
> > > includes. Move it to the global include/linux/pci.h and get rid of these
> > > includes just for that one function.
> >
> > I agree the includes are awkward.
> >
> > But the arch code *using* pci_dev_is_added() seems awkward, too.
>
> See below for my interpretation why s390 has some driver like
> functionality in its arch code which isn't necessarily awkward.
>
> Independent from that I have found pci_dev_is_added() as the only way
> deal with the case that one might be looking at a struct pci_dev
> reference that has been removed via pci_stop_and_remove_bus_device() or
> has never been fully scanned. This is quite useful when handling error
> events which on s390 are part of the adapter event mechanism shared
> with channel I/O devices.
>
> > AFAICS, in powerpc, pci_dev_is_added() is only used by
> > pnv_pci_ioda_fixup_iov() and pseries_pci_fixup_iov_resources(). Those
> > are only called from pcibios_add_device(), which is only called from
> > pci_device_add().
> >
> > Is it even possible for pci_dev_is_added() to be true in that path?
If the pci_dev_is_added() in powerpc is unreachable, we can remove it
and at least reduce this to an s390-only problem.
> > s390 uses pci_dev_is_added() in recover_store()
>
> I'm actually looking into this as I'm working on an s390 implementation
> of the PCI recovery flow described in Documentation/PCI/pci-error-
> recovery.rst that would also call pci_dev_is_added() because when we
> get a platform notification of a PCI reset done by firmware it may be
> that the struct pci_dev is going away i.e. we still have a ref count
> but it is not added to the PCI bus anymore. And pci_dev_is_added() is
> the only way I've found to check for this state.
>
> > , but I don't know what
> > that is (looks like a sysfs file, but it's not documented) or why s390
> > is the only arch that does this.
>
> Good point about this not being documented, I'll look into adding docs.
>
> This is a sysfs attribute that basically removes the pci_dev and re-
> adds it. This has the complication that since the attribute sits at
> /sys/bus/pci/devices/<dev>/recover it deletes its own parent directory
> which requires extra caution and means concurrent accesses block on
> pci_lock_rescan_remove() instead of a kernfs lock.
> Long story short when concurrently triggering the attribute one thread
> proceeds into the pci_lock_rescan_remove() section and does the
> removal, while others would block on pci_lock_rescan_remove(). Now when
> the threads unblock the removal is done. In this case there is a new
> struct pci_dev found in the rescan but the previously blocked threads
> still have references to the old struct pci_dev which was removed and
> as far as I could tell can only be distinguished by checking
> pci_dev_is_added().
Is this locking issue different from concurrently writing to
/sys/.../remove on other architectures?
> > Maybe we should make powerpc and s390 less special?
>
> On s390, as I see it, the reason for this is that all of the PCI
> functionality is directly defined in the Architecture as special CPU
> instructions which are kind of hypercalls but also an ISA extension.
>
> These instructions range from the basic PCI memory accesses (no real
> MMIO) to enumeration of the devices and on to reporting of hot-plug and
> and resets/recovery events. Importantly we do not have any kind of
> direct access to a real or virtual PCI controller and the architecture
> has no concept of a comparable entity.
>
> So in my opinion while there is some of the functionality of a PCI
> controller in arch/s390/pci the cut off between controller
> functionality and arch support isn't clear at all and exposing PCI
> support as CPU instructions doesn't map well to the controller concept.
>
> That said, in principle I'm open to moving some of that into
> drivers/pci/controller/ if you think that would improve things and we
> can find a good argument what should go where. One possible cut off
> would be to have arch/s390/pci/ provide wrappers to the PCI
> instructions but move all their uses to e.g.
> drivers/pci/controller/s390/. This would of course be a major
> refactoring and none of that code would be useful on any other
> architecture but it would move a lot the accesses to PCI common code
> functionality out of the arch code.
Looks like hotplug is already in drivers/pci/hotplug/s390_pci_hpc.c.
Might be worth considering putting the other PCI core-ish code in
drivers/pci as well, though it doesn't feel urgent to me. Maybe a
good internship or mentoring project.
I'm not sure this juggling around is worth it basically to just clean
up the include path. The downside to me is exposing
pci_dev_is_added() to outside the PCI core, because I don't want
to encourage any other users.
> > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > ---
> > > Since v1 (and bad v2):
> > > - Fixed accidental removal of PCI_DPC_RECOVERED, PCI_DPC_RECOVERING
> > > defines and also move these to include/linux/pci.h
> > >
> > > arch/powerpc/platforms/powernv/pci-sriov.c | 3 ---
> > > arch/powerpc/platforms/pseries/setup.c | 1 -
> > > arch/s390/pci/pci_sysfs.c | 2 --
> > > drivers/pci/hotplug/acpiphp_glue.c | 1 -
> > > drivers/pci/pci.h | 15 ---------------
> > > include/linux/pci.h | 15 +++++++++++++++
> > > 6 files changed, 15 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
> > > index 28aac933a439..2e0ca5451e85 100644
> > > --- a/arch/powerpc/platforms/powernv/pci-sriov.c
> > > +++ b/arch/powerpc/platforms/powernv/pci-sriov.c
> > > @@ -9,9 +9,6 @@
> > >
> > > #include "pci.h"
> > >
> > > -/* for pci_dev_is_added() */
> > > -#include "../../../../drivers/pci/pci.h"
> > >
> .. snip ..
>
^ permalink raw reply
* Re: [PATCH v2 2/2] powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto
From: Nathan Chancellor @ 2021-08-25 21:25 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, clang-built-linux, Paul Mackerras, llvm,
linuxppc-dev
In-Reply-To: <389962b1b702e3c78d169e59bcfac56282889173.1618331882.git.christophe.leroy@csgroup.eu>
Hi Christophe,
On Tue, Apr 13, 2021 at 04:38:10PM +0000, Christophe Leroy wrote:
> Using asm goto in __WARN_FLAGS() and WARN_ON() allows more
> flexibility to GCC.
>
> For that add an entry to the exception table so that
> program_check_exception() knowns where to resume execution
> after a WARNING.
>
> Here are two exemples. The first one is done on PPC32 (which
> benefits from the previous patch), the second is on PPC64.
>
> unsigned long test(struct pt_regs *regs)
> {
> int ret;
>
> WARN_ON(regs->msr & MSR_PR);
>
> return regs->gpr[3];
> }
>
> unsigned long test9w(unsigned long a, unsigned long b)
> {
> if (WARN_ON(!b))
> return 0;
> return a / b;
> }
>
> Before the patch:
>
> 000003a8 <test>:
> 3a8: 81 23 00 84 lwz r9,132(r3)
> 3ac: 71 29 40 00 andi. r9,r9,16384
> 3b0: 40 82 00 0c bne 3bc <test+0x14>
> 3b4: 80 63 00 0c lwz r3,12(r3)
> 3b8: 4e 80 00 20 blr
>
> 3bc: 0f e0 00 00 twui r0,0
> 3c0: 80 63 00 0c lwz r3,12(r3)
> 3c4: 4e 80 00 20 blr
>
> 0000000000000bf0 <.test9w>:
> bf0: 7c 89 00 74 cntlzd r9,r4
> bf4: 79 29 d1 82 rldicl r9,r9,58,6
> bf8: 0b 09 00 00 tdnei r9,0
> bfc: 2c 24 00 00 cmpdi r4,0
> c00: 41 82 00 0c beq c0c <.test9w+0x1c>
> c04: 7c 63 23 92 divdu r3,r3,r4
> c08: 4e 80 00 20 blr
>
> c0c: 38 60 00 00 li r3,0
> c10: 4e 80 00 20 blr
>
> After the patch:
>
> 000003a8 <test>:
> 3a8: 81 23 00 84 lwz r9,132(r3)
> 3ac: 71 29 40 00 andi. r9,r9,16384
> 3b0: 40 82 00 0c bne 3bc <test+0x14>
> 3b4: 80 63 00 0c lwz r3,12(r3)
> 3b8: 4e 80 00 20 blr
>
> 3bc: 0f e0 00 00 twui r0,0
>
> 0000000000000c50 <.test9w>:
> c50: 7c 89 00 74 cntlzd r9,r4
> c54: 79 29 d1 82 rldicl r9,r9,58,6
> c58: 0b 09 00 00 tdnei r9,0
> c5c: 7c 63 23 92 divdu r3,r3,r4
> c60: 4e 80 00 20 blr
>
> c70: 38 60 00 00 li r3,0
> c74: 4e 80 00 20 blr
>
> In the first exemple, we see GCC doesn't need to duplicate what
> happens after the trap.
>
> In the second exemple, we see that GCC doesn't need to emit a test
> and a branch in the likely path in addition to the trap.
>
> We've got some WARN_ON() in .softirqentry.text section so it needs
> to be added in the OTHER_TEXT_SECTIONS in modpost.c
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
This patch as commit 1e688dd2a3d6 ("powerpc/bug: Provide better
flexibility to WARN_ON/__WARN_FLAGS() with asm goto") cause a WARN_ON in
klist_add_tail to trigger over and over on boot when compiling with
clang:
[ 2.177416][ T1] WARNING: CPU: 0 PID: 1 at lib/klist.c:62 .klist_add_tail+0x3c/0x110
[ 2.177456][ T1] Modules linked in:
[ 2.177481][ T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 5.14.0-rc7-next-20210825 #1
[ 2.177520][ T1] NIP: c0000000007ff81c LR: c00000000090a038 CTR: 0000000000000000
[ 2.177557][ T1] REGS: c0000000073c32a0 TRAP: 0700 Tainted: G W (5.14.0-rc7-next-20210825)
[ 2.177593][ T1] MSR: 8000000002029032 <SF,VEC,EE,ME,IR,DR,RI> CR: 22000a40 XER: 00000000
[ 2.177667][ T1] CFAR: c00000000090a034 IRQMASK: 0
[ 2.177667][ T1] GPR00: c00000000090a038 c0000000073c3540 c000000001be3200 0000000000000001
[ 2.177667][ T1] GPR04: c0000000072d65c0 0000000000000000 c0000000091ba798 c0000000091bb0a0
[ 2.177667][ T1] GPR08: 0000000000000001 0000000000000000 c000000008581918 fffffffffffffc00
[ 2.177667][ T1] GPR12: 0000000044000240 c000000001dd0000 c000000000012300 0000000000000000
[ 2.177667][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 2.177667][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 2.177667][ T1] GPR24: 0000000000000000 c0000000017e3200 0000000000000000 c000000001a0e778
[ 2.177667][ T1] GPR28: c0000000072d65b0 c0000000072d65a8 c000000007de72c8 c0000000073c35d0
[ 2.178019][ T1] NIP [c0000000007ff81c] .klist_add_tail+0x3c/0x110
[ 2.178058][ T1] LR [c00000000090a038] .bus_add_driver+0x148/0x290
[ 2.178088][ T1] Call Trace:
[ 2.178105][ T1] [c0000000073c3540] [c0000000073c35d0] 0xc0000000073c35d0 (unreliable)
[ 2.178150][ T1] [c0000000073c35d0] [c00000000090a038] .bus_add_driver+0x148/0x290
[ 2.178190][ T1] [c0000000073c3670] [c00000000090fae8] .driver_register+0xb8/0x190
[ 2.178234][ T1] [c0000000073c3700] [c000000000be55c0] .__hid_register_driver+0x70/0xd0
[ 2.178275][ T1] [c0000000073c37a0] [c00000000116955c] .redragon_driver_init+0x34/0x58
[ 2.178314][ T1] [c0000000073c3820] [c000000000011ae0] .do_one_initcall+0x130/0x3b0
[ 2.178357][ T1] [c0000000073c3bb0] [c0000000011065e0] .do_initcall_level+0xd8/0x188
[ 2.178403][ T1] [c0000000073c3c50] [c0000000011064a8] .do_initcalls+0x7c/0xdc
[ 2.178445][ T1] [c0000000073c3ce0] [c000000001106238] .kernel_init_freeable+0x178/0x21c
[ 2.178491][ T1] [c0000000073c3d90] [c000000000012334] .kernel_init+0x34/0x220
[ 2.178530][ T1] [c0000000073c3e10] [c00000000000cf50] .ret_from_kernel_thread+0x58/0x60
[ 2.178569][ T1] Instruction dump:
[ 2.178592][ T1] fba10078 7c7d1b78 38600001 fb810070 3b9d0008 fbc10080 7c9e2378 389d0018
[ 2.178662][ T1] fb9d0008 fb9d0010 90640000 fbdd0000 <0b1e0000> e87e0018 28230000 41820024
[ 2.178728][ T1] ---[ end trace 52ed3431f58f1847 ]---
Is this a bug with clang or is there something wrong with the patch? The
vmlinux image is available at [1] if you want to inspect it and our QEMU
command and the warning at boot can be viewed at [2]. If there is any
other information I can provide, please let me know.
[1] https://builds.tuxbuild.com/1xDcmp3Tvno0TTGxDVPedRKIKM2/
[2] https://github.com/ClangBuiltLinux/continuous-integration2/commit/cee159b66a58eb57fa2359e7888074b9da24126c/checks/3422232736/logs
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH v2 4/5] KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs
From: Sean Christopherson @ 2021-08-26 0:51 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: KVM list, Peter Zijlstra, Catalin Marinas, linux-kernel,
Will Deacon, Guo Ren, linux-kselftest, Ben Gardon, shuah,
Paul Mackerras, linux-s390, gor, Russell King, ARM Linux,
linux-csky, Christian Borntraeger, Ingo Molnar, Darren Hart,
linux-mips, Boqun Feng, paulmck, Heiko Carstens, rostedt,
Shakeel Butt, Andy Lutomirski, Thomas Gleixner, Peter Foley,
linux-arm-kernel, Thomas Bogendoerfer, Oleg Nesterov,
Paolo Bonzini, linuxppc-dev
In-Reply-To: <282257549.21721.1629732017655.JavaMail.zimbra@efficios.com>
On Mon, Aug 23, 2021, Mathieu Desnoyers wrote:
> [ re-send to Darren Hart ]
>
> ----- On Aug 23, 2021, at 11:18 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
>
> > ----- On Aug 20, 2021, at 6:50 PM, Sean Christopherson seanjc@google.com wrote:
> >
> >> Add a test to verify an rseq's CPU ID is updated correctly if the task is
> >> migrated while the kernel is handling KVM_RUN. This is a regression test
> >> for a bug introduced by commit 72c3c0fe54a3 ("x86/kvm: Use generic xfer
> >> to guest work function"), where TIF_NOTIFY_RESUME would be cleared by KVM
> >> without updating rseq, leading to a stale CPU ID and other badness.
> >>
> >
> > [...]
> >
> > +#define RSEQ_SIG 0xdeadbeef
> >
> > Is there any reason for defining a custom signature rather than including
> > tools/testing/selftests/rseq/rseq.h ? This should take care of including
> > the proper architecture header which will define the appropriate signature.
> >
> > Arguably you don't define rseq critical sections in this test per se, but
> > I'm wondering why the custom signature here.
Partly to avoid taking a dependency on rseq.h, and partly to try to call out that
the test doesn't actually do any rseq critical sections.
> > [...]
> >
> >> +
> >> +static void *migration_worker(void *ign)
> >> +{
> >> + cpu_set_t allowed_mask;
> >> + int r, i, nr_cpus, cpu;
> >> +
> >> + CPU_ZERO(&allowed_mask);
> >> +
> >> + nr_cpus = CPU_COUNT(&possible_mask);
> >> +
> >> + for (i = 0; i < 20000; i++) {
> >> + cpu = i % nr_cpus;
> >> + if (!CPU_ISSET(cpu, &possible_mask))
> >> + continue;
> >> +
> >> + CPU_SET(cpu, &allowed_mask);
> >> +
> >> + /*
> >> + * Bump the sequence count twice to allow the reader to detect
> >> + * that a migration may have occurred in between rseq and sched
> >> + * CPU ID reads. An odd sequence count indicates a migration
> >> + * is in-progress, while a completely different count indicates
> >> + * a migration occurred since the count was last read.
> >> + */
> >> + atomic_inc(&seq_cnt);
> >
> > So technically this atomic_inc contains the required barriers because the
> > selftests implementation uses "__sync_add_and_fetch(&addr->val, 1)". But
> > it's rather odd that the semantic differs from the kernel implementation in
> > terms of memory barriers: the kernel implementation of atomic_inc
> > guarantees no memory barriers, but this one happens to provide full
> > barriers pretty much by accident (selftests futex/include/atomic.h
> > documents no such guarantee).
Yeah, I got quite lost trying to figure out what atomics the test would actually
end up with.
> > If this full barrier guarantee is indeed provided by the selftests atomic.h
> > header, I would really like a comment stating that in the atomic.h header
> > so the carpet is not pulled from under our feet by a future optimization.
> >
> >
> >> + r = sched_setaffinity(0, sizeof(allowed_mask), &allowed_mask);
> >> + TEST_ASSERT(!r, "sched_setaffinity failed, errno = %d (%s)",
> >> + errno, strerror(errno));
> >> + atomic_inc(&seq_cnt);
> >> +
> >> + CPU_CLR(cpu, &allowed_mask);
> >> +
> >> + /*
> >> + * Let the read-side get back into KVM_RUN to improve the odds
> >> + * of task migration coinciding with KVM's run loop.
> >
> > This comment should be about increasing the odds of letting the seqlock
> > read-side complete. Otherwise, the delay between the two back-to-back
> > atomic_inc is so small that the seqlock read-side may never have time to
> > complete the reading the rseq cpu id and the sched_getcpu() call, and can
> > retry forever.
Hmm, but that's not why there's a delay. I'm not arguing that a livelock isn't
possible (though that syscall would have to be screaming fast), but the primary
motivation is very much to allow the read-side enough time to get back into KVM
proper.
To encounter the bug, TIF_NOTIFY_RESUME has to be recognized by KVM in its run
loop, i.e. sched_setaffinity() must induce task migration after the read-side has
invoked ioctl(KVM_RUN).
> > I'm wondering if 1 microsecond is sufficient on other architectures as
> > well.
I'm definitely wondering that as well :-)
> > One alternative way to make this depend less on the architecture's
> > implementation of sched_getcpu (whether it's a vDSO, or goes through a
> > syscall) would be to read the rseq cpu id and call sched_getcpu a few times
> > (e.g. 3 times) in the migration thread rather than use usleep, and throw
> > away the value read. This would ensure the delay is appropriate on all
> > architectures.
As above, I think an arbitrary delay is required regardless of how fast
sched_getcpu() can execute. One thought would be to do sched_getcpu() _and_
usleep() to account for sched_getcpu() overhead and to satisfy the KVM_RUN part,
but I don't know that that adds meaningful value.
The real test is if someone could see if the bug repros on non-x86 hardware...
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc/pseries: Parse control memory access error
From: Michael Ellerman @ 2021-08-26 2:36 UTC (permalink / raw)
To: Ganesh, linuxppc-dev; +Cc: mikey, mahesh, npiggin
In-Reply-To: <9734b599-eb00-a1d6-cd3c-ed6d0441f240@linux.ibm.com>
Ganesh <ganeshgr@linux.ibm.com> writes:
> On 8/24/21 12:09 PM, Michael Ellerman wrote:
>> Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
>>> Add support to parse and log control memory access
>>> error for pseries.
>>>
>>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>>> ---
>>> v2: No changes in this patch.
>>> ---
>>> arch/powerpc/platforms/pseries/ras.c | 21 +++++++++++++++++++++
>>> 1 file changed, 21 insertions(+)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
>>> index 167f2e1b8d39..608c35cad0c3 100644
>>> --- a/arch/powerpc/platforms/pseries/ras.c
>>> +++ b/arch/powerpc/platforms/pseries/ras.c
>>> @@ -80,6 +80,7 @@ struct pseries_mc_errorlog {
>>> #define MC_ERROR_TYPE_TLB 0x04
>>> #define MC_ERROR_TYPE_D_CACHE 0x05
>>> #define MC_ERROR_TYPE_I_CACHE 0x07
>>> +#define MC_ERROR_TYPE_CTRL_MEM_ACCESS 0x08
>> ...
>>>
>>> +#define MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK 0
>>> +#define MC_ERROR_CTRL_MEM_ACCESS_OP_ACCESS 1
>>
>> Where do the above values come from?
>
> It is from latest PAPR that added support for control memory error.
Please cite the version of the document and the section number in the
change log.
cheers
^ permalink raw reply
* Re: [PATCH v2 2/2] powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto
From: Michael Ellerman @ 2021-08-26 3:21 UTC (permalink / raw)
To: Nathan Chancellor, Christophe Leroy
Cc: llvm, linux-kernel, clang-built-linux, Paul Mackerras,
linuxppc-dev
In-Reply-To: <YSa1O4fcX1nNKqN/@Ryzen-9-3900X.localdomain>
Nathan Chancellor <nathan@kernel.org> writes:
> On Tue, Apr 13, 2021 at 04:38:10PM +0000, Christophe Leroy wrote:
>> Using asm goto in __WARN_FLAGS() and WARN_ON() allows more
>> flexibility to GCC.
...
>
> This patch as commit 1e688dd2a3d6 ("powerpc/bug: Provide better
> flexibility to WARN_ON/__WARN_FLAGS() with asm goto") cause a WARN_ON in
> klist_add_tail to trigger over and over on boot when compiling with
> clang:
>
> [ 2.177416][ T1] WARNING: CPU: 0 PID: 1 at lib/klist.c:62 .klist_add_tail+0x3c/0x110
> [ 2.177456][ T1] Modules linked in:
> [ 2.177481][ T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 5.14.0-rc7-next-20210825 #1
> [ 2.177520][ T1] NIP: c0000000007ff81c LR: c00000000090a038 CTR: 0000000000000000
> [ 2.177557][ T1] REGS: c0000000073c32a0 TRAP: 0700 Tainted: G W (5.14.0-rc7-next-20210825)
> [ 2.177593][ T1] MSR: 8000000002029032 <SF,VEC,EE,ME,IR,DR,RI> CR: 22000a40 XER: 00000000
> [ 2.177667][ T1] CFAR: c00000000090a034 IRQMASK: 0
> [ 2.177667][ T1] GPR00: c00000000090a038 c0000000073c3540 c000000001be3200 0000000000000001
> [ 2.177667][ T1] GPR04: c0000000072d65c0 0000000000000000 c0000000091ba798 c0000000091bb0a0
> [ 2.177667][ T1] GPR08: 0000000000000001 0000000000000000 c000000008581918 fffffffffffffc00
> [ 2.177667][ T1] GPR12: 0000000044000240 c000000001dd0000 c000000000012300 0000000000000000
> [ 2.177667][ T1] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 2.177667][ T1] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 2.177667][ T1] GPR24: 0000000000000000 c0000000017e3200 0000000000000000 c000000001a0e778
> [ 2.177667][ T1] GPR28: c0000000072d65b0 c0000000072d65a8 c000000007de72c8 c0000000073c35d0
> [ 2.178019][ T1] NIP [c0000000007ff81c] .klist_add_tail+0x3c/0x110
> [ 2.178058][ T1] LR [c00000000090a038] .bus_add_driver+0x148/0x290
> [ 2.178088][ T1] Call Trace:
> [ 2.178105][ T1] [c0000000073c3540] [c0000000073c35d0] 0xc0000000073c35d0 (unreliable)
> [ 2.178150][ T1] [c0000000073c35d0] [c00000000090a038] .bus_add_driver+0x148/0x290
> [ 2.178190][ T1] [c0000000073c3670] [c00000000090fae8] .driver_register+0xb8/0x190
> [ 2.178234][ T1] [c0000000073c3700] [c000000000be55c0] .__hid_register_driver+0x70/0xd0
> [ 2.178275][ T1] [c0000000073c37a0] [c00000000116955c] .redragon_driver_init+0x34/0x58
> [ 2.178314][ T1] [c0000000073c3820] [c000000000011ae0] .do_one_initcall+0x130/0x3b0
> [ 2.178357][ T1] [c0000000073c3bb0] [c0000000011065e0] .do_initcall_level+0xd8/0x188
> [ 2.178403][ T1] [c0000000073c3c50] [c0000000011064a8] .do_initcalls+0x7c/0xdc
> [ 2.178445][ T1] [c0000000073c3ce0] [c000000001106238] .kernel_init_freeable+0x178/0x21c
> [ 2.178491][ T1] [c0000000073c3d90] [c000000000012334] .kernel_init+0x34/0x220
> [ 2.178530][ T1] [c0000000073c3e10] [c00000000000cf50] .ret_from_kernel_thread+0x58/0x60
> [ 2.178569][ T1] Instruction dump:
> [ 2.178592][ T1] fba10078 7c7d1b78 38600001 fb810070 3b9d0008 fbc10080 7c9e2378 389d0018
> [ 2.178662][ T1] fb9d0008 fb9d0010 90640000 fbdd0000 <0b1e0000> e87e0018 28230000 41820024
> [ 2.178728][ T1] ---[ end trace 52ed3431f58f1847 ]---
>
> Is this a bug with clang or is there something wrong with the patch? The
> vmlinux image is available at [1] if you want to inspect it and our QEMU
> command and the warning at boot can be viewed at [2]. If there is any
> other information I can provide, please let me know.
>
> [1] https://builds.tuxbuild.com/1xDcmp3Tvno0TTGxDVPedRKIKM2/
> [2] https://github.com/ClangBuiltLinux/continuous-integration2/commit/cee159b66a58eb57fa2359e7888074b9da24126c/checks/3422232736/logs
Thanks.
This is the generated assembly:
c0000000007ff600 <.klist_add_tail>:
c0000000007ff600: 7c 08 02 a6 mflr r0
c0000000007ff604: f8 01 00 10 std r0,16(r1)
c0000000007ff608: f8 21 ff 71 stdu r1,-144(r1) ^ prolog
c0000000007ff60c: fb a1 00 78 std r29,120(r1) save r29 to stack
c0000000007ff610: 7c 7d 1b 78 mr r29,r3 r29 = struct klist_node *n
c0000000007ff614: 38 60 00 01 li r3,1 r3 = 1
c0000000007ff618: fb 81 00 70 std r28,112(r1) save r28 to stack
c0000000007ff61c: 3b 9d 00 08 addi r28,r29,8 r28 = &n->n_node
c0000000007ff620: fb c1 00 80 std r30,128(r1) save r30 to stack
c0000000007ff624: 7c 9e 23 78 mr r30,r4 r30 = struct klist *k
c0000000007ff628: 38 9d 00 18 addi r4,r29,24 r4 = &n->n_ref
c0000000007ff62c: fb 9d 00 08 std r28,8(r29) n->n_node.next = &n->n_node INIT_LIST_HEAD
c0000000007ff630: fb 9d 00 10 std r28,16(r29) n->n_node.prev = &n->n_node
c0000000007ff634: 90 64 00 00 stw r3,0(r4) kref_init(&n->n_ref)
c0000000007ff638: fb dd 00 00 std r30,0(r29) n->n_klist = k
c0000000007ff63c: 0b 1e 00 00 tdnei r30,0 trap if r30 (k) is not zero
From:
static void knode_set_klist(struct klist_node *knode, struct klist *klist)
{
knode->n_klist = klist;
/* no knode deserves to start its life dead */
WARN_ON(knode_dead(knode));
^^^^^^^^^^^^^^^^^
}
Which expands to:
static void knode_set_klist(struct klist_node *knode, struct klist *klist)
{
knode->n_klist = klist;
({
bool __ret_warn_on = false;
do {
...
} else {
__label__ __label_warn_on;
do {
asm goto(
"1: "
"tdnei"
"
" " % 4,
0 " "\n " ".section __ex_table,\"a\";"
" "
".balign 4;"
" "
".long (1b) - . ;"
" "
".long (%l[__label_warn_on]) - . ;"
" "
".previous"
" "
".section __bug_table,\"aw\"\n"
"2:\t.4byte 1b - 2b, %0 - 2b\n"
"\t.short %1, %2\n"
".org 2b+%3\n"
".previous\n"
:
: "i"("lib/klist.c"), "i"(62),
"i"((1 << 0) | ((9) << 8)),
"i"(sizeof(struct bug_entry)),
"r"(knode_dead(knode))
^^^^^^^^^^^^^^^^^^^^^
:
: __label_warn_on);
asm("");
} while (0);
break;
__label_warn_on:
__ret_warn_on = true;
}
} while (0);
__builtin_expect(!!(__ret_warn_on), 0);
});
}
And knode_dead is:
#define KNODE_DEAD 1LU
static bool knode_dead(struct klist_node *knode)
{
return (unsigned long)knode->n_klist & KNODE_DEAD;
}
So it's meant to warn if (n_klist & KNODE_DEAD) is not equal to zero.
But in the asm:
c0000000007ff600 <.klist_add_tail>:
...
c0000000007ff624: 7c 9e 23 78 mr r30,r4 r30 = struct klist *k
...
c0000000007ff638: fb dd 00 00 std r30,0(r29) n->n_klist = k
c0000000007ff63c: 0b 1e 00 00 tdnei r30,0 trap if r30 (k) is not zero
It's just warning if n_klist is not equal to zero. ie. we lost the "& KNODE_DEAD".
In the GCC output you can see it:
c0000000008c8a30 <.klist_node_init>:
c0000000008c8a30: 39 24 00 08 addi r9,r4,8
c0000000008c8a34: 39 40 00 01 li r10,1
c0000000008c8a38: f9 24 00 08 std r9,8(r4)
c0000000008c8a3c: f9 24 00 10 std r9,16(r4)
c0000000008c8a40: 91 44 00 18 stw r10,24(r4)
c0000000008c8a44: f8 64 00 00 std r3,0(r4)
c0000000008c8a48: 54 69 07 fe clrlwi r9,r3,31
c0000000008c8a4c: 0b 09 00 00 tdnei r9,0
ie. the clrlwi is "clear left (word) immediate", and zeroes everything
except bit 0, which is equivalent to "& KNODE_DEAD".
So there seems to be some misunderstanding with clang, it doesn't like
us passing an expression to the inline asm.
AFAIK it is legal to pass expressions as inputs to inline asm, ie. it
doesn't have to just be a variable name.
This patch seems to fix it. Not sure if that's just papering over it though.
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 1ee0f22313ee..75fcb4370d96 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -119,7 +119,7 @@ __label_warn_on: \
\
WARN_ENTRY(PPC_TLNEI " %4, 0", \
BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
- __label_warn_on, "r" (x)); \
+ __label_warn_on, "r" (!!(x))); \
break; \
__label_warn_on: \
__ret_warn_on = true; \
Generates:
c0000000008e2ac0 <.klist_add_tail>:
c0000000008e2ac0: 7c 08 02 a6 mflr r0
c0000000008e2ac4: f8 01 00 10 std r0,16(r1)
c0000000008e2ac8: f8 21 ff 71 stdu r1,-144(r1)
c0000000008e2acc: fb a1 00 78 std r29,120(r1)
c0000000008e2ad0: 7c 7d 1b 78 mr r29,r3
c0000000008e2ad4: 38 60 00 01 li r3,1
c0000000008e2ad8: fb c1 00 80 std r30,128(r1)
c0000000008e2adc: 7c 9e 23 78 mr r30,r4
c0000000008e2ae0: 38 9d 00 18 addi r4,r29,24
c0000000008e2ae4: 57 c5 07 fe clrlwi r5,r30,31 <-
c0000000008e2ae8: fb 81 00 70 std r28,112(r1)
c0000000008e2aec: 3b 9d 00 08 addi r28,r29,8
c0000000008e2af0: fb 9d 00 08 std r28,8(r29)
c0000000008e2af4: fb 9d 00 10 std r28,16(r29)
c0000000008e2af8: 90 64 00 00 stw r3,0(r4)
c0000000008e2afc: fb dd 00 00 std r30,0(r29)
c0000000008e2b00: 0b 05 00 00 tdnei r5,0 <-
cheers
^ permalink raw reply related
* Re: [PATCH v2 1/2] powerpc/bug: Remove specific powerpc BUG_ON() and WARN_ON() on PPC32
From: Nicholas Piggin @ 2021-08-26 3:26 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <20210818150653.GJ1583@gate.crashing.org>
Excerpts from Segher Boessenkool's message of August 19, 2021 1:06 am:
> On Fri, Aug 13, 2021 at 04:08:13PM +1000, Nicholas Piggin wrote:
>> This one possibly the branches end up in predictors, whereas conditional
>> trap is always just speculated not to hit. Branches may also have a
>> throughput limit on execution whereas trap could be more (1 per cycle
>> vs 4 per cycle on POWER9).
>
> I thought only *taken* branches are just one per cycle?
Taken branches are fetched by the front end at one per cycle (assuming
they hit the BTAC), but all branches have to be executed by BR at one
per cycle. On POWER9 BR even has to execute some other things like mflr
as well, but at least that's improved on POWER10.
Trap is executed at 4 per cycle and will never use branch table entries
or alias with a non-tagged predictor and mispredict.
> And those
> branches are only taken for the exceptional condition (or the case where
> we do not care about performance, anyway, if we do have an error most of
> the time ;-) )
It's not that big a deal, but trap is really the best instruction for
this.
>
>> On typical ppc32 CPUs, maybe it's a more obvious win. As you say there
>> is the CFAR issue as well which makes it a problem for 64s. It would
>> have been nice if it could use the same code though.
>
> On 64-bit the code looks better for the no-error path as well.
>
>> Maybe one day gcc's __builtin_trap() will become smart enough around
>> conditional statements that it it generates better code and tries to
>> avoid branches.
>
> Internally *all* traps are conditional, in GCC. It also can optimise
> them quite well. There must be something in the kernel macros that
> prevents good optimisation.
I did take a look at it at one point.
One problem is that the kernel needs the address of the trap instruction
to create the entry for it. The other problem is that __builtin_trap
does not return so it can't be used for WARN. LLVM at least seems to
have a __builtin_debugtrap which does return.
The first problem seems like the show stopper though. AFAIKS it would
need a special builtin support that does something to create the table
entry, or a guarantee that we could put an inline asm right after the
builtin as a recognized pattern and that would give us the instruction
following the trap.
Thanks,
Nick
^ permalink raw reply
* [powerpc:topic/ppc-kvm] BUILD SUCCESS 0c8fb653d487d2873f5eefdcaf4cecff4e103828
From: kernel test robot @ 2021-08-26 3:25 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git topic/ppc-kvm
branch HEAD: 0c8fb653d487d2873f5eefdcaf4cecff4e103828 powerpc/64s: Remove WORT SPR from POWER9/10
elapsed time: 736m
configs tested: 130
configs skipped: 86
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
i386 randconfig-c001-20210825
sh rsk7269_defconfig
powerpc cm5200_defconfig
arm u8500_defconfig
powerpc mpc512x_defconfig
powerpc ebony_defconfig
arm sama5_defconfig
m68k amiga_defconfig
m68k sun3x_defconfig
xtensa common_defconfig
h8300 defconfig
powerpc ep8248e_defconfig
powerpc katmai_defconfig
xtensa audio_kc705_defconfig
powerpc gamecube_defconfig
mips maltasmvp_defconfig
powerpc microwatt_defconfig
arm assabet_defconfig
sh se7343_defconfig
powerpc canyonlands_defconfig
powerpc mpc885_ads_defconfig
powerpc ep88xc_defconfig
alpha alldefconfig
sh r7780mp_defconfig
powerpc tqm5200_defconfig
arm tegra_defconfig
arm axm55xx_defconfig
mips tb0219_defconfig
arm moxart_defconfig
mips decstation_defconfig
powerpc mpc8560_ads_defconfig
sh migor_defconfig
sh j2_defconfig
powerpc arches_defconfig
arc hsdk_defconfig
mips tb0226_defconfig
arm neponset_defconfig
powerpc mpc837x_mds_defconfig
powerpc ksi8560_defconfig
riscv rv32_defconfig
arm pxa_defconfig
powerpc tqm8540_defconfig
microblaze mmu_defconfig
powerpc mpc834x_itx_defconfig
mips loongson2k_defconfig
powerpc mpc8540_ads_defconfig
powerpc tqm8548_defconfig
ia64 allyesconfig
arm netwinder_defconfig
arm imote2_defconfig
sh rts7751r2d1_defconfig
powerpc tqm8541_defconfig
sh sh7770_generic_defconfig
arm imx_v4_v5_defconfig
sh sdk7786_defconfig
powerpc icon_defconfig
m68k multi_defconfig
ia64 allmodconfig
ia64 defconfig
x86_64 allnoconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a014-20210825
x86_64 randconfig-a015-20210825
x86_64 randconfig-a016-20210825
x86_64 randconfig-a013-20210825
x86_64 randconfig-a012-20210825
x86_64 randconfig-a011-20210825
i386 randconfig-a011-20210825
i386 randconfig-a016-20210825
i386 randconfig-a012-20210825
i386 randconfig-a014-20210825
i386 randconfig-a013-20210825
i386 randconfig-a015-20210825
arc randconfig-r043-20210825
riscv randconfig-r042-20210825
s390 randconfig-r044-20210825
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um x86_64_defconfig
um i386_defconfig
clang tested configs:
x86_64 randconfig-a005-20210825
x86_64 randconfig-a001-20210825
x86_64 randconfig-a006-20210825
x86_64 randconfig-a003-20210825
x86_64 randconfig-a004-20210825
x86_64 randconfig-a002-20210825
i386 randconfig-a006-20210825
i386 randconfig-a001-20210825
i386 randconfig-a002-20210825
i386 randconfig-a005-20210825
i386 randconfig-a004-20210825
i386 randconfig-a003-20210825
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v2 2/3] selftests/powerpc: Add test for real address error handling
From: Michael Ellerman @ 2021-08-26 3:27 UTC (permalink / raw)
To: Ganesh, linuxppc-dev; +Cc: mikey, mahesh, npiggin
In-Reply-To: <8edb13f5-f036-2f9d-4c89-55c51c4cdbc0@linux.ibm.com>
Ganesh <ganeshgr@linux.ibm.com> writes:
> On 8/24/21 6:18 PM, Michael Ellerman wrote:
>
>> Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
>>> Add test for real address or control memory address access
>>> error handling, using NX-GZIP engine.
>>>
>>> The error is injected by accessing the control memory address
>>> using illegal instruction, on successful handling the process
>>> attempting to access control memory address using illegal
>>> instruction receives SIGBUS.
>> ...
>>
>>> diff --git a/tools/testing/selftests/powerpc/mce/inject-ra-err.sh b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
>>> new file mode 100755
>>> index 000000000000..3633cdc651a1
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
>>> @@ -0,0 +1,18 @@
>>> +#!/bin/bash
>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>> +
>>> +if [[ ! -w /dev/crypto/nx-gzip ]]; then
>>> + echo "WARN: Can't access /dev/crypto/nx-gzip, skipping"
>>> + exit 0
>>> +fi
>>> +
>>> +timeout 5 ./inject-ra-err
>>> +
>>> +# 128 + 7 (SIGBUS) = 135, 128 is a exit code with special meaning.
>>> +if [ $? -ne 135 ]; then
>>> + echo "FAILED: Real address or Control memory access error not handled"
>>> + exit $?
>>> +fi
>>> +
>>> +echo "OK: Real address or Control memory access error is handled"
>>> +exit 0
>> I don't think we really need the shell script, we should be able to do
>> all that in the C code.
>>
>> Can you try this?
>
> it works!, We need to set timeout, with 120 sec timeout we may flood the dmesg.
Hmm. Does it keep faulting? The regs->nip += 4 is meant to avoid that.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
From: Michael Ellerman @ 2021-08-26 3:41 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b29c29d205737a833262df38e01c07139f1c3dec.1629899011.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> There is no point in modifying MSR_LE bit on CPUs not supporting
> little endian.
Isn't that an ABI break?
set_endian(PR_ENDIAN_BIG) should work on a big endian CPU, even if it
does nothing useful.
cheers
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 185beb290580..b2b9919795a2 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1995,6 +1995,10 @@ int set_endian(struct task_struct *tsk, unsigned int val)
> {
> struct pt_regs *regs = tsk->thread.regs;
>
> + if (!cpu_has_feature(CPU_FTR_PPC_LE) &&
> + !cpu_has_feature(CPU_FTR_REAL_LE))
> + return -EINVAL;
> +
> if ((val == PR_ENDIAN_LITTLE && !cpu_has_feature(CPU_FTR_REAL_LE)) ||
> (val == PR_ENDIAN_PPC_LITTLE && !cpu_has_feature(CPU_FTR_PPC_LE)))
> return -EINVAL;
> --
> 2.25.0
^ permalink raw reply
* Re: [FSL P50x0] lscpu reports wrong values since the RC1 of kernel 5.13
From: Srikar Dronamraju @ 2021-08-26 3:43 UTC (permalink / raw)
To: Christian Zigotzky
Cc: Darren Stevens, mad skateman, qemu-devel, R.T.Dickinson,
linuxppc-dev
In-Reply-To: <c16c3747-8c6c-fb27-4e07-a893b83a5580@xenosoft.de>
* Christian Zigotzky <chzigotzky@xenosoft.de> [2021-08-16 14:29:21]:
Hi Christian,
> I tested the RC6 of kernel 5.14 today and unfortunately the issue still
> exists. We have figured out that only P5040 SoCs are affected. [1]
> P5020 SoCs display the correct values.
> Please check the CPU changes in the PowerPC updates 5.13-1 and 5.13-2.
>
Thanks for reporting the issue.
Would it be possible to try
https://lore.kernel.org/linuxppc-dev/20210821092419.167454-3-srikar@linux.vnet.ibm.com/t/#u
If the above patch is not helping, then can you please collect the output of
cat /sys/devices/system/cpu/cpu*/topology/core_siblings
Were all the CPUs online at the time of boot?
Did we do any CPU online/offline operations post boot?
If we did CPU online/offline, can you capture the output just after the
boot along with lscpu output..
Since this is being seen on few SOCs, can you summarize the difference
between P5040 and P5020.
>
> [1] https://forum.hyperion-entertainment.com/viewtopic.php?p=53775#p53775
>
>
> On 09 August 2021 um 02:37 pm, Christian Zigotzky wrote:
> > Hi All,
> >
> > Lscpu reports wrong values [1] since the RC1 of kernel 5.13 on my FSL
> > P5040 Cyrus+ board (A-EON AmigaOne X5000). [2]
> > The differences are:
> >
> > Since the RC1 of kernel 5.13 (wrong values):
> >
> > Core(s) per socket: 1
> > Socket(s): 3
> >
I know that the socket count was off by 1, but I cant explain how its off by
2 here.
> > Before (correct values):
> >
> > Core(s) per socket: 4
> > Socket(s): 1
> >
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc/numa: Print debug statements only when required
From: Srikar Dronamraju @ 2021-08-26 4:47 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot, Peter Zijlstra,
Geetika Moolchandani, Valentin Schneider, Laurent Dufour,
linuxppc-dev, Ingo Molnar
In-Reply-To: <87r1ehbrux.fsf@mpe.ellerman.id.au>
* Michael Ellerman <mpe@ellerman.id.au> [2021-08-25 23:01:42]:
> Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> > * Laurent Dufour <ldufour@linux.ibm.com> [2021-08-23 11:21:33]:
> >> Le 21/08/2021 à 12:25, Srikar Dronamraju a écrit :
> >> > Currently, a debug message gets printed every time an attempt to
> >> > add(remove) a CPU. However this is redundant if the CPU is already added
> >> > (removed) from the node.
> >> >
> >
> > Its a fair point.
> >
> > Michael,
> >
> > Do you want me to resend this patch with s/pr_err/pr_warn for the above
> > line?
>
> I think what I'd prefer is if we stopped using this custom dbg() stuff
> in numa.c, and cleaned up all the messages to use pr_xxx().
>
> Those debug statements only appear if you boot with numa=debug, which is
> not documented anywhere and I had completely forgotten existed TBH.
>
> These days there's CONFIG_DYNAMIC_DEBUG for turning on/off messages,
> which is much more flexible.
>
> So can we drop the numa=debug bits, and convert all the dbg()s to
> pr_debug().
>
> And then do a pass converting all the printk("NUMA: ") to pr_xxx() which
> will get "numa:" from pr_fmt().
>
> cheers
Okay, will do the needful.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH] powerpc: Make set_endian() return EINVAL when not supporting little endian
From: Christophe Leroy @ 2021-08-26 4:55 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87bl5kc1os.fsf@mpe.ellerman.id.au>
Le 26/08/2021 à 05:41, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> There is no point in modifying MSR_LE bit on CPUs not supporting
>> little endian.
>
> Isn't that an ABI break?
Or an ABI fix ? I don't know.
My first thought was that all other 32 bits architectures were returning -EINVAL, but looking at the
man page of prctl, it is explicit that this is powerpc only.
>
> set_endian(PR_ENDIAN_BIG) should work on a big endian CPU, even if it
> does nothing useful.
Fair enough. But shouldn't in that case get_endian() return PR_ENDIAN_BIG instead of returning EINVAL ?
We can do one or the other, but I think it should at least be consistant between them, shouldn't it ?
Christophe
^ permalink raw reply
* Re: [PATCH] powerpc: Redefine HMT_xxx macros as empty on PPC32
From: Michael Ellerman @ 2021-08-26 4:57 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <c5a07fadea33d640ad10cecf0ac8faaec1c524e0.1629898474.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> HMT_xxx macros are macros for adjusting thread priority
> (hardware multi-threading) are macros inherited from PPC64
> via commit 5f7c690728ac ("[PATCH] powerpc: Merged ppc_asm.h")
>
> Those instructions are pointless on PPC32, but some common
> fonctions like arch_cpu_idle() use them.
>
> So make them empty on PPC32 to avoid those instructions.
I guess we're pretty sure no 32-bit CPUs do anything with those.
e6500 can run in 32-bit mode, and is 2-way threaded AIUI, so it's
*possible* it could use them.
But I can't find any mention of those special nops in the e6500 core
manual. And actually it does have documentation about thread priority
registers, PPR32, TPRI0/1, but it says they're not used in e6500.
So I guess this seems safe, I'll pick it up.
cheers
> diff --git a/arch/powerpc/include/asm/vdso/processor.h b/arch/powerpc/include/asm/vdso/processor.h
> index e072577bc7c0..8d79f994b4aa 100644
> --- a/arch/powerpc/include/asm/vdso/processor.h
> +++ b/arch/powerpc/include/asm/vdso/processor.h
> @@ -5,12 +5,21 @@
> #ifndef __ASSEMBLY__
>
> /* Macros for adjusting thread priority (hardware multi-threading) */
> +#ifdef CONFIG_PPC64
> #define HMT_very_low() asm volatile("or 31, 31, 31 # very low priority")
> #define HMT_low() asm volatile("or 1, 1, 1 # low priority")
> #define HMT_medium_low() asm volatile("or 6, 6, 6 # medium low priority")
> #define HMT_medium() asm volatile("or 2, 2, 2 # medium priority")
> #define HMT_medium_high() asm volatile("or 5, 5, 5 # medium high priority")
> #define HMT_high() asm volatile("or 3, 3, 3 # high priority")
> +#else
> +#define HMT_very_low()
> +#define HMT_low()
> +#define HMT_medium_low()
> +#define HMT_medium()
> +#define HMT_medium_high()
> +#define HMT_high()
> +#endif
>
> #ifdef CONFIG_PPC64
> #define cpu_relax() do { HMT_low(); HMT_medium(); barrier(); } while (0)
> --
> 2.25.0
^ permalink raw reply
* Re: [PATCH v2 2/2] powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto
From: Christophe Leroy @ 2021-08-26 6:37 UTC (permalink / raw)
To: Michael Ellerman, Nathan Chancellor
Cc: llvm, linux-kernel, clang-built-linux, Paul Mackerras,
linuxppc-dev
In-Reply-To: <87h7fcc2m4.fsf@mpe.ellerman.id.au>
Le 26/08/2021 à 05:21, Michael Ellerman a écrit :
> Nathan Chancellor <nathan@kernel.org> writes:
>> On Tue, Apr 13, 2021 at 04:38:10PM +0000, Christophe Leroy wrote:
>>> Using asm goto in __WARN_FLAGS() and WARN_ON() allows more
>>> flexibility to GCC.
> ...
>>
>> This patch as commit 1e688dd2a3d6 ("powerpc/bug: Provide better
>> flexibility to WARN_ON/__WARN_FLAGS() with asm goto") cause a WARN_ON in
>> klist_add_tail to trigger over and over on boot when compiling with
>> clang:
...
>
> This patch seems to fix it. Not sure if that's just papering over it though.
>
> diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
> index 1ee0f22313ee..75fcb4370d96 100644
> --- a/arch/powerpc/include/asm/bug.h
> +++ b/arch/powerpc/include/asm/bug.h
> @@ -119,7 +119,7 @@ __label_warn_on: \
> \
> WARN_ENTRY(PPC_TLNEI " %4, 0", \
> BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
> - __label_warn_on, "r" (x)); \
> + __label_warn_on, "r" (!!(x))); \
> break; \
> __label_warn_on: \
> __ret_warn_on = true; \
>
>
But for a simple WARN_ON() call:
void test(unsigned long b)
{
WARN_ON(b);
}
Without your change with GCC you get:
00000000000012d0 <.test>:
12d0: 0b 03 00 00 tdnei r3,0
12d4: 4e 80 00 20 blr
With the !! change you get:
00000000000012d0 <.test>:
12d0: 31 23 ff ff addic r9,r3,-1
12d4: 7d 29 19 10 subfe r9,r9,r3
12d8: 0b 09 00 00 tdnei r9,0
12dc: 4e 80 00 20 blr
Christophe
^ permalink raw reply
* [PATCH v2 0/3] powerpc/smp: Misc fixes
From: Srikar Dronamraju @ 2021-08-26 10:03 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Ingo Molnar, linuxppc-dev,
Valentin Schneider
Changelog : v1 -> v2:
v1: https://lore.kernel.org/linuxppc-dev/20210821092419.167454-1-srikar@linux.vnet.ibm.com/t/#u``
[ patch 1: Updated to use DIV_ROUND_UP instead of max to handle more situations ]
[ patch 2: updated changelog to make it more generic powerpc issue ]
The 1st patch fixes a regression which causes a crash when booted with
nr_cpus=2.
The 2nd patch fixes a regression where lscpu on PowerVM reports more
number of sockets that are available.
The 3rd patch updates the fallback when L2-cache properties are not
explicitly exposed to be the cpu_sibling_mask.
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Srikar Dronamraju (3):
powerpc/smp: Fix a crash while booting kvm guest with nr_cpus=2
powerpc/smp: Update cpu_core_map on all PowerPc systems
powerpc/smp: Enable CACHE domain for shared processor
arch/powerpc/kernel/smp.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
--
2.18.2
^ permalink raw reply
* [PATCH v2 1/3] powerpc/smp: Fix a crash while booting kvm guest with nr_cpus=2
From: Srikar Dronamraju @ 2021-08-26 10:03 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Aneesh Kumar K . V,
Valentin Schneider, linuxppc-dev, Ingo Molnar
In-Reply-To: <20210826100401.412519-1-srikar@linux.vnet.ibm.com>
Aneesh reported a crash with a fairly recent upstream kernel when
booting kernel whose commandline was appended with nr_cpus=2
1:mon> e
cpu 0x1: Vector: 300 (Data Access) at [c000000008a67bd0]
pc: c00000000002557c: cpu_to_chip_id+0x3c/0x100
lr: c000000000058380: start_secondary+0x460/0xb00
sp: c000000008a67e70
msr: 8000000000001033
dar: 10
dsisr: 80000
current = 0xc00000000891bb00
paca = 0xc0000018ff981f80 irqmask: 0x03 irq_happened: 0x01
pid = 0, comm = swapper/1
Linux version 5.13.0-rc3-15704-ga050a6d2b7e8 (kvaneesh@ltc-boston8) (gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #433 SMP Tue May 25 02:38:49 CDT 2021
1:mon> t
[link register ] c000000000058380 start_secondary+0x460/0xb00
[c000000008a67e70] c000000008a67eb0 (unreliable)
[c000000008a67eb0] c0000000000589d4 start_secondary+0xab4/0xb00
[c000000008a67f90] c00000000000c654 start_secondary_prolog+0x10/0x14
Current code assumes that num_possible_cpus() is always greater than
threads_per_core. However this may not be true when using nr_cpus=2 or
similar options. Handle the case where num_possible_cpus() is not an
exact multiple of threads_per_core.
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Fixes: c1e53367dab1 ("powerpc/smp: Cache CPU to chip lookup")
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Debugged-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog: v1 -> v2:
v1: - https://lore.kernel.org/linuxppc-dev/20210821092419.167454-2-srikar@linux.vnet.ibm.com/t/#u
Handled comment from Gautham Shenoy
[ Updated to use DIV_ROUND_UP instead of max to handle more situations ]
arch/powerpc/kernel/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 6c6e4d934d86..bf11b3c4eb28 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1074,7 +1074,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
}
if (cpu_to_chip_id(boot_cpuid) != -1) {
- int idx = num_possible_cpus() / threads_per_core;
+ int idx = DIV_ROUND_UP(num_possible_cpus(), threads_per_core);
/*
* All threads of a core will all belong to the same core,
--
2.18.2
^ permalink raw reply related
* [PATCH v2 2/3] powerpc/smp: Update cpu_core_map on all PowerPc systems
From: Srikar Dronamraju @ 2021-08-26 10:04 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Aneesh Kumar K . V,
Valentin Schneider, linuxppc-dev, Ingo Molnar
In-Reply-To: <20210826100401.412519-1-srikar@linux.vnet.ibm.com>
lscpu() uses core_siblings to list the number of sockets in the
system. core_siblings is set using topology_core_cpumask.
While optimizing the powerpc bootup path, Commit 4ca234a9cbd7
("powerpc/smp: Stop updating cpu_core_mask"). it was found that
updating cpu_core_mask() ended up taking a lot of time. It was thought
that on Powerpc, cpu_core_mask() would always be same as
cpu_cpu_mask() i.e number of sockets will always be equal to number of
nodes. As an optimization, cpu_core_mask() was made a snapshot of
cpu_cpu_mask().
However that was found to be false with PowerPc KVM guests, where each
node could have more than one socket. So with Commit c47f892d7aa6
("powerpc/smp: Reintroduce cpu_core_mask"), cpu_core_mask was updated
based on chip_id but in an optimized way using some mask manipulations
and chip_id caching.
However on non-PowerNV and non-pseries KVM guests (i.e not
implementing cpu_to_chip_id(), continued to use a copy of
cpu_cpu_mask().
There are two issues that were noticed on such systems
1. lscpu would report one extra socket.
On a IBM,9009-42A (aka zz system) which has only 2 chips/ sockets/
nodes, lscpu would report
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 160
On-line CPU(s) list: 0-159
Thread(s) per core: 8
Core(s) per socket: 6
Socket(s): 3 <--------------
NUMA node(s): 2
Model: 2.2 (pvr 004e 0202)
Model name: POWER9 (architected), altivec supported
Hypervisor vendor: pHyp
Virtualization type: para
L1d cache: 32K
L1i cache: 32K
L2 cache: 512K
L3 cache: 10240K
NUMA node0 CPU(s): 0-79
NUMA node1 CPU(s): 80-159
2. Currently cpu_cpu_mask is updated when a core is
added/removed. However its not updated when smt mode switching or on
CPUs are explicitly offlined. However all other percpu masks are
updated to ensure only active/online CPUs are in the masks.
This results in build_sched_domain traces since there will be CPUs in
cpu_cpu_mask() but those CPUs are not present in SMT / CACHE / MC /
NUMA domains. A loop of threads running smt mode switching and core
add/remove will soon show this trace.
Hence cpu_cpu_mask has to be update at smt mode switch.
This will have impact on cpu_core_mask(). cpu_core_mask() is a
snapshot of cpu_cpu_mask. Different CPUs within the same socket will
end up having different cpu_core_masks since they are snapshots at
different points of time. This means when lscpu will start reporting
many more sockets than the actual number of sockets/ nodes / chips.
Different ways to handle this problem:
A. Update the snapshot aka cpu_core_mask for all CPUs whenever
cpu_cpu_mask is updated. This would a non-optimal solution.
B. Instead of a cpumask_var_t, make cpu_core_map a cpumask pointer
pointing to cpu_cpu_mask. However percpu cpumask pointer is frowned
upon and we need a clean way to handle PowerPc KVM guest which is
not a snapshot.
C. Update cpu_core_masks all PowerPc systems like in PowerPc KVM
guests using mask manipulations. This approach is relatively simple
and unifies with the existing code.
D. On top of 3, we could also resurrect get_physical_package_id which
could return a nid for the said CPU. However this is not needed at this
time.
Option C is the preferred approach for now.
While this is somewhat a revert of Commit 4ca234a9cbd7 ("powerpc/smp:
Stop updating cpu_core_mask").
1. Plain revert has some conflicts
2. For chip_id == -1, the cpu_core_mask is made identical to
cpu_cpu_mask, unlike previously where cpu_core_mask was set to a core
if chip_id doesn't exist.
This goes by the principle that if chip_id is not exposed, then
sockets / chip / node share the same set of CPUs.
With the fix, lscpu o/p would be
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 160
On-line CPU(s) list: 0-159
Thread(s) per core: 8
Core(s) per socket: 6
Socket(s): 2 <--------------
NUMA node(s): 2
Model: 2.2 (pvr 004e 0202)
Model name: POWER9 (architected), altivec supported
Hypervisor vendor: pHyp
Virtualization type: para
L1d cache: 32K
L1i cache: 32K
L2 cache: 512K
L3 cache: 10240K
NUMA node0 CPU(s): 0-79
NUMA node1 CPU(s): 80-159
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Fixes: 4ca234a9cbd7 ("powerpc/smp: Stop updating cpu_core_mask")
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog : v1 -> v2:
v1:https://lore.kernel.org/linuxppc-dev/20210821092419.167454-3-srikar@linux.vnet.ibm.com/t/#u
Handled comments from Michael Ellerman
[ updated changelog to make it more generic powerpc issue ]
arch/powerpc/kernel/smp.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index bf11b3c4eb28..b5bd5a4708d0 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1492,6 +1492,7 @@ static void add_cpu_to_masks(int cpu)
* add it to it's own thread sibling mask.
*/
cpumask_set_cpu(cpu, cpu_sibling_mask(cpu));
+ cpumask_set_cpu(cpu, cpu_core_mask(cpu));
for (i = first_thread; i < first_thread + threads_per_core; i++)
if (cpu_online(i))
@@ -1509,11 +1510,6 @@ static void add_cpu_to_masks(int cpu)
if (chip_id_lookup_table && ret)
chip_id = cpu_to_chip_id(cpu);
- if (chip_id == -1) {
- cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
- goto out;
- }
-
if (shared_caches)
submask_fn = cpu_l2_cache_mask;
@@ -1523,6 +1519,10 @@ static void add_cpu_to_masks(int cpu)
/* Skip all CPUs already part of current CPU core mask */
cpumask_andnot(mask, cpu_online_mask, cpu_core_mask(cpu));
+ /* If chip_id is -1; limit the cpu_core_mask to within DIE*/
+ if (chip_id == -1)
+ cpumask_and(mask, mask, cpu_cpu_mask(cpu));
+
for_each_cpu(i, mask) {
if (chip_id == cpu_to_chip_id(i)) {
or_cpumasks_related(cpu, i, submask_fn, cpu_core_mask);
@@ -1532,7 +1532,6 @@ static void add_cpu_to_masks(int cpu)
}
}
-out:
free_cpumask_var(mask);
}
--
2.18.2
^ permalink raw reply related
* [PATCH v2 3/3] powerpc/smp: Enable CACHE domain for shared processor
From: Srikar Dronamraju @ 2021-08-26 10:04 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Ingo Molnar, linuxppc-dev,
Valentin Schneider
In-Reply-To: <20210826100401.412519-1-srikar@linux.vnet.ibm.com>
Currently CACHE domain is not enabled on shared processor mode PowerVM
LPARS. On PowerVM systems, 'ibm,thread-group' device-tree property 2
under cpu-device-node indicates which all CPUs share L2-cache. However
'ibm,thread-group' device-tree property 2 is a relatively new property.
In absence of 'ibm,thread-group' property 2, 'l2-cache' device property
under cpu-device-node could help system to identify CPUs sharing L2-cache.
However this property is not exposed by PhyP in shared processor mode
configurations.
In absence of properties that inform OS about which CPUs share L2-cache,
fallback on core boundary.
Here are some stats from Power9 shared LPAR with the changes.
$ lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 32
On-line CPU(s) list: 0-31
Thread(s) per core: 8
Core(s) per socket: 1
Socket(s): 3
NUMA node(s): 2
Model: 2.2 (pvr 004e 0202)
Model name: POWER9 (architected), altivec supported
Hypervisor vendor: pHyp
Virtualization type: para
L1d cache: 32K
L1i cache: 32K
NUMA node0 CPU(s): 16-23
NUMA node1 CPU(s): 0-15,24-31
Physical sockets: 2
Physical chips: 1
Physical cores/chip: 10
Before patch
$ grep -r . /sys/kernel/debug/sched/domains/cpu0/domain*/name
Before
/sys/kernel/debug/sched/domains/cpu0/domain0/name:SMT
/sys/kernel/debug/sched/domains/cpu0/domain1/name:DIE
/sys/kernel/debug/sched/domains/cpu0/domain2/name:NUMA
After
/sys/kernel/debug/sched/domains/cpu0/domain0/name:SMT
/sys/kernel/debug/sched/domains/cpu0/domain1/name:CACHE
/sys/kernel/debug/sched/domains/cpu0/domain2/name:DIE
/sys/kernel/debug/sched/domains/cpu0/domain3/name:NUMA
$ awk '/domain/{print $1, $2}' /proc/schedstat | sort -u | sed -e 's/00000000,//g'
Before
domain0 00000055
domain0 000000aa
domain0 00005500
domain0 0000aa00
domain0 00550000
domain0 00aa0000
domain0 55000000
domain0 aa000000
domain1 00ff0000
domain1 ff00ffff
domain2 ffffffff
After
domain0 00000055
domain0 000000aa
domain0 00005500
domain0 0000aa00
domain0 00550000
domain0 00aa0000
domain0 55000000
domain0 aa000000
domain1 000000ff
domain1 0000ff00
domain1 00ff0000
domain1 ff000000
domain2 ff00ffff
domain2 ffffffff
domain3 ffffffff
(Lower is better)
perf stat -a -r 5 -n perf bench sched pipe | tail -n 2
Before
153.798 +- 0.142 seconds time elapsed ( +- 0.09% )
After
111.545 +- 0.652 seconds time elapsed ( +- 0.58% )
which is an improvement of 27.47%
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index b5bd5a4708d0..b31b8ca3ae2e 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1365,7 +1365,7 @@ static bool update_mask_by_l2(int cpu, cpumask_var_t *mask)
l2_cache = cpu_to_l2cache(cpu);
if (!l2_cache || !*mask) {
/* Assume only core siblings share cache with this CPU */
- for_each_cpu(i, submask_fn(cpu))
+ for_each_cpu(i, cpu_sibling_mask(cpu))
set_cpus_related(cpu, i, cpu_l2_cache_mask);
return false;
--
2.18.2
^ permalink raw reply related
* [PATCH v3 0/5] Updates to powerpc for robust CPU online/offline
From: Srikar Dronamraju @ 2021-08-26 10:05 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Geetika Moolchandani,
Ingo Molnar, Laurent Dufour, linuxppc-dev, Valentin Schneider
Changelog v2 -> v3:
v2: https://lore.kernel.org/linuxppc-dev/20210821102535.169643-1-srikar@linux.vnet.ibm.com/t/#u
Add patch 1: to drop dbg and numa=debug (Suggested by Michael Ellerman)
Add patch 2: to convert printk to pr_xxx (Suggested by Michael Ellerman)
Use pr_warn instead of pr_debug(WARNING) (Suggested by Laurent)
Changelog v1 -> v2:
Moved patch to this series: powerpc/numa: Fill distance_lookup_table for offline nodes
fixed a missing prototype warning
Scheduler expects unique number of node distances to be available
at boot. It uses node distance to calculate this unique node
distances. On Power Servers, node distances for offline nodes is not
available. However, Power Servers already knows unique possible node
distances. Fake the offline node's distance_lookup_table entries so
that all possible node distances are updated.
For example distance info from numactl from a fully populated 8 node
system at boot may look like this.
node distances:
node 0 1 2 3 4 5 6 7
0: 10 20 40 40 40 40 40 40
1: 20 10 40 40 40 40 40 40
2: 40 40 10 20 40 40 40 40
3: 40 40 20 10 40 40 40 40
4: 40 40 40 40 10 20 40 40
5: 40 40 40 40 20 10 40 40
6: 40 40 40 40 40 40 10 20
7: 40 40 40 40 40 40 20 10
However the same system when only two nodes are online at boot, then
distance info from numactl will look like
node distances:
node 0 1
0: 10 20
1: 20 10
With the faked numa distance at boot, the node distance table will look
like
node 0 1 2
0: 10 20 40
1: 20 10 40
2: 40 40 10
The actual distance will be populated once the nodes are onlined.
Also when simultaneously running CPU online/offline with CPU
add/remove in a loop, we see a WARNING messages.
WARNING: CPU: 13 PID: 1142 at kernel/sched/topology.c:898 build_sched_domains+0xd48/0x1720
Modules linked in: rpadlpar_io rpaphp mptcp_diag xsk_diag tcp_diag udp_diag
raw_diag inet_diag unix_diag af_packet_diag netlink_diag bonding tls
nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet
nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat
nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables nfnetlink
pseries_rng xts vmx_crypto uio_pdrv_genirq uio binfmt_misc ip_tables xfs
libcrc32c dm_service_time sd_mod t10_pi sg ibmvfc scsi_transport_fc ibmveth
dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse
CPU: 13 PID: 1142 Comm: kworker/13:2 Not tainted 5.13.0-rc6+ #28
Workqueue: events cpuset_hotplug_workfn
NIP: c0000000001caac8 LR: c0000000001caac4 CTR: 00000000007088ec
REGS: c00000005596f220 TRAP: 0700 Not tainted (5.13.0-rc6+)
MSR: 8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 48828222 XER: 00000009
CFAR: c0000000001ea698 IRQMASK: 0
GPR00: c0000000001caac4 c00000005596f4c0 c000000001c4a400 0000000000000036
GPR04: 00000000fffdffff c00000005596f1d0 0000000000000027 c0000018cfd07f90
GPR08: 0000000000000023 0000000000000001 0000000000000027 c0000018fe68ffe8
GPR12: 0000000000008000 c00000001e9d1880 c00000013a047200 0000000000000800
GPR16: c000000001d3c7d0 0000000000000240 0000000000000048 c000000010aacd18
GPR20: 0000000000000001 c000000010aacc18 c00000013a047c00 c000000139ec2400
GPR24: 0000000000000280 c000000139ec2520 c000000136c1b400 c000000001c93060
GPR28: c00000013a047c20 c000000001d3c6c0 c000000001c978a0 000000000000000d
NIP [c0000000001caac8] build_sched_domains+0xd48/0x1720
LR [c0000000001caac4] build_sched_domains+0xd44/0x1720
Call Trace:
[c00000005596f4c0] [c0000000001caac4] build_sched_domains+0xd44/0x1720 (unreliable)
[c00000005596f670] [c0000000001cc5ec] partition_sched_domains_locked+0x3ac/0x4b0
[c00000005596f710] [c0000000002804e4] rebuild_sched_domains_locked+0x404/0x9e0
[c00000005596f810] [c000000000283e60] rebuild_sched_domains+0x40/0x70
[c00000005596f840] [c000000000284124] cpuset_hotplug_workfn+0x294/0xf10
[c00000005596fc60] [c000000000175040] process_one_work+0x290/0x590
[c00000005596fd00] [c0000000001753c8] worker_thread+0x88/0x620
[c00000005596fda0] [c000000000181704] kthread+0x194/0x1a0
[c00000005596fe10] [c00000000000ccec] ret_from_kernel_thread+0x5c/0x70
Instruction dump:
485af049 60000000 2fa30800 409e0028 80fe0000 e89a00f8 e86100e8 38da0120
7f88e378 7ce53b78 4801fb91 60000000 <0fe00000> 39000000 38e00000 38c00000
This was because cpu_cpu_mask() was not getting updated on CPU
online/offline but would be only updated when add/remove of CPUs.
Other cpumasks get updated both on CPU online/offline and add/remove
Update cpu_cpu_mask() on CPU online/offline too.
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Srikar Dronamraju (5):
powerpc/numa: Drop dbg in favour of pr_debug
powerpc/numa: convert printk to pr_xxx
powerpc/numa: Print debug statements only when required
powerpc/numa: Update cpu_cpu_map on CPU online/offline
powerpc/numa: Fill distance_lookup_table for offline nodes
arch/powerpc/include/asm/topology.h | 12 +++
arch/powerpc/kernel/smp.c | 3 +
arch/powerpc/mm/numa.c | 120 ++++++++++++++++++++--------
3 files changed, 103 insertions(+), 32 deletions(-)
--
2.18.2
^ permalink raw reply
* [PATCH v5 2/5] powerpc/numa: convert printk to pr_xxx
From: Srikar Dronamraju @ 2021-08-26 10:05 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Geetika Moolchandani,
Ingo Molnar, Laurent Dufour, linuxppc-dev, Valentin Schneider
In-Reply-To: <20210826100521.412639-1-srikar@linux.vnet.ibm.com>
Convert the remaining printk to pr_xxx
One advantage would be all prints will now have prefix "numa:" from
pr_fmt().
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
[ convert printk(KERN_ERR) to pr_warn : Suggested by Laurent Dufour ]
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/mm/numa.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 5e9b777a1151..9af38b1c618b 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -154,8 +154,7 @@ static void unmap_cpu_from_node(unsigned long cpu)
if (cpumask_test_cpu(cpu, node_to_cpumask_map[node])) {
cpumask_clear_cpu(cpu, node_to_cpumask_map[node]);
} else {
- printk(KERN_ERR "WARNING: cpu %lu not found in node %d\n",
- cpu, node);
+ pr_warn("WARNING: cpu %lu not found in node %d\n", cpu, node);
}
}
#endif /* CONFIG_HOTPLUG_CPU || CONFIG_PPC_SPLPAR */
@@ -326,8 +325,7 @@ static int __init find_min_common_depth(void)
depth = of_read_number(distance_ref_points, 1);
} else {
if (distance_ref_points_depth < 2) {
- printk(KERN_WARNING "NUMA: "
- "short ibm,associativity-reference-points\n");
+ pr_warn("short ibm,associativity-reference-points\n");
goto err;
}
@@ -339,7 +337,7 @@ static int __init find_min_common_depth(void)
* MAX_DISTANCE_REF_POINTS domains.
*/
if (distance_ref_points_depth > MAX_DISTANCE_REF_POINTS) {
- printk(KERN_WARNING "NUMA: distance array capped at "
+ pr_warn("distance array capped at "
"%d entries\n", MAX_DISTANCE_REF_POINTS);
distance_ref_points_depth = MAX_DISTANCE_REF_POINTS;
}
@@ -701,7 +699,7 @@ static int __init parse_numa_properties(void)
unsigned long i;
if (numa_enabled == 0) {
- printk(KERN_WARNING "NUMA disabled by user\n");
+ pr_warn("disabled by user\n");
return -1;
}
@@ -716,7 +714,7 @@ static int __init parse_numa_properties(void)
return min_common_depth;
}
- pr_debug("NUMA associativity depth for CPU/Memory: %d\n", min_common_depth);
+ pr_debug("associativity depth for CPU/Memory: %d\n", min_common_depth);
/*
* Even though we connect cpus to numa domains later in SMP
@@ -808,10 +806,8 @@ static void __init setup_nonnuma(void)
unsigned int nid = 0;
int i;
- printk(KERN_DEBUG "Top of RAM: 0x%lx, Total RAM: 0x%lx\n",
- top_of_ram, total_ram);
- printk(KERN_DEBUG "Memory hole size: %ldMB\n",
- (top_of_ram - total_ram) >> 20);
+ pr_debug("Top of RAM: 0x%lx, Total RAM: 0x%lx\n", top_of_ram, total_ram);
+ pr_debug("Memory hole size: %ldMB\n", (top_of_ram - total_ram) >> 20);
for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, NULL) {
fake_numa_create_new_node(end_pfn, &nid);
--
2.18.2
^ permalink raw reply related
* [PATCH v3 3/5] powerpc/numa: Print debug statements only when required
From: Srikar Dronamraju @ 2021-08-26 10:05 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Geetika Moolchandani,
Ingo Molnar, Laurent Dufour, linuxppc-dev, Valentin Schneider
In-Reply-To: <20210826100521.412639-1-srikar@linux.vnet.ibm.com>
Currently, a debug message gets printed every time an attempt to
add(remove) a CPU. However this is redundant if the CPU is already added
(removed) from the node.
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/mm/numa.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 9af38b1c618b..6655ecdeddef 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -138,10 +138,10 @@ static void map_cpu_to_node(int cpu, int node)
{
update_numa_cpu_lookup_table(cpu, node);
- pr_debug("adding cpu %d to node %d\n", cpu, node);
-
- if (!(cpumask_test_cpu(cpu, node_to_cpumask_map[node])))
+ if (!(cpumask_test_cpu(cpu, node_to_cpumask_map[node]))) {
+ pr_debug("adding cpu %d to node %d\n", cpu, node);
cpumask_set_cpu(cpu, node_to_cpumask_map[node]);
+ }
}
#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_PPC_SPLPAR)
@@ -149,10 +149,9 @@ static void unmap_cpu_from_node(unsigned long cpu)
{
int node = numa_cpu_lookup_table[cpu];
- pr_debug("removing cpu %lu from node %d\n", cpu, node);
-
if (cpumask_test_cpu(cpu, node_to_cpumask_map[node])) {
cpumask_clear_cpu(cpu, node_to_cpumask_map[node]);
+ pr_debug("removing cpu %lu from node %d\n", cpu, node);
} else {
pr_warn("WARNING: cpu %lu not found in node %d\n", cpu, node);
}
--
2.18.2
^ permalink raw reply related
* [PATCH v3 4/5] powerpc/numa: Update cpu_cpu_map on CPU online/offline
From: Srikar Dronamraju @ 2021-08-26 10:05 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Geetika Moolchandani,
Ingo Molnar, Laurent Dufour, linuxppc-dev, Valentin Schneider
In-Reply-To: <20210826100521.412639-1-srikar@linux.vnet.ibm.com>
cpu_cpu_map holds all the CPUs in the DIE. However in PowerPC, when
onlining/offlining of CPUs, this mask doesn't get updated. This mask
is however updated when CPUs are added/removed. So when both
operations like online/offline of CPUs and adding/removing of CPUs are
done simultaneously, then cpumaps end up broken.
WARNING: CPU: 13 PID: 1142 at kernel/sched/topology.c:898
build_sched_domains+0xd48/0x1720
Modules linked in: rpadlpar_io rpaphp mptcp_diag xsk_diag tcp_diag
udp_diag raw_diag inet_diag unix_diag af_packet_diag netlink_diag
bonding tls nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib
nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct
nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set
rfkill nf_tables nfnetlink pseries_rng xts vmx_crypto uio_pdrv_genirq
uio binfmt_misc ip_tables xfs libcrc32c dm_service_time sd_mod t10_pi sg
ibmvfc scsi_transport_fc ibmveth dm_multipath dm_mirror dm_region_hash
dm_log dm_mod fuse
CPU: 13 PID: 1142 Comm: kworker/13:2 Not tainted 5.13.0-rc6+ #28
Workqueue: events cpuset_hotplug_workfn
NIP: c0000000001caac8 LR: c0000000001caac4 CTR: 00000000007088ec
REGS: c00000005596f220 TRAP: 0700 Not tainted (5.13.0-rc6+)
MSR: 8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 48828222 XER:
00000009
CFAR: c0000000001ea698 IRQMASK: 0
GPR00: c0000000001caac4 c00000005596f4c0 c000000001c4a400 0000000000000036
GPR04: 00000000fffdffff c00000005596f1d0 0000000000000027 c0000018cfd07f90
GPR08: 0000000000000023 0000000000000001 0000000000000027 c0000018fe68ffe8
GPR12: 0000000000008000 c00000001e9d1880 c00000013a047200 0000000000000800
GPR16: c000000001d3c7d0 0000000000000240 0000000000000048 c000000010aacd18
GPR20: 0000000000000001 c000000010aacc18 c00000013a047c00 c000000139ec2400
GPR24: 0000000000000280 c000000139ec2520 c000000136c1b400 c000000001c93060
GPR28: c00000013a047c20 c000000001d3c6c0 c000000001c978a0 000000000000000d
NIP [c0000000001caac8] build_sched_domains+0xd48/0x1720
LR [c0000000001caac4] build_sched_domains+0xd44/0x1720
Call Trace:
[c00000005596f4c0] [c0000000001caac4] build_sched_domains+0xd44/0x1720 (unreliable)
[c00000005596f670] [c0000000001cc5ec] partition_sched_domains_locked+0x3ac/0x4b0
[c00000005596f710] [c0000000002804e4] rebuild_sched_domains_locked+0x404/0x9e0
[c00000005596f810] [c000000000283e60] rebuild_sched_domains+0x40/0x70
[c00000005596f840] [c000000000284124] cpuset_hotplug_workfn+0x294/0xf10
[c00000005596fc60] [c000000000175040] process_one_work+0x290/0x590
[c00000005596fd00] [c0000000001753c8] worker_thread+0x88/0x620
[c00000005596fda0] [c000000000181704] kthread+0x194/0x1a0
[c00000005596fe10] [c00000000000ccec] ret_from_kernel_thread+0x5c/0x70
Instruction dump:
485af049 60000000 2fa30800 409e0028 80fe0000 e89a00f8 e86100e8 38da0120
7f88e378 7ce53b78 4801fb91 60000000 <0fe00000> 39000000 38e00000 38c00000
Fix this by updating cpu_cpu_map aka cpumask_of_node() on every CPU
online/offline.
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/topology.h | 12 ++++++++++++
arch/powerpc/kernel/smp.c | 3 +++
arch/powerpc/mm/numa.c | 7 ++-----
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index e4db64c0e184..2f0a4d7b95f6 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -65,6 +65,11 @@ static inline int early_cpu_to_node(int cpu)
int of_drconf_to_nid_single(struct drmem_lmb *lmb);
+extern void map_cpu_to_node(int cpu, int node);
+#ifdef CONFIG_HOTPLUG_CPU
+extern void unmap_cpu_from_node(unsigned long cpu);
+#endif /* CONFIG_HOTPLUG_CPU */
+
#else
static inline int early_cpu_to_node(int cpu) { return 0; }
@@ -93,6 +98,13 @@ static inline int of_drconf_to_nid_single(struct drmem_lmb *lmb)
return first_online_node;
}
+#ifdef CONFIG_SMP
+static inline void map_cpu_to_node(int cpu, int node) {}
+#ifdef CONFIG_HOTPLUG_CPU
+static inline void unmap_cpu_from_node(unsigned long cpu) {}
+#endif /* CONFIG_HOTPLUG_CPU */
+#endif /* CONFIG_SMP */
+
#endif /* CONFIG_NUMA */
#if defined(CONFIG_NUMA) && defined(CONFIG_PPC_SPLPAR)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index b31b8ca3ae2e..d947a4fd753c 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1407,6 +1407,8 @@ static void remove_cpu_from_masks(int cpu)
struct cpumask *(*mask_fn)(int) = cpu_sibling_mask;
int i;
+ unmap_cpu_from_node(cpu);
+
if (shared_caches)
mask_fn = cpu_l2_cache_mask;
@@ -1491,6 +1493,7 @@ static void add_cpu_to_masks(int cpu)
* This CPU will not be in the online mask yet so we need to manually
* add it to it's own thread sibling mask.
*/
+ map_cpu_to_node(cpu, cpu_to_node(cpu));
cpumask_set_cpu(cpu, cpu_sibling_mask(cpu));
cpumask_set_cpu(cpu, cpu_core_mask(cpu));
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 6655ecdeddef..87ade2f56f45 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -134,7 +134,7 @@ static void reset_numa_cpu_lookup_table(void)
numa_cpu_lookup_table[cpu] = -1;
}
-static void map_cpu_to_node(int cpu, int node)
+void map_cpu_to_node(int cpu, int node)
{
update_numa_cpu_lookup_table(cpu, node);
@@ -145,7 +145,7 @@ static void map_cpu_to_node(int cpu, int node)
}
#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_PPC_SPLPAR)
-static void unmap_cpu_from_node(unsigned long cpu)
+void unmap_cpu_from_node(unsigned long cpu)
{
int node = numa_cpu_lookup_table[cpu];
@@ -592,9 +592,6 @@ static int ppc_numa_cpu_prepare(unsigned int cpu)
static int ppc_numa_cpu_dead(unsigned int cpu)
{
-#ifdef CONFIG_HOTPLUG_CPU
- unmap_cpu_from_node(cpu);
-#endif
return 0;
}
--
2.18.2
^ permalink raw reply related
* [PATCH v3 5/5] powerpc/numa: Fill distance_lookup_table for offline nodes
From: Srikar Dronamraju @ 2021-08-26 10:05 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Geetika Moolchandani,
Ingo Molnar, Laurent Dufour, linuxppc-dev, Valentin Schneider,
kernel test robot
In-Reply-To: <20210826100521.412639-1-srikar@linux.vnet.ibm.com>
Scheduler expects unique number of node distances to be available at
boot. It uses node distance to calculate this unique node distances.
On POWER, node distances for offline nodes is not available. However,
POWER already knows unique possible node distances. Fake the offline
node's distance_lookup_table entries so that all possible node
distances are updated.
However this only needs to be done if the number of unique node
distances that can be computed for online nodes is less than the
number of possible unique node distances as represented by
distance_ref_points_depth. When the node is actually onlined,
distance_lookup_table will be updated with actual entries.
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Cc: kernel test robot <lkp@intel.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/mm/numa.c | 70 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
Changelog:
v1: https://lore.kernel.org/linuxppc-dev/20210701041552.112072-3-srikar@linux.vnet.ibm.com/t/#u
[ Fixed a missing prototype warning Reported-by: kernel test robot <lkp@intel.com>]
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 87ade2f56f45..afa2ede4ac53 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -849,6 +849,75 @@ void __init dump_numa_cpu_topology(void)
}
}
+/*
+ * Scheduler expects unique number of node distances to be available at
+ * boot. It uses node distance to calculate this unique node distances. On
+ * POWER, node distances for offline nodes is not available. However, POWER
+ * already knows unique possible node distances. Fake the offline node's
+ * distance_lookup_table entries so that all possible node distances are
+ * updated.
+ */
+static void __init fake_update_distance_lookup_table(void)
+{
+ unsigned long distance_map;
+ int i, nr_levels, nr_depth, node;
+
+ if (!numa_enabled)
+ return;
+
+ if (!form1_affinity)
+ return;
+
+ /*
+ * distance_ref_points_depth lists the unique numa domains
+ * available. However it ignore LOCAL_DISTANCE. So add +1
+ * to get the actual number of unique distances.
+ */
+ nr_depth = distance_ref_points_depth + 1;
+
+ WARN_ON(nr_depth > sizeof(distance_map));
+
+ bitmap_zero(&distance_map, nr_depth);
+ bitmap_set(&distance_map, 0, 1);
+
+ for_each_online_node(node) {
+ int nd, distance = LOCAL_DISTANCE;
+
+ if (node == first_online_node)
+ continue;
+
+ nd = __node_distance(node, first_online_node);
+ for (i = 0; i < nr_depth; i++, distance *= 2) {
+ if (distance == nd) {
+ bitmap_set(&distance_map, i, 1);
+ break;
+ }
+ }
+ nr_levels = bitmap_weight(&distance_map, nr_depth);
+ if (nr_levels == nr_depth)
+ return;
+ }
+
+ for_each_node(node) {
+ if (node_online(node))
+ continue;
+
+ i = find_first_zero_bit(&distance_map, nr_depth);
+ if (i >= nr_depth || i == 0) {
+ pr_warn("Levels(%d) not matching levels(%d)", nr_levels, nr_depth);
+ return;
+ }
+
+ bitmap_set(&distance_map, i, 1);
+ while (i--)
+ distance_lookup_table[node][i] = node;
+
+ nr_levels = bitmap_weight(&distance_map, nr_depth);
+ if (nr_levels == nr_depth)
+ return;
+ }
+}
+
/* Initialize NODE_DATA for a node on the local memory */
static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
{
@@ -964,6 +1033,7 @@ void __init mem_topology_setup(void)
*/
numa_setup_cpu(cpu);
}
+ fake_update_distance_lookup_table();
}
void __init initmem_init(void)
--
2.18.2
^ permalink raw reply related
* [PATCH v5 1/5] powerpc/numa: Drop dbg in favour of pr_debug
From: Srikar Dronamraju @ 2021-08-26 10:05 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot,
Srikar Dronamraju, Peter Zijlstra, Geetika Moolchandani,
Ingo Molnar, Laurent Dufour, linuxppc-dev, Valentin Schneider
In-Reply-To: <20210826100521.412639-1-srikar@linux.vnet.ibm.com>
PowerPc supported numa=debug which is not documented. This option was
used to print early debug output. However something more flexible can be
achieved by using CONFIG_DYNAMIC_DEBUG.
Hence drop dbg (and numa=debug) in favour of pr_debug
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/mm/numa.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index f2bf98bdcea2..5e9b777a1151 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -40,9 +40,6 @@ static int numa_enabled = 1;
static char *cmdline __initdata;
-static int numa_debug;
-#define dbg(args...) if (numa_debug) { printk(KERN_INFO args); }
-
int numa_cpu_lookup_table[NR_CPUS];
cpumask_var_t node_to_cpumask_map[MAX_NUMNODES];
struct pglist_data *node_data[MAX_NUMNODES];
@@ -79,7 +76,7 @@ static void __init setup_node_to_cpumask_map(void)
alloc_bootmem_cpumask_var(&node_to_cpumask_map[node]);
/* cpumask_of_node() will now work */
- dbg("Node to cpumask map for %u nodes\n", nr_node_ids);
+ pr_debug("Node to cpumask map for %u nodes\n", nr_node_ids);
}
static int __init fake_numa_create_new_node(unsigned long end_pfn,
@@ -123,7 +120,7 @@ static int __init fake_numa_create_new_node(unsigned long end_pfn,
cmdline = p;
fake_nid++;
*nid = fake_nid;
- dbg("created new fake_node with id %d\n", fake_nid);
+ pr_debug("created new fake_node with id %d\n", fake_nid);
return 1;
}
return 0;
@@ -141,7 +138,7 @@ static void map_cpu_to_node(int cpu, int node)
{
update_numa_cpu_lookup_table(cpu, node);
- dbg("adding cpu %d to node %d\n", cpu, node);
+ pr_debug("adding cpu %d to node %d\n", cpu, node);
if (!(cpumask_test_cpu(cpu, node_to_cpumask_map[node])))
cpumask_set_cpu(cpu, node_to_cpumask_map[node]);
@@ -152,7 +149,7 @@ static void unmap_cpu_from_node(unsigned long cpu)
{
int node = numa_cpu_lookup_table[cpu];
- dbg("removing cpu %lu from node %d\n", cpu, node);
+ pr_debug("removing cpu %lu from node %d\n", cpu, node);
if (cpumask_test_cpu(cpu, node_to_cpumask_map[node])) {
cpumask_clear_cpu(cpu, node_to_cpumask_map[node]);
@@ -313,7 +310,7 @@ static int __init find_min_common_depth(void)
&distance_ref_points_depth);
if (!distance_ref_points) {
- dbg("NUMA: ibm,associativity-reference-points not found.\n");
+ pr_debug("NUMA: ibm,associativity-reference-points not found.\n");
goto err;
}
@@ -321,7 +318,7 @@ static int __init find_min_common_depth(void)
if (firmware_has_feature(FW_FEATURE_OPAL) ||
firmware_has_feature(FW_FEATURE_TYPE1_AFFINITY)) {
- dbg("Using form 1 affinity\n");
+ pr_debug("Using form 1 affinity\n");
form1_affinity = 1;
}
@@ -719,7 +716,7 @@ static int __init parse_numa_properties(void)
return min_common_depth;
}
- dbg("NUMA associativity depth for CPU/Memory: %d\n", min_common_depth);
+ pr_debug("NUMA associativity depth for CPU/Memory: %d\n", min_common_depth);
/*
* Even though we connect cpus to numa domains later in SMP
@@ -1014,9 +1011,6 @@ static int __init early_numa(char *p)
if (strstr(p, "off"))
numa_enabled = 0;
- if (strstr(p, "debug"))
- numa_debug = 1;
-
p = strstr(p, "fake=");
if (p)
cmdline = p + strlen("fake=");
@@ -1179,7 +1173,7 @@ static long vphn_get_associativity(unsigned long cpu,
switch (rc) {
case H_SUCCESS:
- dbg("VPHN hcall succeeded. Reset polling...\n");
+ pr_debug("VPHN hcall succeeded. Reset polling...\n");
goto out;
case H_FUNCTION:
--
2.18.2
^ permalink raw reply related
* [PATCH 1/2] ASoC: fsl_rpmsg: add soc specific data structure
From: Shengjiu Wang @ 2021-08-26 10:57 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
Each platform has different supported rates and
formats, so add soc specific data for each platform.
This soc specific data is attached with compatible string.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_rpmsg.c | 47 +++++++++++++++++++++++++++++++++++----
sound/soc/fsl/fsl_rpmsg.h | 12 ++++++++++
2 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/sound/soc/fsl/fsl_rpmsg.c b/sound/soc/fsl/fsl_rpmsg.c
index d60f4dac6c1b..bda6cc96071d 100644
--- a/sound/soc/fsl/fsl_rpmsg.c
+++ b/sound/soc/fsl/fsl_rpmsg.c
@@ -138,11 +138,42 @@ static const struct snd_soc_component_driver fsl_component = {
.name = "fsl-rpmsg",
};
+static const struct fsl_rpmsg_soc_data imx7ulp_data = {
+ .rates = SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |
+ SNDRV_PCM_RATE_48000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE,
+};
+
+static const struct fsl_rpmsg_soc_data imx8mm_data = {
+ .rates = SNDRV_PCM_RATE_KNOT,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE |
+ SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_DSD_U8 |
+ SNDRV_PCM_FMTBIT_DSD_U16_LE | SNDRV_PCM_FMTBIT_DSD_U32_LE,
+};
+
+static const struct fsl_rpmsg_soc_data imx8mn_data = {
+ .rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |
+ SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 |
+ SNDRV_PCM_RATE_96000 | SNDRV_PCM_RATE_176400 |
+ SNDRV_PCM_RATE_192000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE |
+ SNDRV_PCM_FMTBIT_S32_LE,
+};
+
+static const struct fsl_rpmsg_soc_data imx8mp_data = {
+ .rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |
+ SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 |
+ SNDRV_PCM_RATE_96000 | SNDRV_PCM_RATE_176400 |
+ SNDRV_PCM_RATE_192000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE |
+ SNDRV_PCM_FMTBIT_S32_LE,
+};
+
static const struct of_device_id fsl_rpmsg_ids[] = {
- { .compatible = "fsl,imx7ulp-rpmsg-audio"},
- { .compatible = "fsl,imx8mm-rpmsg-audio"},
- { .compatible = "fsl,imx8mn-rpmsg-audio"},
- { .compatible = "fsl,imx8mp-rpmsg-audio"},
+ { .compatible = "fsl,imx7ulp-rpmsg-audio", .data = &imx7ulp_data},
+ { .compatible = "fsl,imx8mm-rpmsg-audio", .data = &imx8mm_data},
+ { .compatible = "fsl,imx8mn-rpmsg-audio", .data = &imx8mn_data},
+ { .compatible = "fsl,imx8mp-rpmsg-audio", .data = &imx8mp_data},
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, fsl_rpmsg_ids);
@@ -157,6 +188,14 @@ static int fsl_rpmsg_probe(struct platform_device *pdev)
if (!rpmsg)
return -ENOMEM;
+ rpmsg->soc_data = of_device_get_match_data(&pdev->dev);
+ if (rpmsg->soc_data) {
+ fsl_rpmsg_dai.playback.rates = rpmsg->soc_data->rates;
+ fsl_rpmsg_dai.capture.rates = rpmsg->soc_data->rates;
+ fsl_rpmsg_dai.playback.formats = rpmsg->soc_data->formats;
+ fsl_rpmsg_dai.capture.formats = rpmsg->soc_data->formats;
+ }
+
if (of_property_read_bool(np, "fsl,enable-lpa")) {
rpmsg->enable_lpa = 1;
rpmsg->buffer_size = LPA_LARGE_BUFFER_SIZE;
diff --git a/sound/soc/fsl/fsl_rpmsg.h b/sound/soc/fsl/fsl_rpmsg.h
index 4f5b49eb18d8..b04086fbf828 100644
--- a/sound/soc/fsl/fsl_rpmsg.h
+++ b/sound/soc/fsl/fsl_rpmsg.h
@@ -6,6 +6,16 @@
#ifndef __FSL_RPMSG_H
#define __FSL_RPMSG_H
+/*
+ * struct fsl_rpmsg_soc_data
+ * @rates: supported rates
+ * @formats: supported formats
+ */
+struct fsl_rpmsg_soc_data {
+ int rates;
+ u64 formats;
+};
+
/*
* struct fsl_rpmsg - rpmsg private data
*
@@ -15,6 +25,7 @@
* @pll8k: parent clock for multiple of 8kHz frequency
* @pll11k: parent clock for multiple of 11kHz frequency
* @card_pdev: Platform_device pointer to register a sound card
+ * @soc_data: soc specific data
* @mclk_streams: Active streams that are using baudclk
* @force_lpa: force enable low power audio routine if condition satisfy
* @enable_lpa: enable low power audio routine according to dts setting
@@ -27,6 +38,7 @@ struct fsl_rpmsg {
struct clk *pll8k;
struct clk *pll11k;
struct platform_device *card_pdev;
+ const struct fsl_rpmsg_soc_data *soc_data;
unsigned int mclk_streams;
int force_lpa;
int enable_lpa;
--
2.17.1
^ permalink raw reply related
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