* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Mike Galbraith @ 2021-11-11 3:16 UTC (permalink / raw)
To: Valentin Schneider, linux-kernel, kasan-dev, linuxppc-dev,
linux-kbuild
Cc: Marco Elver, Michal Marek, Peter Zijlstra, Frederic Weisbecker,
Nick Desaulniers, Steven Rostedt, Paul Mackerras, Masahiro Yamada,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <20211110202448.4054153-3-valentin.schneider@arm.com>
On Wed, 2021-11-10 at 20:24 +0000, Valentin Schneider wrote:
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 5f8db54226af..0640d5622496 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2073,6 +2073,22 @@ static inline void cond_resched_rcu(void)
> #endif
> }
>
> +#ifdef CONFIG_PREEMPT_DYNAMIC
> +
> +extern bool is_preempt_none(void);
> +extern bool is_preempt_voluntary(void);
> +extern bool is_preempt_full(void);
> +
> +#else
> +
> +#define is_preempt_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> +#define is_preempt_voluntary() IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> +#define is_preempt_full() IS_ENABLED(CONFIG_PREEMPT)
I think that should be IS_ENABLED(CONFIG_PREEMPTION), see c1a280b68d4e.
Noticed while applying the series to an RT tree, where tglx
has done that replacement to the powerpc spot your next patch diddles.
-Mike
^ permalink raw reply
* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Mike Galbraith @ 2021-11-11 3:35 UTC (permalink / raw)
To: Valentin Schneider, linux-kernel, kasan-dev, linuxppc-dev,
linux-kbuild
Cc: Marco Elver, Michal Marek, Peter Zijlstra, Frederic Weisbecker,
Nick Desaulniers, Steven Rostedt, Paul Mackerras, Masahiro Yamada,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <a7c704c2ae77e430d7f0657c5db664f877263830.camel@gmx.de>
On Thu, 2021-11-11 at 04:16 +0100, Mike Galbraith wrote:
> On Wed, 2021-11-10 at 20:24 +0000, Valentin Schneider wrote:
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 5f8db54226af..0640d5622496 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -2073,6 +2073,22 @@ static inline void cond_resched_rcu(void)
> > #endif
> > }
> >
> > +#ifdef CONFIG_PREEMPT_DYNAMIC
> > +
> > +extern bool is_preempt_none(void);
> > +extern bool is_preempt_voluntary(void);
> > +extern bool is_preempt_full(void);
> > +
> > +#else
> > +
> > +#define is_preempt_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> > +#define is_preempt_voluntary()
> > IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> > +#define is_preempt_full() IS_ENABLED(CONFIG_PREEMPT)
>
> I think that should be IS_ENABLED(CONFIG_PREEMPTION), see
> c1a280b68d4e.
>
> Noticed while applying the series to an RT tree, where tglx
> has done that replacement to the powerpc spot your next patch
> diddles.
Damn, then comes patch 5 properly differentiating PREEMPT/PREEMPT_RT.
-Mike
^ permalink raw reply
* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Mike Galbraith @ 2021-11-11 3:47 UTC (permalink / raw)
To: Valentin Schneider, linux-kernel, kasan-dev, linuxppc-dev,
linux-kbuild
Cc: Marco Elver, Michal Marek, Peter Zijlstra, Frederic Weisbecker,
Nick Desaulniers, Steven Rostedt, Paul Mackerras, Masahiro Yamada,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <803a905890530ea1b86db6ac45bd1fd940cf0ac3.camel@gmx.de>
On Thu, 2021-11-11 at 04:35 +0100, Mike Galbraith wrote:
> On Thu, 2021-11-11 at 04:16 +0100, Mike Galbraith wrote:
> > On Wed, 2021-11-10 at 20:24 +0000, Valentin Schneider wrote:
> > >
> > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > index 5f8db54226af..0640d5622496 100644
> > > --- a/include/linux/sched.h
> > > +++ b/include/linux/sched.h
> > > @@ -2073,6 +2073,22 @@ static inline void cond_resched_rcu(void)
> > > #endif
> > > }
> > >
> > > +#ifdef CONFIG_PREEMPT_DYNAMIC
> > > +
> > > +extern bool is_preempt_none(void);
> > > +extern bool is_preempt_voluntary(void);
> > > +extern bool is_preempt_full(void);
> > > +
> > > +#else
> > > +
> > > +#define is_preempt_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> > > +#define is_preempt_voluntary()
> > > IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> > > +#define is_preempt_full() IS_ENABLED(CONFIG_PREEMPT)
> >
> > I think that should be IS_ENABLED(CONFIG_PREEMPTION), see
> > c1a280b68d4e.
> >
> > Noticed while applying the series to an RT tree, where tglx
> > has done that replacement to the powerpc spot your next patch
> > diddles.
>
> Damn, then comes patch 5 properly differentiating PREEMPT/PREEMPT_RT.
So I suppose the powerpc spot should remain CONFIG_PREEMPT and become
CONFIG_PREEMPTION when the RT change gets merged, because that spot is
about full preemptibility, not a distinct preemption model.
That's rather annoying :-/
-Mike
^ permalink raw reply
* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Mike Galbraith @ 2021-11-11 3:55 UTC (permalink / raw)
To: Valentin Schneider, linux-kernel, kasan-dev, linuxppc-dev,
linux-kbuild
Cc: Marco Elver, Michal Marek, Peter Zijlstra, Frederic Weisbecker,
Nick Desaulniers, Steven Rostedt, Paul Mackerras, Masahiro Yamada,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <a7febd8825a2ab99bd1999664c6d4aa618b49442.camel@gmx.de>
On Thu, 2021-11-11 at 04:47 +0100, Mike Galbraith wrote:
>
> So I suppose the powerpc spot should remain CONFIG_PREEMPT and become
> CONFIG_PREEMPTION when the RT change gets merged, because that spot is
> about full preemptibility, not a distinct preemption model.
KCSAN needs a little help to be usable by RT, but ditto that spot.
-Mike
^ permalink raw reply
* Re: [PATCH/RFC] of: Shrink struct of_device_id
From: Frank Rowand @ 2021-11-11 4:06 UTC (permalink / raw)
To: Geert Uytterhoeven, Rob Herring
Cc: devicetree, linux-mips, linux-kernel, linux-renesas-soc,
linux-riscv, linuxppc-dev, linux-arm-kernel
In-Reply-To: <ef59d6fd3b2201b912d5eaa7f7a037d8f9adb744.1636561068.git.geert+renesas@glider.be>
Hi Geert,
On 11/10/21 11:23 AM, Geert Uytterhoeven wrote:
> Currently struct of_device_id is 196 (32-bit) or 200 (64-bit) bytes
> large. It contains fixed-size strings for a name, a type, and a
> compatible value, but the first two are barely used.
> OF device ID tables contain multiple entries, plus an empty sentinel
> entry.
>
> Statistics for my current kernel source tree:
> - 4487 tables with 16836 entries (3367200 bytes)
> - 176 names (average 6.7 max 23 chars)
> - 66 types (average 5.1 max 21 chars)
> - 12192 compatible values (average 18.0 max 45 chars)
> Taking into account the minimum needed size to store the strings, only
> 6.9% of the allocated space is used...
I like the idea of using less memory (and thank you for the above data!),
but I do not like the implementation, which reduces the size (of name at
least - I didn't check each field) to less than what the standard allows.
I have an idea of another way to accomplish the same goal, but I need to
dig a bit to make sure I'm not shooting myself in the foot.
-Frank
>
> Reduce kernel size by reducing the sizes of the fixed strings by one
> half.
>
> This reduces the size of an ARM multi_v7_defconfig kernel by ca. 400
> KiB. For a typical kernel supporting a single board, you can expect to
> save 50-100 KiB.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Notes:
> - While gcc complains if the non-NUL characters in a string do not fit
> in the available space, it does not complain if there is no space to
> store the string's NUL-terminator. However, that should be caught
> during testing, as the affected entry won't ever match. The kernel
> won't crash, as such strings will still be terminated by the
> sentinel in the table.
>
> - We could save even more by converting the strings from fixed-size
> arrays to pointers, at the expense of making it harder to mark
> entries __init. Given most drivers support binding and unbinding
> and thus cannot use __init for of_device_id tables, perhaps that's
> the way to go?
>
> Thanks for your comments!
> ---
> include/linux/mod_devicetable.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index ae2e75d15b219920..2bb2558d52d30d2b 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -266,9 +266,9 @@ struct sdw_device_id {
> * Struct used for matching a device
> */
> struct of_device_id {
> - char name[32];
> - char type[32];
> - char compatible[128];
> + char name[24];
> + char type[24];
> + char compatible[48];
> const void *data;
> };
>
>
^ permalink raw reply
* Re: [PATCH v2 3/5] powerpc: Use preemption model accessors
From: Michael Ellerman @ 2021-11-11 4:55 UTC (permalink / raw)
To: Valentin Schneider, linux-kernel, kasan-dev, linuxppc-dev,
linux-kbuild
Cc: Marco Elver, Michal Marek, Peter Zijlstra, Frederic Weisbecker,
Mike Galbraith, Nick Desaulniers, Steven Rostedt, Paul Mackerras,
Masahiro Yamada, Ingo Molnar, Dmitry Vyukov
In-Reply-To: <20211110202448.4054153-4-valentin.schneider@arm.com>
Valentin Schneider <valentin.schneider@arm.com> writes:
> Per PREEMPT_DYNAMIC, checking CONFIG_PREEMPT doesn't tell you the actual
> preemption model of the live kernel. Use the newly-introduced accessors
> instead.
>
> sched_init() -> preempt_dynamic_init() happens way before IRQs are set up,
> so this should be fine.
Despite the name interrupt_exit_kernel_prepare() is called before IRQs
are setup, traps and page faults are "interrupts" here.
So I'm not sure about adding that call there, because it will trigger a
WARN if called early in boot, which will trigger a trap and depending on
the context we may not survive.
I'd be happier if we can make it a build-time check.
cheers
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index de10a2697258..c56c10b59be3 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -552,7 +552,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
> /* Returning to a kernel context with local irqs enabled. */
> WARN_ON_ONCE(!(regs->msr & MSR_EE));
> again:
> - if (IS_ENABLED(CONFIG_PREEMPT)) {
> + if (is_preempt_full()) {
> /* Return to preemptible kernel context */
> if (unlikely(current_thread_info()->flags & _TIF_NEED_RESCHED)) {
> if (preempt_count() == 0)
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index aac8c0412ff9..1cb31bbdc925 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -265,7 +265,7 @@ static int __die(const char *str, struct pt_regs *regs, long err)
> printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s %s\n",
> IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) ? "LE" : "BE",
> PAGE_SIZE / 1024, get_mmu_str(),
> - IS_ENABLED(CONFIG_PREEMPT) ? " PREEMPT" : "",
> + is_preempt_full() ? " PREEMPT" : "",
> IS_ENABLED(CONFIG_SMP) ? " SMP" : "",
> IS_ENABLED(CONFIG_SMP) ? (" NR_CPUS=" __stringify(NR_CPUS)) : "",
> debug_pagealloc_enabled() ? " DEBUG_PAGEALLOC" : "",
> --
> 2.25.1
^ permalink raw reply
* Re: [PASEMI] Nemo board doesn't recognize any ATA disks with the pci-v5.16 updates
From: Christian Zigotzky @ 2021-11-11 5:24 UTC (permalink / raw)
To: Marc Zyngier, Bjorn Helgaas
Cc: axboe, Rob Herring, lorenzo.pieralisi, R.T.Dickinson,
Arnd Bergmann, kw, linux-pci@vger.kernel.org, damien.lemoal,
Olof Johansson, Darren Stevens, mad skateman,
bhelgaas@google.com >> Bjorn Helgaas, robert,
Matthew Leaman, linuxppc-dev, Alyssa Rosenzweig,
Christian Zigotzky
In-Reply-To: <87sfw3969l.wl-maz@kernel.org>
On 10 November 2021 at 08:09 pm, Marc Zyngier wrote:
> HI all,
>
> On Wed, 10 Nov 2021 18:41:06 +0000,
> Bjorn Helgaas <helgaas@kernel.org> wrote:
>> On Wed, Nov 10, 2021 at 07:07:24PM +0100, Christian Zigotzky wrote:
>>> On 09 November 2021 at 03:45 pm, Christian Zigotzky wrote:
>>>> Hello,
>>>>
>>>> The Nemo board [1] doesn't recognize any ATA disks with the pci-v5.16
>>> updates [2].
>>>> Error messages:
>>>>
>>>> ata4.00: gc timeout cmd 0xec
>>>> ata4.00: failed to IDENTIFY (I/O error, error_mask=0x4)
>>>> ata1.00: gc timeout cmd 0xec
>>>> ata1.00: failed to IDENTIFY (I/O error, error_mask=0x4)
>>>> ata3.00: gc timeout cmd 0xec
>>>> ata3.00: failed to IDENTIFY (I/O error, error_mask=0x4)
>>>>
>>>> I was able to revert the new pci-v5.16 updates [2]. After a new
>>> compiling, the kernel recognize all ATA disks correctly.
>>>> Could you please check the pci-v5.16 updates [2]?
>>>>
>>>> Please find attached the kernel config.
>>>>
>>>> Thanks,
>>>> Christian
>>>>
>>>> [1] https://en.wikipedia.org/wiki/AmigaOne_X1000
>>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c5c62ddf88c34bc83b66e4ac9beb2bb0e1887d4
>>> Hi All,
>>>
>>> Many thanks for your nice responses.
>>>
>>> I bisected today [1]. 0412841812265734c306ba5ef8088bcb64d5d3bd (of/irq:
>>> Allow matching of an interrupt-map local to an interrupt controller) [2] is
>>> the first bad commit.
>>>
>>> I was able to revert the first bad commit [1]. After a new compiling, the
>>> kernel detects all ATA disks without any problems.
>>>
>>> I created a patch for an easy reverting the bad commit [1]. With this patch
>>> we can do further our kernel tests.
>>>
>>> Could you please check the first bad commit [2]?
>>>
>>> Thanks,
>>> Christian
>>>
>>> [1] https://forum.hyperion-entertainment.com/viewtopic.php?p=54398#p54398
>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0412841812265734c306ba5ef8088bcb64d5d3bd
>>>
>>> [+ Marc Zyngier, Alyssa Rosenzweig, Lorenzo Pieralisi, and Rob Herring
>>> because of the first bad commit]
>> Thank you very much for the bisection and for also testing the revert!
>>
>> It's easy enough to revert 041284181226 ("of/irq: Allow matching of an
>> interrupt-map local to an interrupt controller"), and it seems like
>> that's what we need to do. I have it tentatively queued up.
>>
>> That commit was part of the new support for the Apple M1 PCIe
>> interface, and I don't know what effect a revert will have on that
>> support. Marc, Alyssa?
> It is going to badly break the M1 support, as we won't be able to take
> interrupts to detect that the PCIe link is up.
>
> Before we apply a full blown revert and decide that this isn't
> workable (and revert the whole M1 PCIe series, because they are
> otherwise somewhat pointless), I'd like to understand *what* breaks
> exactly.
>
> Christian, could you point me to the full DT that this machine uses?
> This would help understanding what goes wrong, and cook something for
> you to test.
>
> Thanks,
>
> M.
>
Hello Marc,
Here you are:
https://forum.hyperion-entertainment.com/viewtopic.php?p=54406#p54406
We are very happy to have the patch for reverting the bad commit because
we want to test the new PASEMI i2c driver with support for the Apple M1
[1] on our Nemo boards.
Thanks for your help,
Christian
[1] https://forum.hyperion-entertainment.com/viewtopic.php?p=54086#p54086
^ permalink raw reply
* Re: [PATCH] ppc64/fadump: fix inaccurate CPU state info in vmcore generated with panic
From: Michael Ellerman @ 2021-11-11 6:14 UTC (permalink / raw)
To: Hari Bathini, linuxppc-dev; +Cc: Hari Bathini, sourabhjain, mahesh, npiggin
In-Reply-To: <20211110190143.186346-1-hbathini@linux.ibm.com>
Hari Bathini <hbathini@linux.ibm.com> writes:
> In panic path, fadump is triggered via a panic notifier function.
> Before calling panic notifier functions, smp_send_stop() gets called,
> which stops all CPUs except the panic'ing CPU. Commit 8389b37dffdc
> ("powerpc: stop_this_cpu: remove the cpu from the online map.") and
> again commit bab26238bbd4 ("powerpc: Offline CPU in stop_this_cpu()")
> started marking CPUs as offline while stopping them. So, if a kernel
> has either of the above commits, vmcore captured with fadump via panic
> path would show only the panic'ing CPU as online. Sample output of
> crash-utility with such vmcore:
>
> # crash vmlinux vmcore
> ...
> KERNEL: vmlinux
> DUMPFILE: vmcore [PARTIAL DUMP]
> CPUS: 1
> DATE: Wed Nov 10 09:56:34 EST 2021
> UPTIME: 00:00:42
> LOAD AVERAGE: 2.27, 0.69, 0.24
> TASKS: 183
> NODENAME: XXXXXXXXX
> RELEASE: 5.15.0+
> VERSION: #974 SMP Wed Nov 10 04:18:19 CST 2021
> MACHINE: ppc64le (2500 Mhz)
> MEMORY: 8 GB
> PANIC: "Kernel panic - not syncing: sysrq triggered crash"
> PID: 3394
> COMMAND: "bash"
> TASK: c0000000150a5f80 [THREAD_INFO: c0000000150a5f80]
> CPU: 1
> STATE: TASK_RUNNING (PANIC)
>
> crash> p -x __cpu_online_mask
> __cpu_online_mask = $1 = {
> bits = {0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}
> }
> crash>
> crash>
> crash> p -x __cpu_active_mask
> __cpu_active_mask = $2 = {
> bits = {0xff, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}
> }
> crash>
>
> While this has been the case since fadump was introduced, the issue
> was not identified for two probable reasons:
>
> - In general, the bulk of the vmcores analyzed were from crash
> due to exception.
>
> - The above did change since commit 8341f2f222d7 ("sysrq: Use
> panic() to force a crash") started using panic() instead of
> deferencing NULL pointer to force a kernel crash. But then
> commit de6e5d38417e ("powerpc: smp_send_stop do not offline
> stopped CPUs") stopped marking CPUs as offline till kernel
> commit bab26238bbd4 ("powerpc: Offline CPU in stop_this_cpu()")
> reverted that change.
>
> To avoid vmcore from showing only one CPU as online in panic path,
> skip marking non panic'ing CPUs as offline while stopping them
> during fadump crash.
Is this really worth the added complexity/bug surface?
Why does it matter if the vmcore shows only one CPU online?
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index c23ee842c4c3..20555d5d5966 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -61,6 +61,7 @@
> #include <asm/cpu_has_feature.h>
> #include <asm/ftrace.h>
> #include <asm/kup.h>
> +#include <asm/fadump.h>
>
> #ifdef DEBUG
> #include <asm/udbg.h>
> @@ -626,7 +627,8 @@ static void nmi_stop_this_cpu(struct pt_regs *regs)
> /*
> * IRQs are already hard disabled by the smp_handle_nmi_ipi.
> */
> - set_cpu_online(smp_processor_id(), false);
> + if (!(oops_in_progress && should_fadump_crash()))
> + set_cpu_online(smp_processor_id(), false);
>
> spin_begin();
> while (1)
> @@ -650,7 +652,8 @@ static void stop_this_cpu(void *dummy)
> * to know other CPUs are offline before it breaks locks to flush
> * printk buffers, in case we panic()ed while holding the lock.
> */
> - set_cpu_online(smp_processor_id(), false);
> + if (!(oops_in_progress && should_fadump_crash()))
> + set_cpu_online(smp_processor_id(), false);
The comment talks about printk_safe_flush_on_panic(), and this change
would presumably break that. Except that printk_safe_flush_on_panic() no
longer exists.
So do we need to set the CPU online here at all?
ie. could we revert bab26238bbd4 ("powerpc: Offline CPU in stop_this_cpu()")
now that printk_safe_flush_on_panic() no longer exists?
cheers
^ permalink raw reply
* Re: [PASEMI] Nemo board doesn't recognize any ATA disks with the pci-v5.16 updates
From: Marc Zyngier @ 2021-11-11 7:13 UTC (permalink / raw)
To: Christian Zigotzky
Cc: axboe, Rob Herring, lorenzo.pieralisi, R.T.Dickinson,
Arnd Bergmann, kw, linux-pci@vger.kernel.org, damien.lemoal,
Olof Johansson, Darren Stevens, Bjorn Helgaas, mad skateman,
bhelgaas@google.com >> Bjorn Helgaas, robert,
Matthew Leaman, linuxppc-dev, Alyssa Rosenzweig,
Christian Zigotzky
In-Reply-To: <8cc64c3b-b0c0-fb41-9836-2e5e6a4459d1@xenosoft.de>
On Thu, 11 Nov 2021 05:24:52 +0000,
Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
>
> On 10 November 2021 at 08:09 pm, Marc Zyngier wrote:
> > HI all,
> >
> > On Wed, 10 Nov 2021 18:41:06 +0000,
> > Bjorn Helgaas <helgaas@kernel.org> wrote:
> >> On Wed, Nov 10, 2021 at 07:07:24PM +0100, Christian Zigotzky wrote:
> >>> On 09 November 2021 at 03:45 pm, Christian Zigotzky wrote:
> >>>> Hello,
> >>>>
> >>>> The Nemo board [1] doesn't recognize any ATA disks with the pci-v5.16
> >>> updates [2].
> >>>> Error messages:
> >>>>
> >>>> ata4.00: gc timeout cmd 0xec
> >>>> ata4.00: failed to IDENTIFY (I/O error, error_mask=0x4)
> >>>> ata1.00: gc timeout cmd 0xec
> >>>> ata1.00: failed to IDENTIFY (I/O error, error_mask=0x4)
> >>>> ata3.00: gc timeout cmd 0xec
> >>>> ata3.00: failed to IDENTIFY (I/O error, error_mask=0x4)
> >>>>
> >>>> I was able to revert the new pci-v5.16 updates [2]. After a new
> >>> compiling, the kernel recognize all ATA disks correctly.
> >>>> Could you please check the pci-v5.16 updates [2]?
> >>>>
> >>>> Please find attached the kernel config.
> >>>>
> >>>> Thanks,
> >>>> Christian
> >>>>
> >>>> [1] https://en.wikipedia.org/wiki/AmigaOne_X1000
> >>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c5c62ddf88c34bc83b66e4ac9beb2bb0e1887d4
> >>> Hi All,
> >>>
> >>> Many thanks for your nice responses.
> >>>
> >>> I bisected today [1]. 0412841812265734c306ba5ef8088bcb64d5d3bd (of/irq:
> >>> Allow matching of an interrupt-map local to an interrupt controller) [2] is
> >>> the first bad commit.
> >>>
> >>> I was able to revert the first bad commit [1]. After a new compiling, the
> >>> kernel detects all ATA disks without any problems.
> >>>
> >>> I created a patch for an easy reverting the bad commit [1]. With this patch
> >>> we can do further our kernel tests.
> >>>
> >>> Could you please check the first bad commit [2]?
> >>>
> >>> Thanks,
> >>> Christian
> >>>
> >>> [1] https://forum.hyperion-entertainment.com/viewtopic.php?p=54398#p54398
> >>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0412841812265734c306ba5ef8088bcb64d5d3bd
> >>>
> >>> [+ Marc Zyngier, Alyssa Rosenzweig, Lorenzo Pieralisi, and Rob Herring
> >>> because of the first bad commit]
> >> Thank you very much for the bisection and for also testing the revert!
> >>
> >> It's easy enough to revert 041284181226 ("of/irq: Allow matching of an
> >> interrupt-map local to an interrupt controller"), and it seems like
> >> that's what we need to do. I have it tentatively queued up.
> >>
> >> That commit was part of the new support for the Apple M1 PCIe
> >> interface, and I don't know what effect a revert will have on that
> >> support. Marc, Alyssa?
> > It is going to badly break the M1 support, as we won't be able to take
> > interrupts to detect that the PCIe link is up.
> >
> > Before we apply a full blown revert and decide that this isn't
> > workable (and revert the whole M1 PCIe series, because they are
> > otherwise somewhat pointless), I'd like to understand *what* breaks
> > exactly.
> >
> > Christian, could you point me to the full DT that this machine uses?
> > This would help understanding what goes wrong, and cook something for
> > you to test.
> >
> > Thanks,
> >
> > M.
> >
> Hello Marc,
>
> Here you are:
> https://forum.hyperion-entertainment.com/viewtopic.php?p=54406#p54406
This is not what I asked. I need the actual source file, or at the
very least the compiled object (the /sys/firmware/fdt file, for
example). Not an interpretation that I can't feed to the kernel.
Without this, I can't debug your problem.
> We are very happy to have the patch for reverting the bad commit
> because we want to test the new PASEMI i2c driver with support for the
> Apple M1 [1] on our Nemo boards.
You can revert the patch on your own. At this stage, we're not blindly
reverting things in the tree, but instead trying to understand what
is happening on your particular system.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PASEMI] Nemo board doesn't recognize any ATA disks with the pci-v5.16 updates
From: Christian Zigotzky @ 2021-11-11 7:47 UTC (permalink / raw)
To: Marc Zyngier
Cc: axboe, Rob Herring, lorenzo.pieralisi, R.T.Dickinson,
Arnd Bergmann, kw, linux-pci@vger.kernel.org, damien.lemoal,
Olof Johansson, Darren Stevens, Bjorn Helgaas, mad skateman,
bhelgaas@google.com >> Bjorn Helgaas, robert,
Matthew Leaman, linuxppc-dev, Alyssa Rosenzweig,
Christian Zigotzky
In-Reply-To: <87r1bn88rt.wl-maz@kernel.org>
On 11 November 2021 at 08:13 am, Marc Zyngier wrote:
> On Thu, 11 Nov 2021 05:24:52 +0000,
> Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
>> On 10 November 2021 at 08:09 pm, Marc Zyngier wrote:
>>> HI all,
>>>
>>> On Wed, 10 Nov 2021 18:41:06 +0000,
>>> Bjorn Helgaas <helgaas@kernel.org> wrote:
>>>> On Wed, Nov 10, 2021 at 07:07:24PM +0100, Christian Zigotzky wrote:
>>>>> On 09 November 2021 at 03:45 pm, Christian Zigotzky wrote:
>>>>>> Hello,
>>>>>>
>>>>>> The Nemo board [1] doesn't recognize any ATA disks with the pci-v5.16
>>>>> updates [2].
>>>>>> Error messages:
>>>>>>
>>>>>> ata4.00: gc timeout cmd 0xec
>>>>>> ata4.00: failed to IDENTIFY (I/O error, error_mask=0x4)
>>>>>> ata1.00: gc timeout cmd 0xec
>>>>>> ata1.00: failed to IDENTIFY (I/O error, error_mask=0x4)
>>>>>> ata3.00: gc timeout cmd 0xec
>>>>>> ata3.00: failed to IDENTIFY (I/O error, error_mask=0x4)
>>>>>>
>>>>>> I was able to revert the new pci-v5.16 updates [2]. After a new
>>>>> compiling, the kernel recognize all ATA disks correctly.
>>>>>> Could you please check the pci-v5.16 updates [2]?
>>>>>>
>>>>>> Please find attached the kernel config.
>>>>>>
>>>>>> Thanks,
>>>>>> Christian
>>>>>>
>>>>>> [1] https://en.wikipedia.org/wiki/AmigaOne_X1000
>>>>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c5c62ddf88c34bc83b66e4ac9beb2bb0e1887d4
>>>>> Hi All,
>>>>>
>>>>> Many thanks for your nice responses.
>>>>>
>>>>> I bisected today [1]. 0412841812265734c306ba5ef8088bcb64d5d3bd (of/irq:
>>>>> Allow matching of an interrupt-map local to an interrupt controller) [2] is
>>>>> the first bad commit.
>>>>>
>>>>> I was able to revert the first bad commit [1]. After a new compiling, the
>>>>> kernel detects all ATA disks without any problems.
>>>>>
>>>>> I created a patch for an easy reverting the bad commit [1]. With this patch
>>>>> we can do further our kernel tests.
>>>>>
>>>>> Could you please check the first bad commit [2]?
>>>>>
>>>>> Thanks,
>>>>> Christian
>>>>>
>>>>> [1] https://forum.hyperion-entertainment.com/viewtopic.php?p=54398#p54398
>>>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0412841812265734c306ba5ef8088bcb64d5d3bd
>>>>>
>>>>> [+ Marc Zyngier, Alyssa Rosenzweig, Lorenzo Pieralisi, and Rob Herring
>>>>> because of the first bad commit]
>>>> Thank you very much for the bisection and for also testing the revert!
>>>>
>>>> It's easy enough to revert 041284181226 ("of/irq: Allow matching of an
>>>> interrupt-map local to an interrupt controller"), and it seems like
>>>> that's what we need to do. I have it tentatively queued up.
>>>>
>>>> That commit was part of the new support for the Apple M1 PCIe
>>>> interface, and I don't know what effect a revert will have on that
>>>> support. Marc, Alyssa?
>>> It is going to badly break the M1 support, as we won't be able to take
>>> interrupts to detect that the PCIe link is up.
>>>
>>> Before we apply a full blown revert and decide that this isn't
>>> workable (and revert the whole M1 PCIe series, because they are
>>> otherwise somewhat pointless), I'd like to understand *what* breaks
>>> exactly.
>>>
>>> Christian, could you point me to the full DT that this machine uses?
>>> This would help understanding what goes wrong, and cook something for
>>> you to test.
>>>
>>> Thanks,
>>>
>>> M.
>>>
>> Hello Marc,
>>
>> Here you are:
>> https://forum.hyperion-entertainment.com/viewtopic.php?p=54406#p54406
> This is not what I asked. I need the actual source file, or at the
> very least the compiled object (the /sys/firmware/fdt file, for
> example). Not an interpretation that I can't feed to the kernel.
>
> Without this, I can't debug your problem.
>
>> We are very happy to have the patch for reverting the bad commit
>> because we want to test the new PASEMI i2c driver with support for the
>> Apple M1 [1] on our Nemo boards.
> You can revert the patch on your own. At this stage, we're not blindly
> reverting things in the tree, but instead trying to understand what
> is happening on your particular system.
>
> Thanks,
>
> M.
>
I added a download link for the fdt file to the post [1]. Please read
also Darren's comments in this post.
Thanks
[1] https://forum.hyperion-entertainment.com/viewtopic.php?p=54406#p54406
^ permalink raw reply
* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Marco Elver @ 2021-11-11 8:54 UTC (permalink / raw)
To: Valentin Schneider
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Mike Galbraith, Nick Desaulniers, linux-kernel, kasan-dev,
Paul Mackerras, Steven Rostedt, Masahiro Yamada, linuxppc-dev,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <20211110202448.4054153-3-valentin.schneider@arm.com>
On Wed, Nov 10, 2021 at 08:24PM +0000, Valentin Schneider wrote:
[...]
> +#ifdef CONFIG_PREEMPT_DYNAMIC
> +
> +extern bool is_preempt_none(void);
> +extern bool is_preempt_voluntary(void);
> +extern bool is_preempt_full(void);
> +
> +#else
> +
> +#define is_preempt_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> +#define is_preempt_voluntary() IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> +#define is_preempt_full() IS_ENABLED(CONFIG_PREEMPT)
> +
> +#endif
> +
> +#define is_preempt_rt() IS_ENABLED(CONFIG_PREEMPT_RT)
> +
Can these callables be real functions in all configs, making the
!DYNAMIC ones just static inline bool ones? That'd catch invalid use in
#if etc. in all configs.
> /*
> * Does a critical section need to be broken due to another
> * task waiting?: (technically does not depend on CONFIG_PREEMPTION,
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 97047aa7b6c2..9db7f77e53c3 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6638,6 +6638,17 @@ static void __init preempt_dynamic_init(void)
> }
> }
>
> +#define PREEMPT_MODE_ACCESSOR(mode) \
> + bool is_preempt_##mode(void) \
> + { \
> + WARN_ON_ONCE(preempt_dynamic_mode == preempt_dynamic_undefined); \
> + return preempt_dynamic_mode == preempt_dynamic_##mode; \
> + }
This needs an EXPORT_SYMBOL, so it's usable from modules like the
kcsan_test module.
^ permalink raw reply
* Re: [PATCH v2 1/5] preempt: Restore preemption model selection configs
From: Marco Elver @ 2021-11-11 8:58 UTC (permalink / raw)
To: Valentin Schneider
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Mike Galbraith, Nick Desaulniers, linux-kernel, kasan-dev,
Paul Mackerras, Steven Rostedt, Masahiro Yamada, linuxppc-dev,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <20211110202448.4054153-2-valentin.schneider@arm.com>
On Wed, Nov 10, 2021 at 08:24PM +0000, Valentin Schneider wrote:
> Commit c597bfddc9e9 ("sched: Provide Kconfig support for default dynamic
> preempt mode") changed the selectable config names for the preemption
> model. This means a config file must now select
>
> CONFIG_PREEMPT_BEHAVIOUR=y
>
> rather than
>
> CONFIG_PREEMPT=y
>
> to get a preemptible kernel. This means all arch config files would need to
> be updated - right now they'll all end up with the default
> CONFIG_PREEMPT_NONE_BEHAVIOUR.
>
> Rather than touch a good hundred of config files, restore usage of
> CONFIG_PREEMPT{_NONE, _VOLUNTARY}. Make them configure:
> o The build-time preemption model when !PREEMPT_DYNAMIC
> o The default boot-time preemption model when PREEMPT_DYNAMIC
>
> Add siblings of those configs with the _BUILD suffix to unconditionally
> designate the build-time preemption model (PREEMPT_DYNAMIC is built with
> the "highest" preemption model it supports, aka PREEMPT). Downstream
> configs should by now all be depending / selected by CONFIG_PREEMPTION
> rather than CONFIG_PREEMPT, so only a few sites need patching up.
>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Acked-by: Marco Elver <elver@google.com>
Much better, thank you!
> ---
> include/linux/kernel.h | 2 +-
> include/linux/vermagic.h | 2 +-
> init/Makefile | 2 +-
> kernel/Kconfig.preempt | 42 ++++++++++++++++++++--------------------
> kernel/sched/core.c | 6 +++---
> 5 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 2776423a587e..9c7d774ef809 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -88,7 +88,7 @@
> struct completion;
> struct user;
>
> -#ifdef CONFIG_PREEMPT_VOLUNTARY
> +#ifdef CONFIG_PREEMPT_VOLUNTARY_BUILD
>
> extern int __cond_resched(void);
> # define might_resched() __cond_resched()
> diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
> index 1eaaa93c37bf..329d63babaeb 100644
> --- a/include/linux/vermagic.h
> +++ b/include/linux/vermagic.h
> @@ -15,7 +15,7 @@
> #else
> #define MODULE_VERMAGIC_SMP ""
> #endif
> -#ifdef CONFIG_PREEMPT
> +#ifdef CONFIG_PREEMPT_BUILD
> #define MODULE_VERMAGIC_PREEMPT "preempt "
> #elif defined(CONFIG_PREEMPT_RT)
> #define MODULE_VERMAGIC_PREEMPT "preempt_rt "
> diff --git a/init/Makefile b/init/Makefile
> index 2846113677ee..04eeee12c076 100644
> --- a/init/Makefile
> +++ b/init/Makefile
> @@ -30,7 +30,7 @@ $(obj)/version.o: include/generated/compile.h
> quiet_cmd_compile.h = CHK $@
> cmd_compile.h = \
> $(CONFIG_SHELL) $(srctree)/scripts/mkcompile_h $@ \
> - "$(UTS_MACHINE)" "$(CONFIG_SMP)" "$(CONFIG_PREEMPT)" \
> + "$(UTS_MACHINE)" "$(CONFIG_SMP)" "$(CONFIG_PREEMPT_BUILD)" \
> "$(CONFIG_PREEMPT_RT)" $(CONFIG_CC_VERSION_TEXT) "$(LD)"
>
> include/generated/compile.h: FORCE
> diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
> index 60f1bfc3c7b2..ce77f0265660 100644
> --- a/kernel/Kconfig.preempt
> +++ b/kernel/Kconfig.preempt
> @@ -1,12 +1,23 @@
> # SPDX-License-Identifier: GPL-2.0-only
>
> +config PREEMPT_NONE_BUILD
> + bool
> +
> +config PREEMPT_VOLUNTARY_BUILD
> + bool
> +
> +config PREEMPT_BUILD
> + bool
> + select PREEMPTION
> + select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
> +
> choice
> prompt "Preemption Model"
> - default PREEMPT_NONE_BEHAVIOUR
> + default PREEMPT_NONE
>
> -config PREEMPT_NONE_BEHAVIOUR
> +config PREEMPT_NONE
> bool "No Forced Preemption (Server)"
> - select PREEMPT_NONE if !PREEMPT_DYNAMIC
> + select PREEMPT_NONE_BUILD if !PREEMPT_DYNAMIC
> help
> This is the traditional Linux preemption model, geared towards
> throughput. It will still provide good latencies most of the
> @@ -18,10 +29,10 @@ config PREEMPT_NONE_BEHAVIOUR
> raw processing power of the kernel, irrespective of scheduling
> latencies.
>
> -config PREEMPT_VOLUNTARY_BEHAVIOUR
> +config PREEMPT_VOLUNTARY
> bool "Voluntary Kernel Preemption (Desktop)"
> depends on !ARCH_NO_PREEMPT
> - select PREEMPT_VOLUNTARY if !PREEMPT_DYNAMIC
> + select PREEMPT_VOLUNTARY_BUILD if !PREEMPT_DYNAMIC
> help
> This option reduces the latency of the kernel by adding more
> "explicit preemption points" to the kernel code. These new
> @@ -37,10 +48,10 @@ config PREEMPT_VOLUNTARY_BEHAVIOUR
>
> Select this if you are building a kernel for a desktop system.
>
> -config PREEMPT_BEHAVIOUR
> +config PREEMPT
> bool "Preemptible Kernel (Low-Latency Desktop)"
> depends on !ARCH_NO_PREEMPT
> - select PREEMPT
> + select PREEMPT_BUILD
> help
> This option reduces the latency of the kernel by making
> all kernel code (that is not executing in a critical section)
> @@ -58,7 +69,7 @@ config PREEMPT_BEHAVIOUR
>
> config PREEMPT_RT
> bool "Fully Preemptible Kernel (Real-Time)"
> - depends on EXPERT && ARCH_SUPPORTS_RT && !PREEMPT_DYNAMIC
> + depends on EXPERT && ARCH_SUPPORTS_RT
> select PREEMPTION
> help
> This option turns the kernel into a real-time kernel by replacing
> @@ -75,17 +86,6 @@ config PREEMPT_RT
>
> endchoice
>
> -config PREEMPT_NONE
> - bool
> -
> -config PREEMPT_VOLUNTARY
> - bool
> -
> -config PREEMPT
> - bool
> - select PREEMPTION
> - select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK
> -
> config PREEMPT_COUNT
> bool
>
> @@ -95,8 +95,8 @@ config PREEMPTION
>
> config PREEMPT_DYNAMIC
> bool "Preemption behaviour defined on boot"
> - depends on HAVE_PREEMPT_DYNAMIC
> - select PREEMPT
> + depends on HAVE_PREEMPT_DYNAMIC && !PREEMPT_RT
> + select PREEMPT_BUILD
> default y
> help
> This option allows to define the preemption model on the kernel
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index f2611b9cf503..97047aa7b6c2 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6625,13 +6625,13 @@ __setup("preempt=", setup_preempt_mode);
> static void __init preempt_dynamic_init(void)
> {
> if (preempt_dynamic_mode == preempt_dynamic_undefined) {
> - if (IS_ENABLED(CONFIG_PREEMPT_NONE_BEHAVIOUR)) {
> + if (IS_ENABLED(CONFIG_PREEMPT_NONE)) {
> sched_dynamic_update(preempt_dynamic_none);
> - } else if (IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY_BEHAVIOUR)) {
> + } else if (IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)) {
> sched_dynamic_update(preempt_dynamic_voluntary);
> } else {
> /* Default static call setting, nothing to do */
> - WARN_ON_ONCE(!IS_ENABLED(CONFIG_PREEMPT_BEHAVIOUR));
> + WARN_ON_ONCE(!IS_ENABLED(CONFIG_PREEMPT));
> preempt_dynamic_mode = preempt_dynamic_full;
> pr_info("Dynamic Preempt: full\n");
> }
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v2 4/5] kscan: Use preemption model accessors
From: Marco Elver @ 2021-11-11 9:11 UTC (permalink / raw)
To: Valentin Schneider
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Mike Galbraith, Nick Desaulniers, linux-kernel, kasan-dev,
Paul Mackerras, Steven Rostedt, Masahiro Yamada, linuxppc-dev,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <20211110202448.4054153-5-valentin.schneider@arm.com>
Subject s/kscan/kcsan/
On Wed, Nov 10, 2021 at 08:24PM +0000, Valentin Schneider wrote:
> Per PREEMPT_DYNAMIC, checking CONFIG_PREEMPT doesn't tell you the actual
> preemption model of the live kernel. Use the newly-introduced accessors
> instead.
>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Reviewed-by: Marco Elver <elver@google.com>
Though it currently doesn't compile as a module due to missing
EXPORT_SYMBOL of is_preempt*().
> ---
> kernel/kcsan/kcsan_test.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/kcsan/kcsan_test.c b/kernel/kcsan/kcsan_test.c
> index dc55fd5a36fc..14d811eb9a21 100644
> --- a/kernel/kcsan/kcsan_test.c
> +++ b/kernel/kcsan/kcsan_test.c
> @@ -1005,13 +1005,13 @@ static const void *nthreads_gen_params(const void *prev, char *desc)
> else
> nthreads *= 2;
>
> - if (!IS_ENABLED(CONFIG_PREEMPT) || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) {
> + if (!is_preempt_full() || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) {
> /*
> * Without any preemption, keep 2 CPUs free for other tasks, one
> * of which is the main test case function checking for
> * completion or failure.
> */
> - const long min_unused_cpus = IS_ENABLED(CONFIG_PREEMPT_NONE) ? 2 : 0;
> + const long min_unused_cpus = is_preempt_none() ? 2 : 0;
> const long min_required_cpus = 2 + min_unused_cpus;
>
> if (num_online_cpus() < min_required_cpus) {
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Marco Elver @ 2021-11-11 9:36 UTC (permalink / raw)
To: Mike Galbraith
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Nick Desaulniers, linux-kernel, kasan-dev, Ingo Molnar,
Paul Mackerras, Steven Rostedt, Masahiro Yamada, linuxppc-dev,
Valentin Schneider, Dmitry Vyukov
In-Reply-To: <a7febd8825a2ab99bd1999664c6d4aa618b49442.camel@gmx.de>
On Thu, 11 Nov 2021 at 04:47, Mike Galbraith <efault@gmx.de> wrote:
>
> On Thu, 2021-11-11 at 04:35 +0100, Mike Galbraith wrote:
> > On Thu, 2021-11-11 at 04:16 +0100, Mike Galbraith wrote:
> > > On Wed, 2021-11-10 at 20:24 +0000, Valentin Schneider wrote:
> > > >
> > > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > > index 5f8db54226af..0640d5622496 100644
> > > > --- a/include/linux/sched.h
> > > > +++ b/include/linux/sched.h
> > > > @@ -2073,6 +2073,22 @@ static inline void cond_resched_rcu(void)
> > > > #endif
> > > > }
> > > >
> > > > +#ifdef CONFIG_PREEMPT_DYNAMIC
> > > > +
> > > > +extern bool is_preempt_none(void);
> > > > +extern bool is_preempt_voluntary(void);
> > > > +extern bool is_preempt_full(void);
> > > > +
> > > > +#else
> > > > +
> > > > +#define is_preempt_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> > > > +#define is_preempt_voluntary()
> > > > IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> > > > +#define is_preempt_full() IS_ENABLED(CONFIG_PREEMPT)
> > >
> > > I think that should be IS_ENABLED(CONFIG_PREEMPTION), see
> > > c1a280b68d4e.
> > >
> > > Noticed while applying the series to an RT tree, where tglx
> > > has done that replacement to the powerpc spot your next patch
> > > diddles.
> >
> > Damn, then comes patch 5 properly differentiating PREEMPT/PREEMPT_RT.
>
> So I suppose the powerpc spot should remain CONFIG_PREEMPT and become
> CONFIG_PREEMPTION when the RT change gets merged, because that spot is
> about full preemptibility, not a distinct preemption model.
>
> That's rather annoying :-/
I guess the question is if is_preempt_full() should be true also if
is_preempt_rt() is true?
Not sure all cases are happy with that, e.g. the kernel/trace/trace.c
case, which wants to print the precise preemption level.
To avoid confusion, I'd introduce another helper that says true if the
preemption level is "at least full", currently that'd be "full or rt".
Something like is_preempt_full_or_rt() (but might as well write
"is_preempt_full() || is_preempt_rt()"), or is_preemption() (to match
that Kconfig variable, although it's slightly confusing). The
implementation of that helper can just be a static inline function
returning "is_preempt_full() || is_preempt_rt()".
Would that help?
^ permalink raw reply
* Re: [PATCH v2 4/5] kscan: Use preemption model accessors
From: Marco Elver @ 2021-11-11 9:39 UTC (permalink / raw)
To: Valentin Schneider
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Mike Galbraith, Nick Desaulniers, linux-kernel, kasan-dev,
Paul Mackerras, Steven Rostedt, Masahiro Yamada, linuxppc-dev,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <YYzeOQNFmuieCk3T@elver.google.com>
On Thu, 11 Nov 2021 at 10:11, Marco Elver <elver@google.com> wrote:
>
> Subject s/kscan/kcsan/
>
> On Wed, Nov 10, 2021 at 08:24PM +0000, Valentin Schneider wrote:
> > Per PREEMPT_DYNAMIC, checking CONFIG_PREEMPT doesn't tell you the actual
> > preemption model of the live kernel. Use the newly-introduced accessors
> > instead.
> >
> > Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
>
> Reviewed-by: Marco Elver <elver@google.com>
>
> Though it currently doesn't compile as a module due to missing
> EXPORT_SYMBOL of is_preempt*().
>
> > ---
> > kernel/kcsan/kcsan_test.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/kcsan/kcsan_test.c b/kernel/kcsan/kcsan_test.c
> > index dc55fd5a36fc..14d811eb9a21 100644
> > --- a/kernel/kcsan/kcsan_test.c
> > +++ b/kernel/kcsan/kcsan_test.c
> > @@ -1005,13 +1005,13 @@ static const void *nthreads_gen_params(const void *prev, char *desc)
> > else
> > nthreads *= 2;
> >
> > - if (!IS_ENABLED(CONFIG_PREEMPT) || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) {
> > + if (!is_preempt_full() || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) {
In case you introduce the 5th helper I suggested
(is_preempt_full_or_rt() or whatever you'll call it), this one can be
switched, because this check really does want to know if "at least
full preemption" and not "precisely full preemption".
Thanks,
-- Marco
^ permalink raw reply
* Re: [PASEMI] Nemo board doesn't recognize any ATA disks with the pci-v5.16 updates
From: Marc Zyngier @ 2021-11-11 10:20 UTC (permalink / raw)
To: Christian Zigotzky
Cc: axboe, Rob Herring, lorenzo.pieralisi, R.T.Dickinson,
Arnd Bergmann, kw, linux-pci@vger.kernel.org, damien.lemoal,
Olof Johansson, Darren Stevens, Bjorn Helgaas, mad skateman,
bhelgaas@google.com >> Bjorn Helgaas, robert,
Matthew Leaman, linuxppc-dev, Alyssa Rosenzweig,
Christian Zigotzky
In-Reply-To: <f40294c6-a088-af53-eeea-4dfbd255c5c9@xenosoft.de>
On Thu, 11 Nov 2021 07:47:08 +0000,
Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
>
> On 11 November 2021 at 08:13 am, Marc Zyngier wrote:
> > On Thu, 11 Nov 2021 05:24:52 +0000,
> > Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
> >> Hello Marc,
> >>
> >> Here you are:
> >> https://forum.hyperion-entertainment.com/viewtopic.php?p=54406#p54406
> > This is not what I asked. I need the actual source file, or at the
> > very least the compiled object (the /sys/firmware/fdt file, for
> > example). Not an interpretation that I can't feed to the kernel.
> >
> > Without this, I can't debug your problem.
> >
> >> We are very happy to have the patch for reverting the bad commit
> >> because we want to test the new PASEMI i2c driver with support for the
> >> Apple M1 [1] on our Nemo boards.
> > You can revert the patch on your own. At this stage, we're not blindly
> > reverting things in the tree, but instead trying to understand what
> > is happening on your particular system.
> >
> > Thanks,
> >
> > M.
> >
> I added a download link for the fdt file to the post [1]. Please read
> also Darren's comments in this post.
Thanks for that. The DT looks absolutely bonkers, no wonder that
things break with something like that.
But Darren's comments made me jump a bit, and I quote them here for
everyone to see:
<quote>
[...]
The dtb passed by the CFE firmware has a number of issues, which up till
now have been fixed by use of patches applied to the mainline kernel.
This occasionally causes problems with changes made to mainline.
[...]
</quote>
Am I right in understanding that the upstream kernel does not support
the machine out of the box, and that you actually have to apply out of
tree patches to make it work? That these patches have to do with the
IRQ routing?
If so, I wonder why upstream should revert a patch to work on a system
that isn't supported upstream the first place. I will still try and
come up with a solution for you. But asking for the revert of a patch
on these grounds is not, IMHO, acceptable. Also, please provide these
patches on the list so that I can help you to some extend (and I mean
*on the list*, not on a random forum that collects my information).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH v2] powerpc/64s: introduce CONFIG_MAXSMP to test very large SMP
From: Michael Ellerman @ 2021-11-11 10:32 UTC (permalink / raw)
To: Christophe Leroy, Nicholas Piggin, linuxppc-dev
In-Reply-To: <c363e53c-cba5-5711-e8c4-6d74c44f99be@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 09/11/2021 à 07:51, Nicholas Piggin a écrit :
...
>> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
>> index a208997ade88..14c275e0ff93 100644
>> --- a/arch/powerpc/platforms/Kconfig.cputype
>> +++ b/arch/powerpc/platforms/Kconfig.cputype
>> @@ -475,9 +475,14 @@ config SMP
>>
>> If you don't know what to do here, say N.
>>
>> +# MAXSMP sets 8192 if COMPILE_TEST because that's what x86 has flushed out.
>> +# Exceeding that will cause a lot of compile errors. Have to deal with those
>> +# first.
>> config NR_CPUS
>> - int "Maximum number of CPUs (2-8192)" if SMP
>> - range 2 8192 if SMP
>> + int "Maximum number of CPUs (2-8192)" if SMP && !MAXSMP
>> + range 2 16384 if SMP
>> + default 16384 if MAXSMP && !COMPILE_TEST
>> + default 8192 if MAXSMP && COMPILE_TEST
>
> You can do less complex. First hit becomes the default, so you can do:
>
> default 8192 if MAXSMP && COMPILE_TEST
> default 16384 if MAXSMP
I did that when applying.
cheers
^ permalink raw reply
* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Mike Galbraith @ 2021-11-11 10:32 UTC (permalink / raw)
To: Marco Elver
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Nick Desaulniers, linux-kernel, kasan-dev, Ingo Molnar,
Paul Mackerras, Steven Rostedt, Masahiro Yamada, linuxppc-dev,
Valentin Schneider, Dmitry Vyukov
In-Reply-To: <CANpmjNPeRwupeg=S8yGGUracoehSUbS-Fkfb8juv5mYN36uiqg@mail.gmail.com>
On Thu, 2021-11-11 at 10:36 +0100, Marco Elver wrote:
> On Thu, 11 Nov 2021 at 04:47, Mike Galbraith <efault@gmx.de> wrote:
> >
> > On Thu, 2021-11-11 at 04:35 +0100, Mike Galbraith wrote:
> > > On Thu, 2021-11-11 at 04:16 +0100, Mike Galbraith wrote:
> > > > On Wed, 2021-11-10 at 20:24 +0000, Valentin Schneider wrote:
> > > > >
> > > > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > > > index 5f8db54226af..0640d5622496 100644
> > > > > --- a/include/linux/sched.h
> > > > > +++ b/include/linux/sched.h
> > > > > @@ -2073,6 +2073,22 @@ static inline void cond_resched_rcu(void)
> > > > > #endif
> > > > > }
> > > > >
> > > > > +#ifdef CONFIG_PREEMPT_DYNAMIC
> > > > > +
> > > > > +extern bool is_preempt_none(void);
> > > > > +extern bool is_preempt_voluntary(void);
> > > > > +extern bool is_preempt_full(void);
> > > > > +
> > > > > +#else
> > > > > +
> > > > > +#define is_preempt_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
> > > > > +#define is_preempt_voluntary()
> > > > > IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
> > > > > +#define is_preempt_full() IS_ENABLED(CONFIG_PREEMPT)
> > > >
> > > > I think that should be IS_ENABLED(CONFIG_PREEMPTION), see
> > > > c1a280b68d4e.
> > > >
> > > > Noticed while applying the series to an RT tree, where tglx
> > > > has done that replacement to the powerpc spot your next patch
> > > > diddles.
> > >
> > > Damn, then comes patch 5 properly differentiating PREEMPT/PREEMPT_RT.
> >
> > So I suppose the powerpc spot should remain CONFIG_PREEMPT and become
> > CONFIG_PREEMPTION when the RT change gets merged, because that spot is
> > about full preemptibility, not a distinct preemption model.
> >
> > That's rather annoying :-/
>
> I guess the question is if is_preempt_full() should be true also if
> is_preempt_rt() is true?
That's what CONFIG_PREEMPTION is. More could follow, but it was added
to allow multiple models to say "preemptible".
> Not sure all cases are happy with that, e.g. the kernel/trace/trace.c
> case, which wants to print the precise preemption level.
Yeah, that's the "annoying" bit, needing one oddball model accessor
that isn't about a particular model.
> To avoid confusion, I'd introduce another helper that says true if the
> preemption level is "at least full", currently that'd be "full or rt".
> Something like is_preempt_full_or_rt() (but might as well write
> "is_preempt_full() || is_preempt_rt()"), or is_preemption() (to match
> that Kconfig variable, although it's slightly confusing). The
> implementation of that helper can just be a static inline function
> returning "is_preempt_full() || is_preempt_rt()".
>
> Would that help?
Yeah, as it sits two accessors are needed, one that says PREEMPT the
other PREEMPTION, spelling optional.
-Mike
^ permalink raw reply
* Re: [PATCH 11/11] powerpc/smp: Add a doorbell=off kernel parameter
From: Michael Ellerman @ 2021-11-11 10:41 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Cédric Le Goater
In-Reply-To: <20211105102636.1016378-12-clg@kaod.org>
Cédric Le Goater <clg@kaod.org> writes:
> On processors with a XIVE interrupt controller (POWER9 and above), the
> kernel can use either doorbells or XIVE to generate CPU IPIs. Sending
> doorbell is generally preferred to using the XIVE IC because it is
> faster. There are cases where we want to avoid doorbells and use XIVE
> only, for debug or performance. Only useful on POWER9 and above.
How much do we want this?
Kernel command line args are a bit of a pain, they tend to be poorly
tested, because someone has to explicitly enable them at boot time, and
then reboot to test the other case.
When would we want to enable this? Can we make the kernel smarter about
when to use doorbells and make it automated?
Could we make it a runtime switch?
cheers
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> arch/powerpc/include/asm/dbell.h | 1 +
> arch/powerpc/kernel/dbell.c | 17 +++++++++++++++++
> arch/powerpc/platforms/powernv/smp.c | 7 +++++--
> arch/powerpc/platforms/pseries/smp.c | 3 +++
> Documentation/admin-guide/kernel-parameters.txt | 10 ++++++++++
> 5 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/dbell.h b/arch/powerpc/include/asm/dbell.h
> index 3e9da22a2779..07775aa3e81b 100644
> --- a/arch/powerpc/include/asm/dbell.h
> +++ b/arch/powerpc/include/asm/dbell.h
> @@ -90,6 +90,7 @@ static inline void ppc_msgsync(void)
> #endif /* CONFIG_PPC_BOOK3S */
>
> extern void doorbell_exception(struct pt_regs *regs);
> +extern bool doorbell_disabled;
>
> static inline void ppc_msgsnd(enum ppc_dbell type, u32 flags, u32 tag)
> {
> diff --git a/arch/powerpc/kernel/dbell.c b/arch/powerpc/kernel/dbell.c
> index 5545c9cd17c1..681ee4775629 100644
> --- a/arch/powerpc/kernel/dbell.c
> +++ b/arch/powerpc/kernel/dbell.c
> @@ -38,6 +38,23 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(doorbell_exception)
>
> set_irq_regs(old_regs);
> }
> +
> +bool doorbell_disabled;
> +
> +static int __init doorbell_cmdline(char *arg)
> +{
> + if (!arg)
> + return -EINVAL;
> +
> + if (strncmp(arg, "off", 3) == 0) {
> + pr_info("Doorbell disabled on kernel command line\n");
> + doorbell_disabled = true;
> + }
> +
> + return 0;
> +}
> +__setup("doorbell=", doorbell_cmdline);
> +
> #else /* CONFIG_SMP */
> DEFINE_INTERRUPT_HANDLER_ASYNC(doorbell_exception)
> {
> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index cbb67813cd5d..1311bda9446a 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -338,10 +338,13 @@ static void __init pnv_smp_probe(void)
> ic_cause_ipi = smp_ops->cause_ipi;
> WARN_ON(!ic_cause_ipi);
>
> - if (cpu_has_feature(CPU_FTR_ARCH_300))
> + if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> + if (doorbell_disabled)
> + return;
> smp_ops->cause_ipi = doorbell_global_ipi;
> - else
> + } else {
> smp_ops->cause_ipi = pnv_cause_ipi;
> + }
> }
> }
>
> diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
> index f47429323eee..3bc9e6aaf645 100644
> --- a/arch/powerpc/platforms/pseries/smp.c
> +++ b/arch/powerpc/platforms/pseries/smp.c
> @@ -229,6 +229,9 @@ static __init void pSeries_smp_probe(void)
> return;
> }
>
> + if (doorbell_disabled)
> + return;
> +
> /*
> * Under PowerVM, FSCR[MSGP] is enabled as guest vCPU siblings are
> * gang scheduled on the same physical core, so doorbells are always
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 10fa093251e8..2e1284febe39 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1041,6 +1041,16 @@
> The filter can be disabled or changed to another
> driver later using sysfs.
>
> + doorbell=off [PPC]
> + On processors with a XIVE interrupt controller
> + (POWER9 and above), the kernel can use either
> + doorbells or XIVE to generate CPU IPIs. Sending
> + doorbell is generally preferred to using the XIVE
> + IC because it is faster. There are cases where
> + we want to avoid doorbells and use XIVE only,
> + for debug or performance. Only useful on
> + POWER9 and above.
> +
> driver_async_probe= [KNL]
> List of driver names to be probed asynchronously.
> Format: <driver_name1>,<driver_name2>...
> --
> 2.31.1
^ permalink raw reply
* Re: [PASEMI] Nemo board doesn't recognize any ATA disks with the pci-v5.16 updates
From: Christian Zigotzky @ 2021-11-11 10:44 UTC (permalink / raw)
To: Marc Zyngier
Cc: axboe, Rob Herring, lorenzo.pieralisi, R.T.Dickinson,
Arnd Bergmann, kw, linux-pci@vger.kernel.org, damien.lemoal,
Olof Johansson, Darren Stevens, Bjorn Helgaas, mad skateman,
bhelgaas@google.com >> Bjorn Helgaas, robert,
Matthew Leaman, linuxppc-dev, Alyssa Rosenzweig,
Christian Zigotzky
In-Reply-To: <87pmr7803l.wl-maz@kernel.org>
On 11 November 2021 at 11:20 am, Marc Zyngier wrote:
> On Thu, 11 Nov 2021 07:47:08 +0000,
> Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
>> On 11 November 2021 at 08:13 am, Marc Zyngier wrote:
>>> On Thu, 11 Nov 2021 05:24:52 +0000,
>>> Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
>>>> Hello Marc,
>>>>
>>>> Here you are:
>>>> https://forum.hyperion-entertainment.com/viewtopic.php?p=54406#p54406
>>> This is not what I asked. I need the actual source file, or at the
>>> very least the compiled object (the /sys/firmware/fdt file, for
>>> example). Not an interpretation that I can't feed to the kernel.
>>>
>>> Without this, I can't debug your problem.
>>>
>>>> We are very happy to have the patch for reverting the bad commit
>>>> because we want to test the new PASEMI i2c driver with support for the
>>>> Apple M1 [1] on our Nemo boards.
>>> You can revert the patch on your own. At this stage, we're not blindly
>>> reverting things in the tree, but instead trying to understand what
>>> is happening on your particular system.
>>>
>>> Thanks,
>>>
>>> M.
>>>
>> I added a download link for the fdt file to the post [1]. Please read
>> also Darren's comments in this post.
>
> Am I right in understanding that the upstream kernel does not support
> the machine out of the box, and that you actually have to apply out of
> tree patches to make it work?
No, you aren't right. The upstream kernel supports the Nemo board out of
the box. [1]
- Christian
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=grep&q=Darren+Stevens
^ permalink raw reply
* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Valentin Schneider @ 2021-11-11 10:56 UTC (permalink / raw)
To: Marco Elver
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Mike Galbraith, Nick Desaulniers, linux-kernel, kasan-dev,
Paul Mackerras, Steven Rostedt, Masahiro Yamada, linuxppc-dev,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <YYzaLTtf1tFbqDSn@elver.google.com>
On 11/11/21 09:54, Marco Elver wrote:
> On Wed, Nov 10, 2021 at 08:24PM +0000, Valentin Schneider wrote:
> [...]
>> +#ifdef CONFIG_PREEMPT_DYNAMIC
>> +
>> +extern bool is_preempt_none(void);
>> +extern bool is_preempt_voluntary(void);
>> +extern bool is_preempt_full(void);
>> +
>> +#else
>> +
>> +#define is_preempt_none() IS_ENABLED(CONFIG_PREEMPT_NONE)
>> +#define is_preempt_voluntary() IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY)
>> +#define is_preempt_full() IS_ENABLED(CONFIG_PREEMPT)
>> +
>> +#endif
>> +
>> +#define is_preempt_rt() IS_ENABLED(CONFIG_PREEMPT_RT)
>> +
>
> Can these callables be real functions in all configs, making the
> !DYNAMIC ones just static inline bool ones? That'd catch invalid use in
> #if etc. in all configs.
>
Ack
>> /*
>> * Does a critical section need to be broken due to another
>> * task waiting?: (technically does not depend on CONFIG_PREEMPTION,
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 97047aa7b6c2..9db7f77e53c3 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -6638,6 +6638,17 @@ static void __init preempt_dynamic_init(void)
>> }
>> }
>>
>> +#define PREEMPT_MODE_ACCESSOR(mode) \
>> + bool is_preempt_##mode(void) \
>> + { \
>> + WARN_ON_ONCE(preempt_dynamic_mode == preempt_dynamic_undefined); \
>> + return preempt_dynamic_mode == preempt_dynamic_##mode; \
>> + }
>
> This needs an EXPORT_SYMBOL, so it's usable from modules like the
> kcsan_test module.
Ah, wasn't sure about that one, thanks!
^ permalink raw reply
* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Valentin Schneider @ 2021-11-11 10:56 UTC (permalink / raw)
To: Mike Galbraith, Marco Elver
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Nick Desaulniers, linux-kernel, kasan-dev, Paul Mackerras,
Steven Rostedt, Masahiro Yamada, linuxppc-dev, Ingo Molnar,
Dmitry Vyukov
In-Reply-To: <26fd47db11763a9c79662a66eed2dbdbcbedaa8a.camel@gmx.de>
On 11/11/21 11:32, Mike Galbraith wrote:
> On Thu, 2021-11-11 at 10:36 +0100, Marco Elver wrote:
>> I guess the question is if is_preempt_full() should be true also if
>> is_preempt_rt() is true?
>
> That's what CONFIG_PREEMPTION is. More could follow, but it was added
> to allow multiple models to say "preemptible".
>
That's what I was gonna say, but you can have CONFIG_PREEMPTION while being
is_preempt_none() due to PREEMPT_DYNAMIC...
>> Not sure all cases are happy with that, e.g. the kernel/trace/trace.c
>> case, which wants to print the precise preemption level.
>
> Yeah, that's the "annoying" bit, needing one oddball model accessor
> that isn't about a particular model.
>
>> To avoid confusion, I'd introduce another helper that says true if the
>> preemption level is "at least full", currently that'd be "full or rt".
>> Something like is_preempt_full_or_rt() (but might as well write
>> "is_preempt_full() || is_preempt_rt()"), or is_preemption() (to match
>> that Kconfig variable, although it's slightly confusing). The
>> implementation of that helper can just be a static inline function
>> returning "is_preempt_full() || is_preempt_rt()".
>>
>> Would that help?
>
> Yeah, as it sits two accessors are needed, one that says PREEMPT the
> other PREEMPTION, spelling optional.
>
Per the above, I think we need the full || rt thingie.
> -Mike
^ permalink raw reply
* Re: [PATCH v2 4/5] kscan: Use preemption model accessors
From: Valentin Schneider @ 2021-11-11 10:57 UTC (permalink / raw)
To: Marco Elver
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Mike Galbraith, Nick Desaulniers, linux-kernel, kasan-dev,
Paul Mackerras, Steven Rostedt, Masahiro Yamada, linuxppc-dev,
Ingo Molnar, Dmitry Vyukov
In-Reply-To: <YYzeOQNFmuieCk3T@elver.google.com>
On 11/11/21 10:11, Marco Elver wrote:
> Subject s/kscan/kcsan/
>
Woops...
> On Wed, Nov 10, 2021 at 08:24PM +0000, Valentin Schneider wrote:
>> Per PREEMPT_DYNAMIC, checking CONFIG_PREEMPT doesn't tell you the actual
>> preemption model of the live kernel. Use the newly-introduced accessors
>> instead.
>>
>> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
>
> Reviewed-by: Marco Elver <elver@google.com>
>
> Though it currently doesn't compile as a module due to missing
> EXPORT_SYMBOL of is_preempt*().
>
>> ---
>> kernel/kcsan/kcsan_test.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/kcsan/kcsan_test.c b/kernel/kcsan/kcsan_test.c
>> index dc55fd5a36fc..14d811eb9a21 100644
>> --- a/kernel/kcsan/kcsan_test.c
>> +++ b/kernel/kcsan/kcsan_test.c
>> @@ -1005,13 +1005,13 @@ static const void *nthreads_gen_params(const void *prev, char *desc)
>> else
>> nthreads *= 2;
>>
>> - if (!IS_ENABLED(CONFIG_PREEMPT) || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) {
>> + if (!is_preempt_full() || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) {
>> /*
>> * Without any preemption, keep 2 CPUs free for other tasks, one
>> * of which is the main test case function checking for
>> * completion or failure.
>> */
>> - const long min_unused_cpus = IS_ENABLED(CONFIG_PREEMPT_NONE) ? 2 : 0;
>> + const long min_unused_cpus = is_preempt_none() ? 2 : 0;
>> const long min_required_cpus = 2 + min_unused_cpus;
>>
>> if (num_online_cpus() < min_required_cpus) {
>> --
>> 2.25.1
^ permalink raw reply
* Re: [PATCH v2 2/5] preempt/dynamic: Introduce preempt mode accessors
From: Mike Galbraith @ 2021-11-11 11:09 UTC (permalink / raw)
To: Valentin Schneider, Marco Elver
Cc: Michal Marek, linux-kbuild, Peter Zijlstra, Frederic Weisbecker,
Nick Desaulniers, linux-kernel, kasan-dev, Paul Mackerras,
Steven Rostedt, Masahiro Yamada, linuxppc-dev, Ingo Molnar,
Dmitry Vyukov
In-Reply-To: <8735o3rmej.mognet@arm.com>
On Thu, 2021-11-11 at 10:56 +0000, Valentin Schneider wrote:
> On 11/11/21 11:32, Mike Galbraith wrote:
> > On Thu, 2021-11-11 at 10:36 +0100, Marco Elver wrote:
> > > I guess the question is if is_preempt_full() should be true also if
> > > is_preempt_rt() is true?
> >
> > That's what CONFIG_PREEMPTION is. More could follow, but it was added
> > to allow multiple models to say "preemptible".
> >
>
> That's what I was gonna say, but you can have CONFIG_PREEMPTION while being
> is_preempt_none() due to PREEMPT_DYNAMIC...
Ah, right.. this is gonna take some getting used to.
-Mike
^ permalink raw reply
* Re: [PASEMI] Nemo board doesn't recognize any ATA disks with the pci-v5.16 updates
From: Marc Zyngier @ 2021-11-11 11:24 UTC (permalink / raw)
To: Christian Zigotzky
Cc: axboe, Rob Herring, lorenzo.pieralisi, R.T.Dickinson,
Arnd Bergmann, kw, linux-pci@vger.kernel.org, damien.lemoal,
Olof Johansson, Darren Stevens, Bjorn Helgaas, mad skateman,
bhelgaas@google.com >> Bjorn Helgaas, robert,
Matthew Leaman, linuxppc-dev, Alyssa Rosenzweig,
Christian Zigotzky
In-Reply-To: <c93c7f72-6e46-797b-bee3-c9ae3b444f60@xenosoft.de>
On Thu, 11 Nov 2021 10:44:30 +0000,
Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
>
> On 11 November 2021 at 11:20 am, Marc Zyngier wrote:
> > On Thu, 11 Nov 2021 07:47:08 +0000,
> > Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
> >> On 11 November 2021 at 08:13 am, Marc Zyngier wrote:
> >>> On Thu, 11 Nov 2021 05:24:52 +0000,
> >>> Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
> >>>> Hello Marc,
> >>>>
> >>>> Here you are:
> >>>> https://forum.hyperion-entertainment.com/viewtopic.php?p=54406#p54406
> >>> This is not what I asked. I need the actual source file, or at the
> >>> very least the compiled object (the /sys/firmware/fdt file, for
> >>> example). Not an interpretation that I can't feed to the kernel.
> >>>
> >>> Without this, I can't debug your problem.
> >>>
> >>>> We are very happy to have the patch for reverting the bad commit
> >>>> because we want to test the new PASEMI i2c driver with support for the
> >>>> Apple M1 [1] on our Nemo boards.
> >>> You can revert the patch on your own. At this stage, we're not blindly
> >>> reverting things in the tree, but instead trying to understand what
> >>> is happening on your particular system.
> >>>
> >>> Thanks,
> >>>
> >>> M.
> >>>
> >> I added a download link for the fdt file to the post [1]. Please read
> >> also Darren's comments in this post.
> >
> > Am I right in understanding that the upstream kernel does not support
> > the machine out of the box, and that you actually have to apply out of
> > tree patches to make it work?
> No, you aren't right. The upstream kernel supports the Nemo board out
> of the box. [1]
That's not the way I interpret Darren's comments, but you surely know
better than I do.
I'll try to come up with something for you to test shortly.
M.
--
Without deviation from the norm, progress is not possible.
^ 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