* Re: [v2] powerpc/perf: Fix the kernel address leak to userspace via SDAR
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <1521632426-30770-3-git-send-email-maddy@linux.vnet.ibm.com>
On Wed, 2018-03-21 at 11:40:26 UTC, Madhavan Srinivasan wrote:
> Sampled Data Address Register (SDAR) is a 64-bit
> register that contains the effective address of
> the storage operand of an instruction that was
> being executed, possibly out-of-order, at or around
> the time that the Performance Monitor alert occurred.
>
> In certain scenario SDAR happen to contain the kernel
> address even for userspace only sampling. Add checks
> to prevent it.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/cd1231d7035fea894118d5155ff984
cheers
^ permalink raw reply
* Re: [v2] powerpc/perf: Fix kernel address leak to userspace via BHRB buffer
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <1521632426-30770-2-git-send-email-maddy@linux.vnet.ibm.com>
On Wed, 2018-03-21 at 11:40:25 UTC, Madhavan Srinivasan wrote:
> The current Branch History Rolling Buffer (BHRB) code does
> not check for any privilege levels before updating the data
> from BHRB. This leaks kernel addresses to userspace even when
> profiling only with userspace privileges. Add proper checks
> to prevent it.
>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/bb19af816025d495376bd76bf6fbcf
cheers
^ permalink raw reply
* Re: [v2] powerpc/perf: Fix kernel address leaks via Sampling registers
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <1521632426-30770-1-git-send-email-maddy@linux.vnet.ibm.com>
On Wed, 2018-03-21 at 11:40:24 UTC, Madhavan Srinivasan wrote:
> From: Michael Ellerman <mpe@ellerman.id.au>
>
> Current code in power_pmu_disable() does not clear the sampling
> registers like Sampling Instruction Address Register (SAIR) and
> Sampling Data Address Register (SDAR) after disabling the PMU.
> Since these are userspace readable and could contain kernel
> address, add code to explicitly clear the content of these registers.
> Patch also adds a "context synchronizing instruction" to enforce
> no further updates to these registers as mandated by PowerISA.
>
> "If an mtspr instruction is executed that changes the
> value of a Performance Monitor register other than
> SIAR, SDAR, and SIER, the change is not guaranteed
> to have taken effect until after a subsequent context
> synchronizing instruction has been executed (see
> Chapter 11. "Synchronization Requirements for Con-
> text Alterations" on page 1133)."
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/e1ebd0e5b9d0a10ba65e63a3514b6d
cheers
^ permalink raw reply
* Re: [v2,1/9] powerpc/eeh: Remove eeh_handle_event()
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <d3d01073da02a5eb844cc8998fdac602b8121b62.1521427331.git.sam.bobroff@au1.ibm.com>
On Mon, 2018-03-19 at 02:46:20 UTC, Sam Bobroff wrote:
> The function eeh_handle_event(pe) does nothing other than switching
> between calling eeh_handle_normal_event(pe) and
> eeh_handle_special_event(). However it is only called in two places,
> one where pe can't be NULL and the other where it must be NULL (see
> eeh_event_handler()) so it does nothing but obscure the flow of
> control.
>
> So, remove it.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/68701780712f7ddb2fa81032aa1b4a
cheers
^ permalink raw reply
* Re: [v3, 1/5] rfi-flush: Move the logic to avoid a redo into the debugfs code
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Mauricio Faria de Oliveira, linuxppc-dev, msuchanek
In-Reply-To: <1521067243-19520-2-git-send-email-mauricfo@linux.vnet.ibm.com>
On Wed, 2018-03-14 at 22:40:38 UTC, Mauricio Faria de Oliveira wrote:
> From: Michael Ellerman <mpe@ellerman.id.au>
>
> rfi_flush_enable() includes a check to see if we're already
> enabled (or disabled), and in that case does nothing.
>
> But that means calling setup_rfi_flush() a 2nd time doesn't actually
> work, which is a bit confusing.
>
> Move that check into the debugfs code, where it really belongs.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/1e2a9fc7496955faacbbed49461d61
cheers
^ permalink raw reply
* Re: powerpc/mm: Fix section mismatch warning in stop_machine_change_mapping()
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Mauricio Faria de Oliveira, linuxppc-dev; +Cc: aneesh.kumar, npiggin
In-Reply-To: <1520628358-1762-1-git-send-email-mauricfo@linux.vnet.ibm.com>
On Fri, 2018-03-09 at 20:45:58 UTC, Mauricio Faria de Oliveira wrote:
> Fix the warning messages for stop_machine_change_mapping(), and a number
> of other affected functions in its call chain.
>
> All modified functions are under CONFIG_MEMORY_HOTPLUG, so __meminit
> is okay (keeps them / does not discard them).
>
> Boot-tested on powernv/power9/radix-mmu and pseries/power8/hash-mmu.
>
> $ make -j$(nproc) CONFIG_DEBUG_SECTION_MISMATCH=y vmlinux
> ...
> MODPOST vmlinux.o
> WARNING: vmlinux.o(.text+0x6b130): Section mismatch in reference from the function stop_machine_change_mapping() to the function .meminit.text:create_physical_mapping()
> The function stop_machine_change_mapping() references
> the function __meminit create_physical_mapping().
> This is often because stop_machine_change_mapping lacks a __meminit
> annotation or the annotation of create_physical_mapping is wrong.
>
> WARNING: vmlinux.o(.text+0x6b13c): Section mismatch in reference from the function stop_machine_change_mapping() to the function .meminit.text:create_physical_mapping()
> The function stop_machine_change_mapping() references
> the function __meminit create_physical_mapping().
> This is often because stop_machine_change_mapping lacks a __meminit
> annotation or the annotation of create_physical_mapping is wrong.
> ...
>
> Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/bde709a70884bfc790da6fbc4467c9
cheers
^ permalink raw reply
* Re: [1/3] powerpc/perf: Infrastructure to support addition of blacklisted events
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <1520164588-19167-1-git-send-email-maddy@linux.vnet.ibm.com>
On Sun, 2018-03-04 at 11:56:26 UTC, Madhavan Srinivasan wrote:
> Introduce code to support addition of blacklisted events for a
> processor version. A 'pointer' and 'int' variable to hold the
> number of events are added to 'struct power_pmu', along with a
> generic function to loop through the list to validate the given
> event. Generic function 'is_event_blacklisted' is called in
> power_pmu_event_init() to detect and reject early.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/b58064da046243f0c988afd939997e
cheers
^ permalink raw reply
* Re: [kernel] powerpc/npu: Do not try invalidating 32bit table when 64bit table is enabled
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: Alexey Kardashevskiy, Alistair Popple, David Gibson
In-Reply-To: <20180213055135.25639-1-aik@ozlabs.ru>
On Tue, 2018-02-13 at 05:51:35 UTC, Alexey Kardashevskiy wrote:
> GPUs and the corresponding NVLink bridges get different PEs as they have
> separate translation validation entries (TVEs). We put these PEs to
> the same IOMMU group so they cannot be passed through separately.
> So the iommu_table_group_ops::set_window/unset_window for GPUs do set
> tables to the NPU PEs as well which means that iommu_table's list of
> attached PEs (iommu_table_group_link) has both GPU and NPU PEs linked.
> This list is used for TCE cache invalidation.
>
> The problem is that NPU PE has just a single TVE and can be programmed
> to point to 32bit or 64bit windows while GPU PE has two (as any other PCI
> device). So we end up having an 32bit iommu_table struct linked to both
> PEs even though only the 64bit TCE table cache can be invalidated on NPU.
> And a relatively recent skiboot detects this and prints errors.
>
> This changes GPU's iommu_table_group_ops::set_window/unset_window to make
> sure that NPU PE is only linked to the table actually used by the hardware.
> If there are two tables used by an IOMMU group, the NPU PE will use
> the last programmed one which with the current use scenarios is expected
> to be a 64bit one.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/d41ce7b1bcc3e1d02cc9da3b83c0fe
cheers
^ permalink raw reply
* Re: [kernel] powerpc/mm: Fix typo in comments
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy
In-Reply-To: <20180201050725.30247-1-aik@ozlabs.ru>
On Thu, 2018-02-01 at 05:07:25 UTC, Alexey Kardashevskiy wrote:
> Fixes: 912cc87a6 "powerpc/mm/radix: Add LPID based tlb flush helpers"
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/b574df94883df4d37f1b9d648867d6
cheers
^ permalink raw reply
* Re: [kernel] powerpc/lpar/debug: Initialize flags before printing debug message
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy
In-Reply-To: <20180109055214.32199-1-aik@ozlabs.ru>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 411 bytes --]
On Tue, 2018-01-09 at 05:52:14 UTC, Alexey Kardashevskiy wrote:
> With enabled DEBUG, there is a compile error:
> "error: ‘flags’ is used uninitialized in this function".
>
> This moves pr_devel() little further where @flags are initialized.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/a8c0bf3c621e0acc01451e27fe47c4
cheers
^ permalink raw reply
* Re: [kernel] powerpc/init: Do not advertise radix during client-architecture-support
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy
In-Reply-To: <20180109054520.31704-1-aik@ozlabs.ru>
On Tue, 2018-01-09 at 05:45:20 UTC, Alexey Kardashevskiy wrote:
> Currently the pseries kernel advertises radix MMU support even if
> the actual support is disabled via the CONFIG_PPC_RADIX_MMU option.
>
> This adds a check for CONFIG_PPC_RADIX_MMU to avoid advertising radix
> to the hypervisor.
>
> Suggested-by: Paul Mackerras <paulus@ozlabs.org>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/79b4686857029cdea97d0102d179ae
cheers
^ permalink raw reply
* Re: powerpc/64: Call H_REGISTER_PROC_TBL when running as a HPT guest on POWER9
From: Michael Ellerman @ 2018-03-28 14:13 UTC (permalink / raw)
To: Paul Mackerras, linuxppc-dev; +Cc: Michael Ellerman
In-Reply-To: <20170216050339.GC19110@fergus.ozlabs.ibm.com>
On Thu, 2017-02-16 at 05:03:39 UTC, Paul Mackerras wrote:
> On POWER9, since commit cc3d2940133d ("powerpc/64: Enable use of radix
> MMU under hypervisor on POWER9", 2017-01-30), we set both the radix and
> HPT bits in the client-architecture-support (CAS) vector, which tells
> the hypervisor that we can do either radix or HPT. According to PAPR,
> if we use this combination we are promising to do a H_REGISTER_PROC_TBL
> hcall later on to let the hypervisor know whether we are doing radix
> or HPT. We currently do this call if we are doing radix but not if
> we are doing HPT. If the hypervisor is able to support both radix
> and HPT guests, it would be entitled to defer allocation of the HPT
> until the H_REGISTER_PROC_TBL call, and to fail any attempts to create
> HPTEs until the H_REGISTER_PROC_TBL call. Thus we need to do a
> H_REGISTER_PROC_TBL call when we are doing HPT; otherwise we may
> crash at boot time.
>
> This adds the code to call H_REGISTER_PROC_TBL in this case, before
> we attempt to create any HPT entries using H_ENTER.
>
> Fixes: cc3d2940133d ("powerpc/64: Enable use of radix MMU under hypervisor on POWER9")
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/dbfcf3cb9c681aa0c5d0bb46068f98
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Fix smp_wmb barrier definition use use lwsync consistently
From: Nicholas Piggin @ 2018-03-28 13:43 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <874ll02x22.fsf@concordia.ellerman.id.au>
On Wed, 28 Mar 2018 23:40:05 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > asm/barrier.h is not always included after asm/synch.h, which meant
> > it was missing __SUBARCH_HAS_LWSYNC, so in some files smp_wmb() would
> > be eieio when it should be lwsync. kernel/time/hrtimer.c is one case.
>
> Wow nice catch. Only broken since 2008 presumably.
>
> Some days I think maybe we aren't very good at this writing software
> thing, good to have some certainty :)
Yeah, I only caught it by luck when looking through instruction traces.
The pipeline model just happens to make eieio look different to most
other instructions (which is likely a bug in the model) which made me
look closer at it. Could have been with us for another 10 years.
> > __SUBARCH_HAS_LWSYNC is only used in one place, so just fold it in
> > to where it's used. Previously with my small simulator config, 377
> > instances of eieio in the tree. After this patch there are 55.
>
> At least for Book3S this isn't actually a terrible bug AFAICS:
>
> - smp_wmb() is only defined to order accesses to cacheable memory.
> - smp_wmb() only orders prior stores vs later stores.
> - eieio orders all prior stores vs all later stores for cacheable
> memory.
> - lwsync orders everything except prior stores vs later loads for
> cacheable memory.
>
> So eieio and lwsync are both valid to use as smp_wmb(), but it's still
> terrible fishy that we were using both in different places depending on
> include ordering.
Oh yeah it's not a bug in that it would cause violation of memory
ordering, only performance (and expectations when debugging and
observing things I guess). eieio works fine for smp_wmb().
> I'm inclined to tag this for stable unless anyone can think of a reason
> not to?
I think that would be good.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 2/2] smp: introduce kick_active_cpus_sync()
From: Yury Norov @ 2018-03-28 13:36 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Chris Metcalf, Christopher Lameter, Russell King - ARM Linux,
Mark Rutland, Steven Rostedt, Mathieu Desnoyers, Catalin Marinas,
Will Deacon, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, linux-arm-kernel, linuxppc-dev, kvm-ppc,
linux-mm, linux-kernel, luto
In-Reply-To: <20180326124555.GJ3675@linux.vnet.ibm.com>
On Mon, Mar 26, 2018 at 05:45:55AM -0700, Paul E. McKenney wrote:
> On Sun, Mar 25, 2018 at 11:11:54PM +0300, Yury Norov wrote:
> > On Sun, Mar 25, 2018 at 12:23:28PM -0700, Paul E. McKenney wrote:
> > > On Sun, Mar 25, 2018 at 08:50:04PM +0300, Yury Norov wrote:
> > > > kick_all_cpus_sync() forces all CPUs to sync caches by sending broadcast IPI.
> > > > If CPU is in extended quiescent state (idle task or nohz_full userspace), this
> > > > work may be done at the exit of this state. Delaying synchronization helps to
> > > > save power if CPU is in idle state and decrease latency for real-time tasks.
> > > >
> > > > This patch introduces kick_active_cpus_sync() and uses it in mm/slab and arm64
> > > > code to delay syncronization.
> > > >
> > > > For task isolation (https://lkml.org/lkml/2017/11/3/589), IPI to the CPU running
> > > > isolated task would be fatal, as it breaks isolation. The approach with delaying
> > > > of synchronization work helps to maintain isolated state.
> > > >
> > > > I've tested it with test from task isolation series on ThunderX2 for more than
> > > > 10 hours (10k giga-ticks) without breaking isolation.
> > > >
> > > > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> > > > ---
> > > > arch/arm64/kernel/insn.c | 2 +-
> > > > include/linux/smp.h | 2 ++
> > > > kernel/smp.c | 24 ++++++++++++++++++++++++
> > > > mm/slab.c | 2 +-
> > > > 4 files changed, 28 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> > > > index 2718a77da165..9d7c492e920e 100644
> > > > --- a/arch/arm64/kernel/insn.c
> > > > +++ b/arch/arm64/kernel/insn.c
> > > > @@ -291,7 +291,7 @@ int __kprobes aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt)
> > > > * synchronization.
> > > > */
> > > > ret = aarch64_insn_patch_text_nosync(addrs[0], insns[0]);
> > > > - kick_all_cpus_sync();
> > > > + kick_active_cpus_sync();
> > > > return ret;
> > > > }
> > > > }
> > > > diff --git a/include/linux/smp.h b/include/linux/smp.h
> > > > index 9fb239e12b82..27215e22240d 100644
> > > > --- a/include/linux/smp.h
> > > > +++ b/include/linux/smp.h
> > > > @@ -105,6 +105,7 @@ int smp_call_function_any(const struct cpumask *mask,
> > > > smp_call_func_t func, void *info, int wait);
> > > >
> > > > void kick_all_cpus_sync(void);
> > > > +void kick_active_cpus_sync(void);
> > > > void wake_up_all_idle_cpus(void);
> > > >
> > > > /*
> > > > @@ -161,6 +162,7 @@ smp_call_function_any(const struct cpumask *mask, smp_call_func_t func,
> > > > }
> > > >
> > > > static inline void kick_all_cpus_sync(void) { }
> > > > +static inline void kick_active_cpus_sync(void) { }
> > > > static inline void wake_up_all_idle_cpus(void) { }
> > > >
> > > > #ifdef CONFIG_UP_LATE_INIT
> > > > diff --git a/kernel/smp.c b/kernel/smp.c
> > > > index 084c8b3a2681..0358d6673850 100644
> > > > --- a/kernel/smp.c
> > > > +++ b/kernel/smp.c
> > > > @@ -724,6 +724,30 @@ void kick_all_cpus_sync(void)
> > > > }
> > > > EXPORT_SYMBOL_GPL(kick_all_cpus_sync);
> > > >
> > > > +/**
> > > > + * kick_active_cpus_sync - Force CPUs that are not in extended
> > > > + * quiescent state (idle or nohz_full userspace) sync by sending
> > > > + * IPI. Extended quiescent state CPUs will sync at the exit of
> > > > + * that state.
> > > > + */
> > > > +void kick_active_cpus_sync(void)
> > > > +{
> > > > + int cpu;
> > > > + struct cpumask kernel_cpus;
> > > > +
> > > > + smp_mb();
> > > > +
> > > > + cpumask_clear(&kernel_cpus);
> > > > + preempt_disable();
> > > > + for_each_online_cpu(cpu) {
> > > > + if (!rcu_eqs_special_set(cpu))
> > >
> > > If we get here, the CPU is not in a quiescent state, so we therefore
> > > must IPI it, correct?
> > >
> > > But don't you also need to define rcu_eqs_special_exit() so that RCU
> > > can invoke it when it next leaves its quiescent state? Or are you able
> > > to ignore the CPU in that case? (If you are able to ignore the CPU in
> > > that case, I could give you a lower-cost function to get your job done.)
> > >
> > > Thanx, Paul
> >
> > What's actually needed for synchronization is issuing memory barrier on target
> > CPUs before we start executing kernel code.
> >
> > smp_mb() is implicitly called in smp_call_function*() path for it. In
> > rcu_eqs_special_set() -> rcu_dynticks_eqs_exit() path, smp_mb__after_atomic()
> > is called just before rcu_eqs_special_exit().
> >
> > So I think, rcu_eqs_special_exit() may be left untouched. Empty
> > rcu_eqs_special_exit() in new RCU path corresponds empty do_nothing() in old
> > IPI path.
> >
> > Or my understanding of smp_mb__after_atomic() is wrong? By default,
> > smp_mb__after_atomic() is just alias to smp_mb(). But some
> > architectures define it differently. x86, for example, aliases it to
> > just barrier() with a comment: "Atomic operations are already
> > serializing on x86".
> >
> > I was initially thinking that it's also fine to leave
> > rcu_eqs_special_exit() empty in this case, but now I'm not sure...
> >
> > Anyway, answering to your question, we shouldn't ignore quiescent
> > CPUs, and rcu_eqs_special_set() path is really needed as it issues
> > memory barrier on them.
>
> An alternative approach would be for me to make something like this
> and export it:
>
> bool rcu_cpu_in_eqs(int cpu)
> {
> struct rcu_dynticks *rdtp = &per_cpu(rcu_dynticks, cpu);
> int snap;
>
> smp_mb(); /* Obtain consistent snapshot, pairs with update. */
> snap = READ_ONCE(&rdtp->dynticks);
> smp_mb(); /* See above. */
> return !(snap & RCU_DYNTICK_CTRL_CTR);
> }
>
> Then you could replace your use of rcu_cpu_in_eqs() above with
Did you mean replace rcu_eqs_special_set()?
> the new rcu_cpu_in_eqs(). This would avoid the RMW atomic, and, more
> important, the unnecessary write to ->dynticks.
>
> Or am I missing something?
>
> Thanx, Paul
This will not work because EQS CPUs will not be charged to call
smp_mb() on exit of EQS.
Lets sync our understanding of IPI and RCU mechanisms.
Traditional IPI scheme looks like this:
CPU1: CPU2:
touch shared resource(); /* running any code */
smp_mb();
smp_call_function(); ---> handle_IPI()
{
/* Make resource visible */
smp_mb();
do_nothing();
}
And new RCU scheme for eqs CPUs looks like this:
CPU1: CPU2:
touch shared resource(); /* Running EQS */
smp_mb();
if (RCU_DYNTICK_CTRL_CTR)
set(RCU_DYNTICK_CTRL_MASK); /* Still in EQS */
/* And later */
rcu_dynticks_eqs_exit()
{
if (RCU_DYNTICK_CTRL_MASK) {
/* Make resource visible */
smp_mb();
rcu_eqs_special_exit();
}
}
Is it correct?
Yury
^ permalink raw reply
* Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key
From: Laurent Dufour @ 2018-03-28 13:30 UTC (permalink / raw)
To: David Rientjes
Cc: kernel test robot, paulmck, peterz, akpm, kirill, ak, mhocko,
dave, jack, Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner,
Ingo Molnar, hpa, Will Deacon, Sergey Senozhatsky,
Andrea Arcangeli, Alexei Starovoitov, kemi.wang,
sergey.senozhatsky.work, Daniel Jordan, linux-kernel, linux-mm,
haren, khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev,
x86, lkp
In-Reply-To: <alpine.DEB.2.20.1803251510040.80485@chino.kir.corp.google.com>
On 26/03/2018 00:10, David Rientjes wrote:
> On Wed, 21 Mar 2018, Laurent Dufour wrote:
>
>> I found the root cause of this lockdep warning.
>>
>> In mmap_region(), unmap_region() may be called while vma_link() has not been
>> called. This happens during the error path if call_mmap() failed.
>>
>> The only to fix that particular case is to call
>> seqcount_init(&vma->vm_sequence) when initializing the vma in mmap_region().
>>
>
> Ack, although that would require a fixup to dup_mmap() as well.
You're right, I'll fix that too.
Thanks a lot.
Laurent.
^ permalink raw reply
* Re: [PATCH 2/2] smp: introduce kick_active_cpus_sync()
From: Yury Norov @ 2018-03-28 12:59 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andrea Parri, Paul E. McKenney, Chris Metcalf,
Christopher Lameter, Russell King - ARM Linux, Mark Rutland,
Mathieu Desnoyers, Catalin Marinas, Will Deacon, Pekka Enberg,
David Rientjes, Joonsoo Kim, Andrew Morton,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linux-arm-kernel, linuxppc-dev, kvm-ppc, linux-mm, linux-kernel
In-Reply-To: <20180326145735.57ba306b@gandalf.local.home>
On Mon, Mar 26, 2018 at 02:57:35PM -0400, Steven Rostedt wrote:
> On Mon, 26 Mar 2018 10:53:13 +0200
> Andrea Parri <andrea.parri@amarulasolutions.com> wrote:
>
> > > --- a/kernel/smp.c
> > > +++ b/kernel/smp.c
> > > @@ -724,6 +724,30 @@ void kick_all_cpus_sync(void)
> > > }
> > > EXPORT_SYMBOL_GPL(kick_all_cpus_sync);
> > >
> > > +/**
> > > + * kick_active_cpus_sync - Force CPUs that are not in extended
> > > + * quiescent state (idle or nohz_full userspace) sync by sending
> > > + * IPI. Extended quiescent state CPUs will sync at the exit of
> > > + * that state.
> > > + */
> > > +void kick_active_cpus_sync(void)
> > > +{
> > > + int cpu;
> > > + struct cpumask kernel_cpus;
> > > +
> > > + smp_mb();
> >
> > (A general remark only:)
> >
> > checkpatch.pl should have warned about the fact that this barrier is
> > missing an accompanying comment (which accesses are being "ordered",
> > what is the pairing barrier, etc.).
>
> He could have simply copied the comment above the smp_mb() for
> kick_all_cpus_sync():
>
> /* Make sure the change is visible before we kick the cpus */
>
> The kick itself is pretty much a synchronization primitive.
>
> That is, you make some changes and then you need all CPUs to see it,
> and you call: kick_active_cpus_synch(), which is the barrier to make
> sure you previous changes are seen on all CPUS before you proceed
> further. Note, the matching barrier is implicit in the IPI itself.
>
> -- Steve
I know that I had to copy the comment from kick_all_cpus_sync(), but I
don't like copy-pasting in general, and as Steven told, this smp_mb() is
already inside synchronization routine, so we may hope that users of
kick_*_cpus_sync() will explain better what for they need it...
>
> >
> > Moreover if, as your reply above suggested, your patch is relying on
> > "implicit barriers" (something I would not recommend) then even more
> > so you should comment on these requirements.
> >
> > This could: (a) force you to reason about the memory ordering stuff,
> > (b) easy the task of reviewing and adopting your patch, (c) easy the
> > task of preserving those requirements (as implementations changes).
> >
> > Andrea
I need v2 anyway, and I will add comments to address all questions in this
thread.
I also hope that we'll agree that for powerpc it's also safe to delay
synchronization, and if so, we will have no users of kick_all_cpus_sync(),
and can drop it.
(It looks like this, because nohz_full userspace CPU cannot have pending
IPIs, but I'd like to get confirmation from powerpc people.)
Would it make sense to rename kick_all_cpus_sync() to smp_mb_sync(), which
would stand for 'synchronous memory barrier on all online CPUs'?
Yury
^ permalink raw reply
* [GIT PULL] Please pull powerpc/linux.git powerpc-4.16-6 tag
From: Michael Ellerman @ 2018-03-28 12:48 UTC (permalink / raw)
To: Linus Torvalds; +Cc: aneesh.kumar, benh, linux-kernel, linuxppc-dev, npiggin
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hi Linus,
Please pull some more powerpc fixes for 4.16. Apologies if this is a bit
big at rc7, but they're all reasonably important fixes. None are
actually for new code, so they aren't indicative of 4.16 being in bad
shape from our point of view.
cheers
The following changes since commit b0c41b8b6e43120d7c35e4709508a3d90a09646e:
powerpc/pseries: Fix vector5 in ibm architecture vector table (2018-03-06 23:05:38 +1100)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-4.16-6
for you to fetch changes up to 52396500f97c53860164debc7d4f759077853423:
powerpc/64s: Fix i-side SLB miss bad address handler saving nonvolatile GPRs (2018-03-26 07:40:17 +1100)
- ----------------------------------------------------------------
powerpc fixes for 4.16 #6
These are actually all fixes for pre-4.16 code, or new hardware workarounds.
Fix missing AT_BASE_PLATFORM (in auxv) when we're using a new firmware interface
for describing CPU features.
Fix lost pending interrupts due to a race in our interrupt soft-masking code.
A workaround for a nest MMU bug with TLB invalidations on Power9.
A workaround for broadcast TLB invalidations on Power9.
Fix a bug in our instruction SLB miss handler, when handling bad addresses
(eg. >= TASK_SIZE), which could corrupt non-volatile user GPRs.
Thanks to:
Aneesh Kumar K.V, Balbir Singh, Benjamin Herrenschmidt, Nicholas Piggin.
- ----------------------------------------------------------------
Aneesh Kumar K.V (3):
powerpc/mm/radix: Remove unused code
powerpc/mm/radix: Move the functions that does the actual tlbie closer
powerpc/mm: Fixup tlbie vs store ordering issue on POWER9
Benjamin Herrenschmidt (2):
powerpc/mm: Add tracking of the number of coprocessors using a context
powerpc/mm: Workaround Nest MMU bug with TLB invalidations
Michael Ellerman (1):
powerpc/64s: Fix NULL AT_BASE_PLATFORM when using DT CPU features
Nicholas Piggin (2):
powerpc/64s: Fix lost pending interrupt due to race causing lost update to irq_happened
powerpc/64s: Fix i-side SLB miss bad address handler saving nonvolatile GPRs
arch/powerpc/include/asm/book3s/64/mmu.h | 3 +
.../powerpc/include/asm/book3s/64/tlbflush-radix.h | 3 -
arch/powerpc/include/asm/cputable.h | 3 +-
arch/powerpc/include/asm/mmu_context.h | 18 ++-
arch/powerpc/kernel/dt_cpu_ftrs.c | 6 +
arch/powerpc/kernel/exceptions-64s.S | 2 +-
arch/powerpc/kernel/irq.c | 8 +
arch/powerpc/kvm/book3s_64_mmu_radix.c | 3 +
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 11 ++
arch/powerpc/mm/hash_native_64.c | 16 +-
arch/powerpc/mm/mmu_context_book3s64.c | 1 +
arch/powerpc/mm/pgtable_64.c | 1 +
arch/powerpc/mm/tlb-radix.c | 169 +++++++++++----------
13 files changed, 154 insertions(+), 90 deletions(-)
-----BEGIN PGP SIGNATURE-----
iQIcBAEBCAAGBQJau480AAoJEFHr6jzI4aWAYwIP/RBX30EIQegM7GUwEYulVVhG
E16yFb+QLvUrh3YXU9Oqyb0tj1y/gLJcPojin28TWBpt+wGejriCoSIOPjoI915E
W8jnnxrR8u/nZfzARNh+zsR6+qbNVz3MyVSAsn9BrS2jjWqiekXuvF7WLr34M22N
6xiu2BJ7B4cjvNJa2AFj1GYLCFeed4BDhERn/bNaK3jhdp3hYpnpaxG6y+od5M2J
GdQRO+GhCfBzho/ypLAX62klnprAVYCh+qnDKOOHGpE0qz0WOTHnZ3d5VVJO4sAe
zCdadR66h6YvukrGvTLmJfz1ykZv1vbrr+WbgN0kt+8EsYAqRtO1OB1wm0hjbQYK
e5hA+PccnVkxqjv0b/dg81OS/3PCA7/4xNtBRFxc4YNO3JGebU87gMYepaJAIt0A
zCySqOYOIZMznfuk9310XYY7Z+LnpnRxRLTNZFq5tvaI/O9I/5P3a5f1IUwvkJk3
PCY/EyzLuyIac8FFcMQh5UIJLqebAVVAnNC75YTZG576gf0NmJYO/jwxPz5Yep48
oO1er783SX4vojcYfEVyp+mA4KGbVKel615L1pvgX4CH9tXM4PhX5y/ODZjjP9ua
XksvQdaUlakHIBJwNboFS70gaBHoivN1OFEUINf3XtOhwZRTo51ufilONM8XZhoX
r8o+pDgD/5QCyJqMhlcr
=VKmI
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH] powerpc: Fix smp_wmb barrier definition use use lwsync consistently
From: Michael Ellerman @ 2018-03-28 12:40 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Anton Blanchard, Nicholas Piggin
In-Reply-To: <20180322104146.5350-1-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> asm/barrier.h is not always included after asm/synch.h, which meant
> it was missing __SUBARCH_HAS_LWSYNC, so in some files smp_wmb() would
> be eieio when it should be lwsync. kernel/time/hrtimer.c is one case.
Wow nice catch. Only broken since 2008 presumably.
Some days I think maybe we aren't very good at this writing software
thing, good to have some certainty :)
> __SUBARCH_HAS_LWSYNC is only used in one place, so just fold it in
> to where it's used. Previously with my small simulator config, 377
> instances of eieio in the tree. After this patch there are 55.
At least for Book3S this isn't actually a terrible bug AFAICS:
- smp_wmb() is only defined to order accesses to cacheable memory.
- smp_wmb() only orders prior stores vs later stores.
- eieio orders all prior stores vs all later stores for cacheable
memory.
- lwsync orders everything except prior stores vs later loads for
cacheable memory.
So eieio and lwsync are both valid to use as smp_wmb(), but it's still
terrible fishy that we were using both in different places depending on
include ordering.
I'm inclined to tag this for stable unless anyone can think of a reason
not to?
cheers
> diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
> index 10daa1d56e0a..c7c63959ba91 100644
> --- a/arch/powerpc/include/asm/barrier.h
> +++ b/arch/powerpc/include/asm/barrier.h
> @@ -35,7 +35,8 @@
> #define rmb() __asm__ __volatile__ ("sync" : : : "memory")
> #define wmb() __asm__ __volatile__ ("sync" : : : "memory")
>
> -#ifdef __SUBARCH_HAS_LWSYNC
> +/* The sub-arch has lwsync */
> +#if defined(__powerpc64__) || defined(CONFIG_PPC_E500MC)
> # define SMPWMB LWSYNC
> #else
> # define SMPWMB eieio
> diff --git a/arch/powerpc/include/asm/synch.h b/arch/powerpc/include/asm/synch.h
> index 63e7f5a1f105..6ec546090ba1 100644
> --- a/arch/powerpc/include/asm/synch.h
> +++ b/arch/powerpc/include/asm/synch.h
> @@ -6,10 +6,6 @@
> #include <linux/stringify.h>
> #include <asm/feature-fixups.h>
>
> -#if defined(__powerpc64__) || defined(CONFIG_PPC_E500MC)
> -#define __SUBARCH_HAS_LWSYNC
> -#endif
> -
> #ifndef __ASSEMBLY__
> extern unsigned int __start___lwsync_fixup, __stop___lwsync_fixup;
> extern void do_lwsync_fixups(unsigned long value, void *fixup_start,
> --
> 2.16.1
^ permalink raw reply
* Re: Linux 4.16: Reported regressions as of Tuesday, 2018-03-27 (Was: Linux 4.16-rc7)
From: David Sterba @ 2018-03-28 12:32 UTC (permalink / raw)
To: Thorsten Leemhuis
Cc: Linus Torvalds, Linux Kernel Mailing List, linuxppc-dev,
Jonathan Corbet
In-Reply-To: <d1f5b976-f293-5767-9faf-c4f569f8ba55@leemhuis.info>
On Tue, Mar 27, 2018 at 09:13:32PM +0200, Thorsten Leemhuis wrote:
> On 26.03.2018 01:37, Linus Torvalds wrote:
> > […] Anyway. Go out and test. And let's hope next week is nice and calm and
> > I can release the final 4.16 next Sunday without any extra rc's.
> >
> > Linus
>
> Hi! Find below my sixth regression report for Linux 4.16. It lists 7
> regressions I'm currently aware of. 2 were fixed since last weeks
> report; 2 are new.
>
> Are you aware of any other regressions that got introduced this
> development cycle? Then please let me know by mail (a simple bounce or
> forward to the sender of this email address is enough!). And please tell
> me if there is anything in the report that shouldn't be there.
>
> Ciao, Thorsten
>
> == Current regressions ==
>
> Dell R640 does not boot due to SCSI/SATA failure
> - Status: Afaics still unfixed; lost track, ask reporter for an update
> on Monday morning, no reply yet
> - Cause: https://git.kernel.org/torvalds/c/84676c1f21e8
> - Reported: 2018-02-22
> https://marc.info/?l=linux-kernel&m=151931128006031
> - Note: Issue understood and even (kind of accidentally) fixed by a
> patch series that was proposed for 4.17 (see links)
> - Last known developer activity: 2018-03-14
> https://marc.info/?l=linux-block&m=152102086831636&w=2
> - Other relevant links:
> https://marc.info/?l=linux-block&m=152051511802229&w=2
> https://marc.info/?l=linux-kernel&m=152026091325037
I'm not the original reporter but I can confirm this regression has been
fixed by git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git
scsi-fixes (tag 0c30612d535460af7fe16e7e6ed8b97defe4adbe)
The machine has a HP array and stopped booting some time around rc1. I
tried to bisect it and in the end it was the commit 84676c1f21e8 that I
reverted for testing builds, so I'm sure it's the same issue as you've
been tracking.
^ permalink raw reply
* Re: [PATCH 01/16] initrd: Add generic code path for common initrd unloading logic.
From: Geert Uytterhoeven @ 2018-03-28 12:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Shea Levy, alpha, Linux Kernel Mailing List, arcml, linux-c6x-dev,
moderated list:H8/300 ARCHITECTURE, linux-m68k, nios2-dev,
Openrisc, Parisc List, linuxppc-dev, linux-riscv, Linux-sh list,
uml-devel
In-Reply-To: <20180328120440.GB1838@infradead.org>
On Wed, Mar 28, 2018 at 2:04 PM, Christoph Hellwig <hch@infradead.org> wrote:
>> +#ifdef CONFIG_INITRAMFS_GENERIC_UNLOAD
>> +void free_initrd_mem(unsigned long start, unsigned long end)
>> +{
>> + free_reserved_area((void *)start, (void *)end, -1, "initrd");
>> +}
>> +#endif
>
> Given how trivial this is and how many architectures can use it I'd
> reverse the polarity and add a CONFIG_HAVE_ARCH_FREE_INITRD_MEM
> instead.
And while adding "special" functionality to the generic version, more and more
users of CONFIG_HAVE_ARCH_FREE_INITRD_MEM will be removed.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 01/16] initrd: Add generic code path for common initrd unloading logic.
From: Christoph Hellwig @ 2018-03-28 12:04 UTC (permalink / raw)
To: Shea Levy
Cc: linux-alpha, linux-kernel, linux-snps-arc, linux-c6x-dev,
uclinux-h8-devel, linux-m68k, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-sh, user-mode-linux-devel
In-Reply-To: <20180324174458.26423-2-shea@shealevy.com>
> +#ifdef CONFIG_INITRAMFS_GENERIC_UNLOAD
> +void free_initrd_mem(unsigned long start, unsigned long end)
> +{
> + free_reserved_area((void *)start, (void *)end, -1, "initrd");
> +}
> +#endif
Given how trivial this is and how many architectures can use it I'd
reverse the polarity and add a CONFIG_HAVE_ARCH_FREE_INITRD_MEM
instead.
^ permalink raw reply
* Re: [PATCH 14/19] powerpc/altivec: Add missing prototypes for altivec
From: Mathieu Malaterre @ 2018-03-28 11:53 UTC (permalink / raw)
To: LEROY Christophe; +Cc: LKML, Paul Mackerras, linuxppc-dev
In-Reply-To: <CA+7wUsz-ihu7aoE4UfVhojDjH0pcR8QbVGLCzMyA3RocLJz5-A@mail.gmail.com>
On Wed, Mar 28, 2018 at 9:26 AM, Mathieu Malaterre <malat@debian.org> wrote=
:
> On Tue, Mar 27, 2018 at 7:33 PM, LEROY Christophe
> <christophe.leroy@c-s.fr> wrote:
>> LEROY Christophe <christophe.leroy@c-s.fr> a =C3=A9crit :
>>
>>
>>> Mathieu Malaterre <malat@debian.org> a =C3=A9crit :
>>>
>>>> Christophe,
>>>>
>>>> On Sat, Mar 24, 2018 at 9:10 PM, LEROY Christophe
>>>> <christophe.leroy@c-s.fr> wrote:
>>>>>
>>>>> Mathieu Malaterre <malat@debian.org> a =C3=A9crit :
>>>>>
>>>>>
>>>>>> On Fri, Mar 23, 2018 at 1:19 PM, christophe leroy
>>>>>> <christophe.leroy@c-s.fr> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Le 22/03/2018 =C3=A0 21:20, Mathieu Malaterre a =C3=A9crit :
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Some functions prototypes were missing for the non-altivec code. A=
dd
>>>>>>>> the
>>>>>>>> missing prototypes directly in xor_vmx, fix warnings treated as
>>>>>>>> errors
>>>>>>>> with
>>>>>>>> W=3D1:
>>>>>>>>
>>>>>>>> arch/powerpc/lib/xor_vmx_glue.c:18:6: error: no previous prototy=
pe
>>>>>>>> for
>>>>>>>> =E2=80=98xor_altivec_2=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>>>>> arch/powerpc/lib/xor_vmx_glue.c:29:6: error: no previous prototy=
pe
>>>>>>>> for
>>>>>>>> =E2=80=98xor_altivec_3=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>>>>> arch/powerpc/lib/xor_vmx_glue.c:40:6: error: no previous prototy=
pe
>>>>>>>> for
>>>>>>>> =E2=80=98xor_altivec_4=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>>>>> arch/powerpc/lib/xor_vmx_glue.c:52:6: error: no previous prototy=
pe
>>>>>>>> for
>>>>>>>> =E2=80=98xor_altivec_5=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>>>>>
>>>>>>>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>>>>>>>> ---
>>>>>>>> arch/powerpc/lib/xor_vmx.h | 14 ++++++++++++++
>>>>>>>> 1 file changed, 14 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/powerpc/lib/xor_vmx.h b/arch/powerpc/lib/xor_vmx=
.h
>>>>>>>> index 5c2b0839b179..2173e3c84151 100644
>>>>>>>> --- a/arch/powerpc/lib/xor_vmx.h
>>>>>>>> +++ b/arch/powerpc/lib/xor_vmx.h
>>>>>>>> @@ -19,3 +19,17 @@ void __xor_altivec_4(unsigned long bytes, unsig=
ned
>>>>>>>> long
>>>>>>>> *v1_in,
>>>>>>>> void __xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
>>>>>>>> unsigned long *v2_in, unsigned long
>>>>>>>> *v3_in,
>>>>>>>> unsigned long *v4_in, unsigned long
>>>>>>>> *v5_in);
>>>>>>>> +
>>>>>>>> +void xor_altivec_2(unsigned long bytes, unsigned long *v1_in,
>>>>>>>> + unsigned long *v2_in);
>>>>>>>> +
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Only used in one place, should be static instead of adding it in a =
.h
>>>>>>>
>>>>>>> Same for the other ones.
>>>>>>
>>>>>>
>>>>>>
>>>>>> $ git grep xor_altivec_2
>>>>>> [...]
>>>>>> arch/powerpc/lib/xor_vmx_glue.c:EXPORT_SYMBOL(xor_altivec_2);
>>>>>>
>>>>>> Are you sure I can change this function to static ?
>>>>>
>>>>>
>>>>>
>>>>> Yes you are right. But in fact those fonctions are already defined i=
n
>>>>> asm/xor. h
>>>>> So you just need to add the missing #include
>>>>
>>>>
>>>> I originally tried it, but this leads to:
>>>>
>>>> CC arch/powerpc/lib/xor_vmx_glue.o
>>>> In file included from arch/powerpc/lib/xor_vmx_glue.c:16:0:
>>>> ./arch/powerpc/include/asm/xor.h:39:15: error: variable
>>>> =E2=80=98xor_block_altivec=E2=80=99 has initializer but incomplete typ=
e
>>>> static struct xor_block_template xor_block_altivec =3D {
>>>> ^~~~~~~~~~~~~~~~~~
>>>> ./arch/powerpc/include/asm/xor.h:40:2: error: unknown field =E2=80=98n=
ame=E2=80=99
>>>> specified in initializer
>>>> .name =3D "altivec",
>>>> ^
>>>> [...]
>>>>
>>>> The file <asm/xor.h> (powerpc) is pretty much expected to be included
>>>> after <include/linux/raid/xor.h>.
>>>>
>>>> I did not want to tweak <asm/xor.h> to test for #ifdef _XOR_H just bef=
ore
>>>>
>>>> #ifdef _XOR_H
>>>> static struct xor_block_template xor_block_altivec =3D {
>>>> [...]
>>>>
>>>> since this seems like a hack to me.
>>>>
>>>> Is this ok to test for #ifdef _XOR_H in <arch/powerpc/include/asm/xor.=
h>
>>>> ?
>>>
>>>
>>> What about including linux/raid/xor.h in asm/xor.h ?
>
> This leads to:
>
> CALL ../arch/powerpc/kernel/systbl_chk.sh
> In file included from ../arch/powerpc/include/asm/xor.h:57:0,
> from ../arch/powerpc/lib/xor_vmx_glue.c:17:
> ../include/asm-generic/xor.h:688:34: error: =E2=80=98xor_block_32regs=E2=
=80=99 defined
> but not used [-Werror=3Dunused-variable]
> static struct xor_block_template xor_block_32regs =3D {
> ^~~~~~~~~~~~~~~~
> ../include/asm-generic/xor.h:680:34: error: =E2=80=98xor_block_8regs=E2=
=80=99 defined
> but not used [-Werror=3Dunused-variable]
> static struct xor_block_template xor_block_8regs =3D {
> ^~~~~~~~~~~~~~~
> In file included from ../arch/powerpc/lib/xor_vmx_glue.c:17:0:
> ../arch/powerpc/include/asm/xor.h:39:34: error: =E2=80=98xor_block_altive=
c=E2=80=99
> defined but not used [-Werror=3Dunused-variable]
> static struct xor_block_template xor_block_altivec =3D {
> ^~~~~~~~~~~~~~~~~
> CALL ../arch/powerpc/kernel/prom_init_check.sh
>
I'll prepare a patch which moves the prototypes from
arch/powerpc/include/asm/xor.h to
arch/powerpc/include/asm/xor_altivec.h (new file). I believe this
should be the simplest path.
>>
>> Or better: including linux/raid/xor.h then asm/xor.h in xor_vmx_glue.c ?
>>
>> Christophe
>>
>>>
>>> Christophe
>>>>
>>>>
>>>>> Christophe
>>>>>
>>>>>
>>>>>>
>>>>>>> Christophe
>>>>>>>
>>>>>>>
>>>>>>>> +void xor_altivec_3(unsigned long bytes, unsigned long *v1_in,
>>>>>>>> + unsigned long *v2_in, unsigned long
>>>>>>>> *v3_in);
>>>>>>>> +
>>>>>>>> +void xor_altivec_4(unsigned long bytes, unsigned long *v1_in,
>>>>>>>> + unsigned long *v2_in, unsigned long
>>>>>>>> *v3_in,
>>>>>>>> + unsigned long *v4_in);
>>>>>>>> +
>>>>>>>> +void xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
>>>>>>>> + unsigned long *v2_in, unsigned long
>>>>>>>> *v3_in,
>>>>>>>> + unsigned long *v4_in, unsigned long
>>>>>>>> *v5_in);
>>>>>>>>
>>>>>>>
>>>>>>> ---
>>>>>>> L'absence de virus dans ce courrier =C3=A9lectronique a =C3=A9t=C3=
=A9 v=C3=A9rifi=C3=A9e par le
>>>>>>> logiciel antivirus Avast.
>>>>>>> https://www.avast.com/antivirus
>>>>>>>
>>>>>
>>>>>
>>
>>
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: okaya @ 2018-03-28 11:41 UTC (permalink / raw)
To: Linus Torvalds
Cc: Benjamin Herrenschmidt, Alexander Duyck, Will Deacon,
Arnd Bergmann, Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-rdma,
Alexander Duyck, Paul E. McKenney, netdev, linus971
In-Reply-To: <CA+55aFxULkhZGhg-tTBBxBBvXbL7QV7f6zvwW7U2wHFijqGEqw@mail.gmail.com>
On 2018-03-28 02:14, Linus Torvalds wrote:
> On Tue, Mar 27, 2018 at 5:24 PM, Sinan Kaya <okaya@codeaurora.org>
> wrote:
>>
>> Basically changing it to
>>
>> dma_buffer->foo = 1; /* WB */
>> wmb()
>> writel_relaxed(KICK, DMA_KICK_REGISTER); /* UC */
>> mmiowb()
>
> Why?
>
> Why not just remove the wmb(), and keep the barrier in the writel()?
Yes, we want to get there indeed. It is because of some arch not
implementing writel properly. Maintainers want to play safe.
That is why I asked if IA64 and other well known archs follow the
strongly ordered rule at this moment like PPC and ARM.
Or should we go and inform every arch about this before yanking wmb()?
Maintainers are afraid of introducing a regression.
>
> The above code makes no sense, and just looks stupid to me. It also
> generates pointlessly bad code on x86, so it's bad there too.
>
> Linus
^ permalink raw reply
* RE: RFC on writel and writel_relaxed
From: David Laight @ 2018-03-28 11:30 UTC (permalink / raw)
To: 'Benjamin Herrenschmidt', Will Deacon
Cc: Linus Torvalds, Alexander Duyck, Sinan Kaya, Arnd Bergmann,
Jason Gunthorpe, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Paul E. McKenney,
netdev@vger.kernel.org
In-Reply-To: <1522230988.21446.7.camel@kernel.crashing.org>
RnJvbTogQmVuamFtaW4gSGVycmVuc2NobWlkdA0KPiBTZW50OiAyOCBNYXJjaCAyMDE4IDEwOjU2
DQouLi4NCj4gRm9yIGV4YW1wbGUsIGxldCdzIHNheSBJIGhhdmUgYSBkZXZpY2Ugd2l0aCBhIHJl
c2V0IGJpdCBhbmQgdGhlIHNwZWMNCj4gc2F5cyB0aGUgcmVzZXQgYml0IG5lZWRzIHRvIGJlIHNl
dCBmb3IgYXQgbGVhc3QgMTB1cy4NCj4gDQo+IFRoaXMgaXMgd3Jvbmc6DQo+IA0KPiAJd3JpdGVs
KDEsIFJFU0VUX1JFRyk7DQo+IAl1c2xlZXAoMTApOw0KPiAJd3JpdGVsKDAsIFJFU0VUX1JFRyk7
DQo+IA0KPiBCZWNhdXNlIG9mIHdyaXRlIHBvc3RpbmcsIHRoZSBmaXJzdCB3cml0ZSBtaWdodCBh
cnJpdmUgdG8gdGhlIGRldmljZQ0KPiByaWdodCBiZWZvcmUgdGhlIHNlY29uZCBvbmUuDQo+IA0K
PiBUaGUgdHlwaWNhbCAiZml4IiBpcyB0byB0dXJuIHRoYXQgaW50bzoNCj4gDQo+IAl3cml0ZWwo
MSwgUkVTRVRfUkVHKTsNCj4gCXJlYWRsKFJFU0VUX1JFRyk7IC8qIEZsdXNoIHBvc3RlZCB3cml0
ZXMgKi8NCg0KV291bGQgYSB3cml0ZWwoMSwgUkVTRVRfUkVHKSBoZXJlIHByb3ZpZGUgZW5vdWdo
IHN5bmNocm9uc2lhdGlvbj8NCg0KPiAJdXNsZWVwKDEwKTsNCj4gCXdyaXRlbCgwLCBSRVNFVF9S
RUcpOw0KPiANCj4gKkhvd2V2ZXIqIHRoZSBpc3N1ZSBoZXJlLCBhdCBsZWFzdCBvbiBwb3dlciwg
aXMgdGhhdCB0aGUgQ1BVIGNhbiBpc3N1ZQ0KPiB0aGF0IHJlYWRsIGJ1dCBkb2Vzbid0IG5lY2Vz
c2FyaWx5IHdhaXQgZm9yIGl0IHRvIGNvbXBsZXRlIChpZSwgdGhlDQo+IGRhdGEgdG8gcmV0dXJu
KSwgYmVmb3JlIHByb2NlZWRpbmcgdG8gdGhlIHVzbGVlcC4gTm93IGEgdXNsZWVwIGNvbnRhaW5z
DQo+IGEgYnVuY2ggb2YgbG9hZHMgYW5kIHN0b3JlcyBhbmQgaXMgcHJvYmFibHkgZmluZSwgYnV0
IGEgdWRlbGF5IHdoaWNoDQo+IGp1c3QgbG9vcHMgb24gdGhlIHRpbWViYXNlIG1heSBub3QgYmUu
DQo+IA0KPiBUaHVzIHdlIG1heSBzdGlsbCB2aW9sYXRlIHRoZSB0aW1pbmcgcmVxdWlyZW1lbnQu
DQoNCkkndmUgc2VlbSB0aGF0IHNvcnQgb2YgY29kZSAod2l0aCB1ZGVsYXkoKSBhbmQgbm8gcmVh
ZCBiYWNrKSBxdWl0ZSBvZnRlbi4NCkhvdyBtYW55IHdlcmUgaW4gbGludXggSSBkb24ndCBrbm93
Lg0KDQpGb3Igc21hbGwgZGVsYXlzIEkgdXN1YWxseSBmaXggaXQgYnkgcmVwZWF0ZWQgd3JpdGVz
IChvZiB0aGUgc2FtZSB2YWx1ZSkNCnRvIHRoZSBkZXZpY2UgcmVnaXN0ZXIuIFRoYXQgY2FuIGd1
YXJhbnRlZSB2ZXJ5IHNob3J0IGludGVydmFscy4NCg0KVGhlIG9ubHkgdGltZSBJJ3ZlIGFjdHVh
bGx5IHNlZW4gYnVmZmVyZWQgd3JpdGVzIGJyZWFrIHRpbWluZyB3YXMNCmJldHdlZW4gYSAyODYg
YW5kIGFuIDg4NTkgaW50ZXJydXB0IGNvbnRyb2xsZXIuDQpJZiB5b3Ugd3JvdGUgdG8gdGhlIG1h
c2sgdGhlbiBlbmFibGVkIGludGVycnVwdHMgdGhlIGZpcnN0IElBQ0sgY3ljbGUNCmNvdWxkIGJl
IHRvbyBjbG9zZSB0byB3cml0ZSBhbmQgYnJlYWsgdGhlIGN5Y2xlIHJlY292ZXJ5IHRpbWUuDQpU
aGF0IGNsb2JiZXJlZCBtb3N0IG9mIHRoZSBpbnRlcnJ1cHQgY29udHJvbGxlciByZWdpc3RlcnMu
DQpUaGF0IHByb2JhYmx5IGFmZmVjdGVkIGV2ZXJ5IDI4NiBib2FyZCBldmVyIGJ1aWx0IQ0KTm90
IHN1cmUgaG93IG11Y2ggc29mdHdhcmUgYWRkZWQgdGhlIHJlcXVpcmVkIGV4dHJhIGJ1cyBjeWNs
ZS4NCg0KPiBXaGF0IHdlIGRpZCBpbnNpZGUgcmVhZGwsIHdpdGggdGhlIHR3aTtpc3luYyBzZXF1
ZW5jZSAod2hpY2ggYmFzaWNhbGx5DQo+IG1lYW5zLCB0cmFwIG9uIHJldHVybiB2YWx1ZSB3aXRo
ICJ0cmFwIG5ldmVyIiBhcyBhIGNvbmRpdGlvbiwgZm9sbG93ZWQNCj4gYnkgaXN5bmMgdGhhdCBl
bnN1cmVzIGFsbCBleGNwZXRpb24gY29uZGl0aW9ucyBhcmUgcmVzb2x2ZWQpLCBpcyBmb3JjZQ0K
PiB0aGUgQ1BVIHRvICJjb25zdW1lIiB0aGUgZGF0YSBmcm9tIHRoZSByZWFkIGJlZm9yZSBtb3Zp
bmcgb24uDQo+IA0KPiBUaGlzIGVmZmVjdGl2ZWx5IG1ha2VzIHJlYWRsIGZ1bGx5IHN5bmNocm9u
b3VzICh3ZSB3b3VsZCBwcm9iYWJseSBhdm9pZA0KPiB0aGF0IGlmIHdlIHdlcmUgdG8gaW1wbGVt
ZW50IGEgcmVhZGxfcmVsYXhlZCkuDQoNCkkndmUgYWx3YXlzIHdvbmRlcmVkIGV4YWN0bHkgd2hh
dCB0aGUgdHdpO2lzeW5jIHdlcmUgZm9yIC0gYWx3YXlzIHNlZW1lZA0KdmVyeSBoZWF2eSBoYW5k
ZWQgZm9yIG1vc3QgbW1pbyByZWFkcy4NClBhcnRpY3VsYXJseSBpZiB5b3UgYXJlIGRvaW5nIG1t
aW8gcmVhZHMgZnJvbSBhIGRhdGEgZmlmby4NCg0KUGVyaGFwcyB0aGVyZSBzaG91bGQgYmUgYSB3
cml0ZWxfd2FpdCgpIHRoYXQgaXMgYWxsb3dlZCB0byBkbyBhIHJlYWQgYmFjaw0KZm9yIHN1Y2gg
Y29kZSBwYXRocz8NCg0KCURhdmlkDQoNCg==
^ permalink raw reply
* Re: [PATCH v9 01/24] mm: Introduce CONFIG_SPECULATIVE_PAGE_FAULT
From: Laurent Dufour @ 2018-03-28 11:15 UTC (permalink / raw)
To: David Rientjes
Cc: Thomas Gleixner, paulmck, peterz, akpm, kirill, ak, mhocko, dave,
jack, Matthew Wilcox, benh, mpe, paulus, Ingo Molnar, hpa,
Will Deacon, Sergey Senozhatsky, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work,
Daniel Jordan, linux-kernel, linux-mm, haren, khandual, npiggin,
bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <alpine.DEB.2.20.1803280310380.68839@chino.kir.corp.google.com>
On 28/03/2018 12:16, David Rientjes wrote:
> On Wed, 28 Mar 2018, Laurent Dufour wrote:
>
>>>> This configuration variable will be used to build the code needed to
>>>> handle speculative page fault.
>>>>
>>>> By default it is turned off, and activated depending on architecture
>>>> support.
>>>>
>>>> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
>>>> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
>>>> ---
>>>> mm/Kconfig | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/mm/Kconfig b/mm/Kconfig
>>>> index abefa573bcd8..07c566c88faf 100644
>>>> --- a/mm/Kconfig
>>>> +++ b/mm/Kconfig
>>>> @@ -759,3 +759,6 @@ config GUP_BENCHMARK
>>>> performance of get_user_pages_fast().
>>>>
>>>> See tools/testing/selftests/vm/gup_benchmark.c
>>>> +
>>>> +config SPECULATIVE_PAGE_FAULT
>>>> + bool
>>>
>>> Should this be configurable even if the arch supports it?
>>
>> Actually, this is not configurable unless by manually editing the .config file.
>>
>> I made it this way on the Thomas's request :
>> https://lkml.org/lkml/2018/1/15/969
>>
>> That sounds to be the smarter way to achieve that, isn't it ?
>>
>
> Putting this in mm/Kconfig is definitely the right way to go about it
> instead of any generic option in arch/*.
>
> My question, though, was making this configurable by the user:
>
> config SPECULATIVE_PAGE_FAULT
> bool "Speculative page faults"
> depends on X86_64 || PPC
> default y
> help
> ..
>
> It's a question about whether we want this always enabled on x86_64 and
> power or whether the user should be able to disable it (right now they
> can't). With a large feature like this, you may want to offer something
> simple (disable CONFIG_SPECULATIVE_PAGE_FAULT) if someone runs into
> regressions.
I agree, but I think it would be important to get the per architecture
enablement to avoid complex check here. For instance in the case of powerPC
this is only supported for PPC_BOOK3S_64.
To avoid exposing such per architecture define here, what do you think about
having supporting architectures setting ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT
and the SPECULATIVE_PAGE_FAULT depends on this, like this:
In mm/Kconfig:
config SPECULATIVE_PAGE_FAULT
bool "Speculative page faults"
depends on ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT && SMP
default y
help
...
In arch/powerpc/Kconfig:
config PPC
...
select ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT if PPC_BOOK3S_64
In arch/x86/Kconfig:
config X86_64
...
select ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT
^ 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