* Re: powerpc 5.10-rcN boot failures with RCU_SCALE_TEST=m
From: Uladzislau Rezki @ 2020-12-03 14:34 UTC (permalink / raw)
To: Michael Ellerman
Cc: rcu, Uladzislau Rezki, linuxppc-dev, Paul E . McKenney,
Daniel Axtens
In-Reply-To: <87sg8nv277.fsf@mpe.ellerman.id.au>
On Thu, Dec 03, 2020 at 05:22:20PM +1100, Michael Ellerman wrote:
> Uladzislau Rezki <urezki@gmail.com> writes:
> > On Thu, Dec 03, 2020 at 01:03:32AM +1100, Michael Ellerman wrote:
> ...
> >>
> >> The SMP bringup stalls because _cpu_up() is blocked trying to take
> >> cpu_hotplug_lock for writing:
> >>
> >> [ 401.403132][ T0] task:swapper/0 state:D stack:12512 pid: 1 ppid: 0 flags:0x00000800
> >> [ 401.403502][ T0] Call Trace:
> >> [ 401.403907][ T0] [c0000000062c37d0] [c0000000062c3830] 0xc0000000062c3830 (unreliable)
> >> [ 401.404068][ T0] [c0000000062c39b0] [c000000000019d70] __switch_to+0x2e0/0x4a0
> >> [ 401.404189][ T0] [c0000000062c3a10] [c000000000b87228] __schedule+0x288/0x9b0
> >> [ 401.404257][ T0] [c0000000062c3ad0] [c000000000b879b8] schedule+0x68/0x120
> >> [ 401.404324][ T0] [c0000000062c3b00] [c000000000184ad4] percpu_down_write+0x164/0x170
> >> [ 401.404390][ T0] [c0000000062c3b50] [c000000000116b68] _cpu_up+0x68/0x280
> >> [ 401.404475][ T0] [c0000000062c3bb0] [c000000000116e70] cpu_up+0xf0/0x140
> >> [ 401.404546][ T0] [c0000000062c3c30] [c00000000011776c] bringup_nonboot_cpus+0xac/0xf0
> >> [ 401.404643][ T0] [c0000000062c3c80] [c000000000eea1b8] smp_init+0x40/0xcc
> >> [ 401.404727][ T0] [c0000000062c3ce0] [c000000000ec43dc] kernel_init_freeable+0x1e0/0x3a0
> >> [ 401.404799][ T0] [c0000000062c3db0] [c000000000011ec4] kernel_init+0x24/0x150
> >> [ 401.404958][ T0] [c0000000062c3e20] [c00000000000daf0] ret_from_kernel_thread+0x5c/0x6c
> >>
> >> It can't get it because kprobe_optimizer() has taken it for read and is now
> >> blocked waiting for synchronize_rcu_tasks():
> >>
> >> [ 401.418808][ T0] task:kworker/0:1 state:D stack:13392 pid: 12 ppid: 2 flags:0x00000800
> >> [ 401.418951][ T0] Workqueue: events kprobe_optimizer
> >> [ 401.419078][ T0] Call Trace:
> >> [ 401.419121][ T0] [c0000000062ef650] [c0000000062ef710] 0xc0000000062ef710 (unreliable)
> >> [ 401.419213][ T0] [c0000000062ef830] [c000000000019d70] __switch_to+0x2e0/0x4a0
> >> [ 401.419281][ T0] [c0000000062ef890] [c000000000b87228] __schedule+0x288/0x9b0
> >> [ 401.419347][ T0] [c0000000062ef950] [c000000000b879b8] schedule+0x68/0x120
> >> [ 401.419415][ T0] [c0000000062ef980] [c000000000b8e664] schedule_timeout+0x2a4/0x340
> >> [ 401.419484][ T0] [c0000000062efa80] [c000000000b894ec] wait_for_completion+0x9c/0x170
> >> [ 401.419552][ T0] [c0000000062efae0] [c0000000001ac85c] __wait_rcu_gp+0x19c/0x210
> >> [ 401.419619][ T0] [c0000000062efb40] [c0000000001ac90c] synchronize_rcu_tasks_generic+0x3c/0x70
> >> [ 401.419690][ T0] [c0000000062efbe0] [c00000000022a3dc] kprobe_optimizer+0x1dc/0x470
> >> [ 401.419757][ T0] [c0000000062efc60] [c000000000136684] process_one_work+0x2f4/0x530
> >> [ 401.419823][ T0] [c0000000062efd20] [c000000000138d28] worker_thread+0x78/0x570
> >> [ 401.419891][ T0] [c0000000062efdb0] [c000000000142424] kthread+0x194/0x1a0
> >> [ 401.419976][ T0] [c0000000062efe20] [c00000000000daf0] ret_from_kernel_thread+0x5c/0x6c
> >>
> >> But why is the synchronize_rcu_tasks() not completing?
> >>
> > I think that it is because RCU is not fully initialized by that time.
>
> Yeah that would explain it :)
>
> > The 36dadef23fcc ("kprobes: Init kprobes in early_initcall") patch
> > switches to early_initcall() that has a higher priority sequence than
> > core_initcall() that is used to complete an RCU setup in the rcu_set_runtime_mode().
>
> I was looking at debug_lockdep_rcu_enabled(), which is:
>
> noinstr int notrace debug_lockdep_rcu_enabled(void)
> {
> return rcu_scheduler_active != RCU_SCHEDULER_INACTIVE && debug_locks &&
> current->lockdep_recursion == 0;
> }
>
> That is not firing any warnings for me because rcu_scheduler_active is:
>
> (gdb) p/x rcu_scheduler_active
> $1 = 0x1
>
> Which is:
>
> #define RCU_SCHEDULER_INIT 1
>
Agree with that.
> But that's different to RCU_SCHEDULER_RUNNING, which is set in
> rcu_set_runtime_mode() as you mentioned:
>
> static int __init rcu_set_runtime_mode(void)
> {
> rcu_test_sync_prims();
> rcu_scheduler_active = RCU_SCHEDULER_RUNNING;
> kfree_rcu_scheduler_running();
> rcu_test_sync_prims();
> return 0;
> }
>
BTW, since you can reproduce it and have a test setup, could you please
check that:
<snip>
-core_initcall(rcu_set_runtime_mode);
+early_initcall(rcu_set_runtime_mode);
<snip>
Just in case. The the synchronize_rcu_tasks() gets blocked:
<snip>
void __wait_rcu_gp(bool checktiny, int n, call_rcu_func_t *crcu_array,
if (crcu_array[j] == crcu_array[i])
break;
if (j == i) {
wait_for_completion(&rs_array[i].completion); <--- here
destroy_rcu_head_on_stack(&rs_array[i].head);
}
...
<snip>
but that is obvious when looking at your full traces.
> The comment on rcu_scheduler_active implies that once we're at
> RCU_SCHEDULER_INIT things should work:
>
> /*
> * The rcu_scheduler_active variable is initialized to the value
> * RCU_SCHEDULER_INACTIVE and transitions RCU_SCHEDULER_INIT just before the
> * first task is spawned. So when this variable is RCU_SCHEDULER_INACTIVE,
> * RCU can assume that there is but one task, allowing RCU to (for example)
> * optimize synchronize_rcu() to a simple barrier(). When this variable
> * is RCU_SCHEDULER_INIT, RCU must actually do all the hard work required
> * to detect real grace periods. This variable is also used to suppress
> * boot-time false positives from lockdep-RCU error checking. Finally, it
> * transitions from RCU_SCHEDULER_INIT to RCU_SCHEDULER_RUNNING after RCU
> * is fully initialized, including all of its kthreads having been spawned.
> */
>
>
> So I'm not sure, the comments and the debug checks imply that it is OK
> for kprobes to be using RCU this early.
>
Sounds like it should be possible.
>
> I guess I'll keep digging.
>
Thank you! I also will dig further with that even though i do not have a setup
for reproducing it.
--
Vlad Rezki
^ permalink raw reply
* Re: [MOCKUP] x86/mm: Lightweight lazy mm refcounting
From: Rik van Riel @ 2020-12-03 14:26 UTC (permalink / raw)
To: Matthew Wilcox, Andy Lutomirski
Cc: linux-arch, Arnd Bergmann, Dave Hansen, Will Deacon, X86 ML, LKML,
Nicholas Piggin, Linux-MM, Mathieu Desnoyers, Catalin Marinas,
linuxppc-dev
In-Reply-To: <20201203123129.GH11935@casper.infradead.org>
[-- Attachment #1: Type: text/plain, Size: 771 bytes --]
On Thu, 2020-12-03 at 12:31 +0000, Matthew Wilcox wrote:
> And this just makes me think RCU freeing of mm_struct. I'm sure it's
> more complicated than that (then, or now), but if an anonymous
> process
> is borrowing a freed mm, and the mm is freed by RCU then it will not
> go
> away until the task context switches. When we context switch back to
> the anon task, it'll borrow some other task's MM and won't even
> notice
> that the MM it was using has gone away.
One major complication here is that most of the
active_mm borrowing is done by the idle task,
but RCU does not wait for idle tasks to context
switch.
That means RCU, as it is today, is not a
mechanism that mm_struct freeing could just
piggyback off.
--
All Rights Reversed.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.9 27/39] sched/idle: Fix arch_cpu_idle() vs tracing
From: Heiko Carstens @ 2020-12-03 14:54 UTC (permalink / raw)
To: Sasha Levin
Cc: Mark Rutland, uclinux-h8-devel, linux-ia64, linux-parisc,
linux-s390, Peter Zijlstra, linux-hexagon, linux-sh, linux-um,
linux-kernel, stable, linux-mips, openrisc, linux-csky,
Sven Schnelle, linux-alpha, sparclinux, linux-riscv, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20201203132834.930999-27-sashal@kernel.org>
On Thu, Dec 03, 2020 at 08:28:21AM -0500, Sasha Levin wrote:
> From: Peter Zijlstra <peterz@infradead.org>
>
> [ Upstream commit 58c644ba512cfbc2e39b758dd979edd1d6d00e27 ]
>
> We call arch_cpu_idle() with RCU disabled, but then use
> local_irq_{en,dis}able(), which invokes tracing, which relies on RCU.
>
> Switch all arch_cpu_idle() implementations to use
> raw_local_irq_{en,dis}able() and carefully manage the
> lockdep,rcu,tracing state like we do in entry.
>
> (XXX: we really should change arch_cpu_idle() to not return with
> interrupts enabled)
>
> Reported-by: Sven Schnelle <svens@linux.ibm.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Tested-by: Mark Rutland <mark.rutland@arm.com>
> Link: https://lkml.kernel.org/r/20201120114925.594122626@infradead.org
> Signed-off-by: Sasha Levin <sashal@kernel.org>
This patch broke s390 irq state tracing. A patch to fix this is
scheduled to be merged upstream today (hopefully).
Therefore I think this patch should not yet go into 5.9 stable.
^ permalink raw reply
* [PATCH AUTOSEL 4.19 10/14] soc: fsl: dpio: Get the cpumask through cpumask_of(cpu)
From: Sasha Levin @ 2020-12-03 13:30 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Yi Wang, Sasha Levin, Hao Si, Li Yang, Lin Chen, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20201203133010.931600-1-sashal@kernel.org>
From: Hao Si <si.hao@zte.com.cn>
[ Upstream commit 2663b3388551230cbc4606a40fabf3331ceb59e4 ]
The local variable 'cpumask_t mask' is in the stack memory, and its address
is assigned to 'desc->affinity' in 'irq_set_affinity_hint()'.
But the memory area where this variable is located is at risk of being
modified.
During LTP testing, the following error was generated:
Unable to handle kernel paging request at virtual address ffff000012e9b790
Mem abort info:
ESR = 0x96000007
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x00000007
CM = 0, WnR = 0
swapper pgtable: 4k pages, 48-bit VAs, pgdp = 0000000075ac5e07
[ffff000012e9b790] pgd=00000027dbffe003, pud=00000027dbffd003,
pmd=00000027b6d61003, pte=0000000000000000
Internal error: Oops: 96000007 [#1] PREEMPT SMP
Modules linked in: xt_conntrack
Process read_all (pid: 20171, stack limit = 0x0000000044ea4095)
CPU: 14 PID: 20171 Comm: read_all Tainted: G B W
Hardware name: NXP Layerscape LX2160ARDB (DT)
pstate: 80000085 (Nzcv daIf -PAN -UAO)
pc : irq_affinity_hint_proc_show+0x54/0xb0
lr : irq_affinity_hint_proc_show+0x4c/0xb0
sp : ffff00001138bc10
x29: ffff00001138bc10 x28: 0000ffffd131d1e0
x27: 00000000007000c0 x26: ffff8025b9480dc0
x25: ffff8025b9480da8 x24: 00000000000003ff
x23: ffff8027334f8300 x22: ffff80272e97d000
x21: ffff80272e97d0b0 x20: ffff8025b9480d80
x19: ffff000009a49000 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000
x15: 0000000000000000 x14: 0000000000000000
x13: 0000000000000000 x12: 0000000000000040
x11: 0000000000000000 x10: ffff802735b79b88
x9 : 0000000000000000 x8 : 0000000000000000
x7 : ffff000009a49848 x6 : 0000000000000003
x5 : 0000000000000000 x4 : ffff000008157d6c
x3 : ffff00001138bc10 x2 : ffff000012e9b790
x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
irq_affinity_hint_proc_show+0x54/0xb0
seq_read+0x1b0/0x440
proc_reg_read+0x80/0xd8
__vfs_read+0x60/0x178
vfs_read+0x94/0x150
ksys_read+0x74/0xf0
__arm64_sys_read+0x24/0x30
el0_svc_common.constprop.0+0xd8/0x1a0
el0_svc_handler+0x34/0x88
el0_svc+0x10/0x14
Code: f9001bbf 943e0732 f94066c2 b4000062 (f9400041)
---[ end trace b495bdcb0b3b732b ]---
Kernel panic - not syncing: Fatal exception
SMP: stopping secondary CPUs
SMP: failed to stop secondary CPUs 0,2-4,6,8,11,13-15
Kernel Offset: disabled
CPU features: 0x0,21006008
Memory Limit: none
---[ end Kernel panic - not syncing: Fatal exception ]---
Fix it by using 'cpumask_of(cpu)' to get the cpumask.
Signed-off-by: Hao Si <si.hao@zte.com.cn>
Signed-off-by: Lin Chen <chen.lin5@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Signed-off-by: Li Yang <leoyang.li@nxp.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/soc/fsl/dpio/dpio-driver.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/dpio/dpio-driver.c b/drivers/soc/fsl/dpio/dpio-driver.c
index b60b77bfaffae..ea6f8904c01b5 100644
--- a/drivers/soc/fsl/dpio/dpio-driver.c
+++ b/drivers/soc/fsl/dpio/dpio-driver.c
@@ -53,7 +53,6 @@ static int register_dpio_irq_handlers(struct fsl_mc_device *dpio_dev, int cpu)
struct dpio_priv *priv;
int error;
struct fsl_mc_device_irq *irq;
- cpumask_t mask;
priv = dev_get_drvdata(&dpio_dev->dev);
@@ -72,9 +71,7 @@ static int register_dpio_irq_handlers(struct fsl_mc_device *dpio_dev, int cpu)
}
/* set the affinity hint */
- cpumask_clear(&mask);
- cpumask_set_cpu(cpu, &mask);
- if (irq_set_affinity_hint(irq->msi_desc->irq, &mask))
+ if (irq_set_affinity_hint(irq->msi_desc->irq, cpumask_of(cpu)))
dev_err(&dpio_dev->dev,
"irq_set_affinity failed irq %d cpu %d\n",
irq->msi_desc->irq, cpu);
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 6/8] lazy tlb: shoot lazies, a non-refcounting lazy tlb option
From: Alexander Gordeev @ 2020-12-03 17:03 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-arch, linuxppc-dev, Arnd Bergmann, Vasily Gorbik,
Christian Borntraeger, Peter Zijlstra, Catalin Marinas,
Heiko Carstens, X86 ML, LKML, Nicholas Piggin, Linux-MM,
Dave Hansen, Mathieu Desnoyers, Will Deacon
In-Reply-To: <CALCETrXAR_9EGaOF8ymVkZycxgZkYk0dR+NjEpTfVzdcS3sOVw@mail.gmail.com>
On Mon, Nov 30, 2020 at 10:31:51AM -0800, Andy Lutomirski wrote:
> other arch folk: there's some background here:
>
> https://lkml.kernel.org/r/CALCETrVXUbe8LfNn-Qs+DzrOQaiw+sFUg1J047yByV31SaTOZw@mail.gmail.com
>
> On Sun, Nov 29, 2020 at 12:16 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > On Sat, Nov 28, 2020 at 7:54 PM Andy Lutomirski <luto@kernel.org> wrote:
> > >
> > > On Sat, Nov 28, 2020 at 8:02 AM Nicholas Piggin <npiggin@gmail.com> wrote:
> > > >
> > > > On big systems, the mm refcount can become highly contented when doing
> > > > a lot of context switching with threaded applications (particularly
> > > > switching between the idle thread and an application thread).
> > > >
> > > > Abandoning lazy tlb slows switching down quite a bit in the important
> > > > user->idle->user cases, so so instead implement a non-refcounted scheme
> > > > that causes __mmdrop() to IPI all CPUs in the mm_cpumask and shoot down
> > > > any remaining lazy ones.
> > > >
> > > > Shootdown IPIs are some concern, but they have not been observed to be
> > > > a big problem with this scheme (the powerpc implementation generated
> > > > 314 additional interrupts on a 144 CPU system during a kernel compile).
> > > > There are a number of strategies that could be employed to reduce IPIs
> > > > if they turn out to be a problem for some workload.
> > >
> > > I'm still wondering whether we can do even better.
> > >
> >
> > Hold on a sec.. __mmput() unmaps VMAs, frees pagetables, and flushes
> > the TLB. On x86, this will shoot down all lazies as long as even a
> > single pagetable was freed. (Or at least it will if we don't have a
> > serious bug, but the code seems okay. We'll hit pmd_free_tlb, which
> > sets tlb->freed_tables, which will trigger the IPI.) So, on
> > architectures like x86, the shootdown approach should be free. The
> > only way it ought to have any excess IPIs is if we have CPUs in
> > mm_cpumask() that don't need IPI to free pagetables, which could
> > happen on paravirt.
>
> Indeed, on x86, we do this:
>
> [ 11.558844] flush_tlb_mm_range.cold+0x18/0x1d
> [ 11.559905] tlb_finish_mmu+0x10e/0x1a0
> [ 11.561068] exit_mmap+0xc8/0x1a0
> [ 11.561932] mmput+0x29/0xd0
> [ 11.562688] do_exit+0x316/0xa90
> [ 11.563588] do_group_exit+0x34/0xb0
> [ 11.564476] __x64_sys_exit_group+0xf/0x10
> [ 11.565512] do_syscall_64+0x34/0x50
>
> and we have info->freed_tables set.
>
> What are the architectures that have large systems like?
>
> x86: we already zap lazies, so it should cost basically nothing to do
> a little loop at the end of __mmput() to make sure that no lazies are
> left. If we care about paravirt performance, we could implement one
> of the optimizations I mentioned above to fix up the refcounts instead
> of sending an IPI to any remaining lazies.
>
> arm64: AFAICT arm64's flush uses magic arm64 hardware support for
> remote flushes, so any lazy mm references will still exist after
> exit_mmap(). (arm64 uses lazy TLB, right?) So this is kind of like
> the x86 paravirt case. Are there large enough arm64 systems that any
> of this matters?
>
> s390x: The code has too many acronyms for me to understand it fully,
> but I think it's more or less the same situation as arm64. How big do
> s390x systems come?
>
> power: Ridiculously complicated, seems to vary by system and kernel config.
>
> So, Nick, your unconditional IPI scheme is apparently a big
> improvement for power, and it should be an improvement and have low
> cost for x86. On arm64 and s390x it will add more IPIs on process
> exit but reduce contention on context switching depending on how lazy
s390 does not invalidate TLBs per-CPU explicitly - we have special
instructions for that. Those in turn initiate signalling to other
CPUs, completely transparent to OS.
Apart from mm_count, I am struggling to realize how the suggested
scheme could change the the contention on s390 in connection with
TLB. Could you clarify a bit here, please?
> TLB works. I suppose we could try it for all architectures without
> any further optimizations. Or we could try one of the perhaps
> excessively clever improvements I linked above. arm64, s390x people,
> what do you think?
I do not immediately see anything in the series that would harm
performance on s390.
We however use mm_cpumask to distinguish between local and global TLB
flushes. With this series it looks like mm_cpumask is *required* to
be consistent with lazy users. And that is something quite diffucult
for us to adhere (at least in the foreseeable future).
But actually keeping track of lazy users in a cpumask is something
the generic code would rather do AFAICT.
Thanks!
^ permalink raw reply
* Re: [PATCH AUTOSEL 5.9 27/39] sched/idle: Fix arch_cpu_idle() vs tracing
From: Peter Zijlstra @ 2020-12-03 17:10 UTC (permalink / raw)
To: Heiko Carstens
Cc: Sasha Levin, Mark Rutland, linux-ia64, linux-parisc, linux-s390,
linux-hexagon, linux-sh, linux-um, linux-kernel, stable,
linux-mips, openrisc, sparclinux, linux-csky, Sven Schnelle,
linux-alpha, uclinux-h8-devel, linux-riscv, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20201203145442.GC9994@osiris>
On Thu, Dec 03, 2020 at 03:54:42PM +0100, Heiko Carstens wrote:
> On Thu, Dec 03, 2020 at 08:28:21AM -0500, Sasha Levin wrote:
> > From: Peter Zijlstra <peterz@infradead.org>
> >
> > [ Upstream commit 58c644ba512cfbc2e39b758dd979edd1d6d00e27 ]
> >
> > We call arch_cpu_idle() with RCU disabled, but then use
> > local_irq_{en,dis}able(), which invokes tracing, which relies on RCU.
> >
> > Switch all arch_cpu_idle() implementations to use
> > raw_local_irq_{en,dis}able() and carefully manage the
> > lockdep,rcu,tracing state like we do in entry.
> >
> > (XXX: we really should change arch_cpu_idle() to not return with
> > interrupts enabled)
> >
> > Reported-by: Sven Schnelle <svens@linux.ibm.com>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> > Tested-by: Mark Rutland <mark.rutland@arm.com>
> > Link: https://lkml.kernel.org/r/20201120114925.594122626@infradead.org
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
>
> This patch broke s390 irq state tracing. A patch to fix this is
> scheduled to be merged upstream today (hopefully).
> Therefore I think this patch should not yet go into 5.9 stable.
Agreed.
^ permalink raw reply
* Re: [PATCH 6/8] lazy tlb: shoot lazies, a non-refcounting lazy tlb option
From: Andy Lutomirski @ 2020-12-03 17:14 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-arch, linuxppc-dev, Arnd Bergmann, Vasily Gorbik,
Dave Hansen, Peter Zijlstra, Catalin Marinas, Heiko Carstens,
X86 ML, LKML, Nicholas Piggin, Linux-MM, Christian Borntraeger,
Mathieu Desnoyers, Andy Lutomirski, Will Deacon
In-Reply-To: <20201203170332.GA27195@oc3871087118.ibm.com>
> On Dec 3, 2020, at 9:09 AM, Alexander Gordeev <agordeev@linux.ibm.com> wrote:
>
> On Mon, Nov 30, 2020 at 10:31:51AM -0800, Andy Lutomirski wrote:
>> other arch folk: there's some background here:
>
>>
>> power: Ridiculously complicated, seems to vary by system and kernel config.
>>
>> So, Nick, your unconditional IPI scheme is apparently a big
>> improvement for power, and it should be an improvement and have low
>> cost for x86. On arm64 and s390x it will add more IPIs on process
>> exit but reduce contention on context switching depending on how lazy
>
> s390 does not invalidate TLBs per-CPU explicitly - we have special
> instructions for that. Those in turn initiate signalling to other
> CPUs, completely transparent to OS.
Just to make sure I understand: this means that you broadcast flushes to all CPUs, not just a subset?
>
> Apart from mm_count, I am struggling to realize how the suggested
> scheme could change the the contention on s390 in connection with
> TLB. Could you clarify a bit here, please?
I’m just talking about mm_count. Maintaining mm_count is quite expensive on some workloads.
>
>> TLB works. I suppose we could try it for all architectures without
>> any further optimizations. Or we could try one of the perhaps
>> excessively clever improvements I linked above. arm64, s390x people,
>> what do you think?
>
> I do not immediately see anything in the series that would harm
> performance on s390.
>
> We however use mm_cpumask to distinguish between local and global TLB
> flushes. With this series it looks like mm_cpumask is *required* to
> be consistent with lazy users. And that is something quite diffucult
> for us to adhere (at least in the foreseeable future).
You don’t actually need to maintain mm_cpumask — we could scan all CPUs instead.
>
> But actually keeping track of lazy users in a cpumask is something
> the generic code would rather do AFAICT.
The problem is that arches don’t agree on what the contents of mm_cpumask should be. Tracking a mask of exactly what the arch wants in generic code is a nontrivial operation.
^ permalink raw reply
* Re: [PATCH] powerpc/hotplug: assign hot added LMB to the right node
From: Greg KH @ 2020-12-03 18:25 UTC (permalink / raw)
To: Laurent Dufour
Cc: Nathan Lynch, Scott Cheloha, linux-kernel, stable, Paul Mackerras,
linuxppc-dev
In-Reply-To: <20201203101514.33591-1-ldufour@linux.ibm.com>
On Thu, Dec 03, 2020 at 11:15:14AM +0100, Laurent Dufour wrote:
> This patch applies to 5.9 and earlier kernels only.
>
> Since 5.10, this has been fortunately fixed by the commit
> e5e179aa3a39 ("pseries/drmem: don't cache node id in drmem_lmb struct").
Why can't we just backport that patch instead? It's almost always
better to do that than to have a one-off patch, as almost always those
have bugs in them.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 6/8] lazy tlb: shoot lazies, a non-refcounting lazy tlb option
From: Alexander Gordeev @ 2020-12-03 18:33 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-arch, linuxppc-dev, Arnd Bergmann, Vasily Gorbik,
Dave Hansen, Peter Zijlstra, Catalin Marinas, Heiko Carstens,
X86 ML, LKML, Nicholas Piggin, Linux-MM, Christian Borntraeger,
Mathieu Desnoyers, Andy Lutomirski, Will Deacon
In-Reply-To: <E6BC2596-6087-49F2-8758-CA5598998BBE@amacapital.net>
On Thu, Dec 03, 2020 at 09:14:22AM -0800, Andy Lutomirski wrote:
>
>
> > On Dec 3, 2020, at 9:09 AM, Alexander Gordeev <agordeev@linux.ibm.com> wrote:
> >
> > On Mon, Nov 30, 2020 at 10:31:51AM -0800, Andy Lutomirski wrote:
> >> other arch folk: there's some background here:
>
> >
> >>
> >> power: Ridiculously complicated, seems to vary by system and kernel config.
> >>
> >> So, Nick, your unconditional IPI scheme is apparently a big
> >> improvement for power, and it should be an improvement and have low
> >> cost for x86. On arm64 and s390x it will add more IPIs on process
> >> exit but reduce contention on context switching depending on how lazy
> >
> > s390 does not invalidate TLBs per-CPU explicitly - we have special
> > instructions for that. Those in turn initiate signalling to other
> > CPUs, completely transparent to OS.
>
> Just to make sure I understand: this means that you broadcast flushes to all CPUs, not just a subset?
Correct.
If mm has one CPU attached we flush TLB only for that CPU.
If mm has more than one CPUs attached we flush all CPUs' TLBs.
In fact, details are bit more complicated, since the hardware
is able to flush subsets of TLB entries depending on provided
parameters (e.g page tables used to create that entries).
But we can not select a CPU subset.
> > Apart from mm_count, I am struggling to realize how the suggested
> > scheme could change the the contention on s390 in connection with
> > TLB. Could you clarify a bit here, please?
>
> I’m just talking about mm_count. Maintaining mm_count is quite expensive on some workloads.
>
> >
> >> TLB works. I suppose we could try it for all architectures without
> >> any further optimizations. Or we could try one of the perhaps
> >> excessively clever improvements I linked above. arm64, s390x people,
> >> what do you think?
> >
> > I do not immediately see anything in the series that would harm
> > performance on s390.
> >
> > We however use mm_cpumask to distinguish between local and global TLB
> > flushes. With this series it looks like mm_cpumask is *required* to
> > be consistent with lazy users. And that is something quite diffucult
> > for us to adhere (at least in the foreseeable future).
>
> You don’t actually need to maintain mm_cpumask — we could scan all CPUs instead.
>
> >
> > But actually keeping track of lazy users in a cpumask is something
> > the generic code would rather do AFAICT.
>
> The problem is that arches don’t agree on what the contents of mm_cpumask should be. Tracking a mask of exactly what the arch wants in generic code is a nontrivial operation.
It could be yet another cpumask or the CPU scan you mentioned.
Just wanted to make sure there is no new requirement for an arch
to maintain mm_cpumask ;)
Thanks, Andy!
^ permalink raw reply
* Re: [PATCH 3/7] powerpc/64s: flush L1D after user accesses
From: Qian Cai @ 2020-12-03 17:17 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev; +Cc: cmr, spoorts2, npiggin
In-Reply-To: <20201119231333.361771-4-dja@axtens.net>
On Fri, 2020-11-20 at 10:13 +1100, Daniel Axtens wrote:
> From: Nicholas Piggin <npiggin@gmail.com>
>
> IBM Power9 processors can speculatively operate on data in the L1 cache
> before it has been completely validated, via a way-prediction mechanism. It
> is not possible for an attacker to determine the contents of impermissible
> memory using this method, since these systems implement a combination of
> hardware and software security measures to prevent scenarios where
> protected data could be leaked.
>
> However these measures don't address the scenario where an attacker induces
> the operating system to speculatively execute instructions using data that
> the attacker controls. This can be used for example to speculatively bypass
> "kernel user access prevention" techniques, as discovered by Anthony
> Steinhauser of Google's Safeside Project. This is not an attack by itself,
> but there is a possibility it could be used in conjunction with
> side-channels or other weaknesses in the privileged code to construct an
> attack.
>
> This issue can be mitigated by flushing the L1 cache between privilege
> boundaries of concern. This patch flushes the L1 cache after user accesses.
>
> This is part of the fix for CVE-2020-4788.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
> .../admin-guide/kernel-parameters.txt | 4 +
> .../powerpc/include/asm/book3s/64/kup-radix.h | 66 ++++++++------
> arch/powerpc/include/asm/exception-64s.h | 3 +
> arch/powerpc/include/asm/feature-fixups.h | 9 ++
> arch/powerpc/include/asm/kup.h | 19 +++--
> arch/powerpc/include/asm/security_features.h | 3 +
> arch/powerpc/include/asm/setup.h | 1 +
> arch/powerpc/kernel/exceptions-64s.S | 85 ++++++-------------
> arch/powerpc/kernel/setup_64.c | 62 ++++++++++++++
> arch/powerpc/kernel/vmlinux.lds.S | 7 ++
> arch/powerpc/lib/feature-fixups.c | 50 +++++++++++
> arch/powerpc/platforms/powernv/setup.c | 10 ++-
> arch/powerpc/platforms/pseries/setup.c | 4 +
> 13 files changed, 233 insertions(+), 90 deletions(-)
[]
> diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
> index 3ee1ec60be84..97c2394e7dea 100644
> --- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
[]
> +static inline bool
> +bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
> +{
> + return WARN(mmu_has_feature(MMU_FTR_RADIX_KUAP) &&
> + (regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ)),
> + "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
> +}
A simple "echo t > /proc/sysrq-trigger" will trigger this warning almost
endlessly on Power8 NV.
.config:
https://cailca.coding.net/public/linux/mm/git/files/master/powerpc.config
[ 391.734028][ T1986] Bug: Read fault blocked by AMR!
[ 391.734032][ T1986] WARNING: CPU: 80 PID: 1986 at arch/powerpc/include/asm/book3s/64/kup-radix.h:145 do_page_fault+0x8fc/0xb70
[ 391.734232][ T1986] Modules linked in: kvm_hv kvm ip_tables x_tables sd_mod ahci libahci tg3 libata firmware_class libphy dm_mirror dm_region_hash dm_log dm_mod
[ 391.734425][ T1986] CPU: 80 PID: 1986 Comm: bash Tainted: G W 5.10.0-rc6-next-20201203+ #3
[ 391.734535][ T1986] NIP: c00000000004dd1c LR: c00000000004dd18 CTR: 0000000000000000
[ 391.734648][ T1986] REGS: c00020003a0bf3a0 TRAP: 0700 Tainted: G W (5.10.0-rc6-next-20201203+)
[ 391.734768][ T1986] MSR: 9000000000021033 <SF,HV,ME,IR,DR,RI,LE> CR: 48222284 XER: 00000000
[ 391.734906][ T1986] CFAR: c0000000000bb05c IRQMASK: 1
[ 391.734906][ T1986] GPR00: c00000000004dd18 c00020003a0bf630 c000000007fe0d00 000000000000001f
[ 391.734906][ T1986] GPR04: c000000000f1cc58 0000000000000003 0000000000000027 c000201cc6207218
[ 391.734906][ T1986] GPR08: 0000000000000023 0000000000000002 c00020004753bd80 c000000007f1cee8
[ 391.734906][ T1986] GPR12: 0000000000002000 c000201fff7f8380 0000000040000000 0000000110929798
[ 391.734906][ T1986] GPR16: 0000000110929724 00000001108c6988 000000011085f290 000000011092d568
[ 391.734906][ T1986] GPR20: 00000001229f1f80 0000000000000001 0000000000000001 c000000000aa8dc8
[ 391.734906][ T1986] GPR24: c000000000ab4a00 c00020001cc8c880 0000000000000000 0000000000000000
[ 391.734906][ T1986] GPR28: c00000000801aa18 0000000000000160 c00020003a0bf760 0000000000000300
[ 391.735865][ T1986] NIP [c00000000004dd1c] do_page_fault+0x8fc/0xb70
[ 391.735947][ T1986] LR [c00000000004dd18] do_page_fault+0x8f8/0xb70
[ 391.736033][ T1986] Call Trace:
[ 391.736072][ T1986] [c00020003a0bf630] [c00000000004dd18] do_page_fault+0x8f8/0xb70 (unreliable)
[ 391.736181][ T1986] [c00020003a0bf6f0] [c00000000000c1b8] handle_page_fault+0x10/0x2c
[ 391.736294][ T1986] --- interrupt: 300 at copy_from_kernel_nofault+0x68/0x190
[ 391.736294][ T1986] LR = copy_from_kernel_nofault+0x40/0x190
[ 391.736441][ T1986] [c00020003a0bf9f0] [c00020003a0bfa30] 0xc00020003a0bfa30 (unreliable)
[ 391.736565][ T1986] [c00020003a0bfa30] [c0000000000edd98] print_worker_info+0xe8/0x1c0
[ 391.736672][ T1986] [c00020003a0bfaf0] [c000000000104b0c] sched_show_task+0x2dc/0x350
[ 391.736807][ T1986] [c00020003a0bfb70] [c000000000112cd8] show_state_filter+0x148/0x320
[ 391.736899][ T1986] [c00020003a0bfbe0] [c00000000070a3f4] sysrq_handle_showstate+0x24/0x40
[ 391.736995][ T1986] [c00020003a0bfc00] [c00000000070add4] __handle_sysrq+0x164/0x280
[ 391.737111][ T1986] [c00020003a0bfca0] [c00000000070b03c] write_sysrq_trigger+0xfc/0x13c
[ 391.737233][ T1986] [c00020003a0bfce0] [c0000000004c579c] proc_reg_write+0x10c/0x1b0
[ 391.737327][ T1986] [c00020003a0bfd10] [c0000000003e5ec4] vfs_write+0xf4/0x480
[ 391.737431][ T1986] [c00020003a0bfd70] [c0000000003e642c] ksys_write+0x7c/0x140
[ 391.737536][ T1986] [c00020003a0bfdc0] [c00000000002c578] system_call_exception+0xf8/0x1d0
[ 391.737623][ T1986] [c00020003a0bfe20] [c00000000000d1c8] system_call_common+0xe8/0x218
[ 391.737708][ T1986] Instruction dump:
[ 391.737777][ T1986] 60000000 2fbb0000 e93e0168 419e007c 2fa90000 3c82f8ac 388467a0 409cfed4
[ 391.737913][ T1986] 3c62f8ac 386368a8 4806d2e1 60000000 <0fe00000> e80100d0 3ae0000b eb410090
[ 391.738041][ T1986] irq event stamp: 126198
[ 391.738077][ T1986] hardirqs last enabled at (126197): [<c00000000002cbf4>] interrupt_exit_kernel_prepare+0xb4/0x250
[ 391.738196][ T1986] hardirqs last disabled at (126198): [<c00000000000897c>] data_access_common_virt+0x16c/0x180
[ 391.738327][ T1986] softirqs last enabled at (126196): [<c000000000948f08>] __do_softirq+0x388/0x704
[ 391.738427][ T1986] softirqs last disabled at (126191): [<c0000000000c75b8>] irq_exit+0x198/0x1c0
[ 392.177827][ T1986] ---[ end trace 8eaf99b33f09def0 ]---
[ 392.177934][ T1986] Workqueue: 0x0 (mm_percpu_wq)
[ 392.177994][ T1986] Call Trace:
[ 392.178048][ T1986] [c00000002afbf9d0] [c000000000ab0ee8] __func__.4060+0x125178/0x185
^ permalink raw reply
* Re: [PATCH 3/7] powerpc/64s: flush L1D after user accesses
From: Qian Cai @ 2020-12-03 17:31 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev; +Cc: cmr, spoorts2, npiggin
In-Reply-To: <e82f315e08fe9f13ce4e94259968e0782ebb57a3.camel@redhat.com>
On Thu, 2020-12-03 at 12:17 -0500, Qian Cai wrote:
> A simple "echo t > /proc/sysrq-trigger" will trigger this warning almost
> endlessly on Power8 NV.
Correction -- POWER9 NV.
^ permalink raw reply
* Re: [PATCH 3/7] powerpc/64s: flush L1D after user accesses
From: Qian Cai @ 2020-12-03 18:50 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev; +Cc: cmr, spoorts2, npiggin
In-Reply-To: <e82f315e08fe9f13ce4e94259968e0782ebb57a3.camel@redhat.com>
On Thu, 2020-12-03 at 12:17 -0500, Qian Cai wrote:
> []
> > +static inline bool
> > +bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
> > +{
> > + return WARN(mmu_has_feature(MMU_FTR_RADIX_KUAP) &&
> > + (regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ)),
> > + "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
> > +}
>
> A simple "echo t > /proc/sysrq-trigger" will trigger this warning almost
> endlessly on POWER9 NV.
I have just realized the patch just moved this warning around, so the issue was
pre-existent. Since I have not tested sysrq-t regularly, I am not sure when it
started to break. So far, I have reverted some of those for testing which did
not help, i.e., the sysrq-t issue remains.
16852975f0f Revert "powerpc/64s: Use early_mmu_has_feature() in set_kuap()"
129e240ead32 Revert "powerpc: Implement user_access_save() and user_access_restore()"
edb0046c842c Revert "powerpc/64s/kuap: Add missing isync to KUAP restore paths"
2d46ee87ce44 Revert "powerpc/64/kuap: Conditionally restore AMR in interrupt exit"
c1e0e805fc57 Revert "powerpc/64s/kuap: Conditionally restore AMR in kuap_restore_amr asm"
7f30b7aaf23a Revert "selftests/powerpc: rfi_flush: disable entry flush if present"
bc9b9967a100 Revert "powerpc/64s: flush L1D on kernel entry"
b77e7b54f5eb Revert "powerpc/64s: flush L1D after user accesses"
22dddf532c64 Revert "powerpc: Only include kup-radix.h for 64-bit Book3S"
2679d155c46a Revert "selftests/powerpc: entry flush test"
87954b9b4243 Revert "selftests/powerpc: refactor entry and rfi_flush tests"
342d82bd4c5d Revert "powerpc/64s: rename pnv|pseries_setup_rfi_flush to _setup_security_mitigations"
^ permalink raw reply
* Re: [MOCKUP] x86/mm: Lightweight lazy mm refcounting
From: Nicholas Piggin @ 2020-12-03 22:13 UTC (permalink / raw)
To: Andy Lutomirski, Peter Zijlstra
Cc: linux-arch, X86 ML, Arnd Bergmann, Linux-MM, Catalin Marinas,
Rik van Riel, LKML, Dave Hansen, Will Deacon, Mathieu Desnoyers,
linuxppc-dev
In-Reply-To: <20201203084448.GF2414@hirez.programming.kicks-ass.net>
Excerpts from Peter Zijlstra's message of December 3, 2020 6:44 pm:
> On Wed, Dec 02, 2020 at 09:25:51PM -0800, Andy Lutomirski wrote:
>
>> power: same as ARM, except that the loop may be rather larger since
>> the systems are bigger. But I imagine it's still faster than Nick's
>> approach -- a cmpxchg to a remote cacheline should still be faster than
>> an IPI shootdown.
>
> While a single atomic might be cheaper than an IPI, the comparison
> doesn't work out nicely. You do the xchg() on every unlazy, while the
> IPI would be once per process exit.
>
> So over the life of the process, it might do very many unlazies, adding
> up to a total cost far in excess of what the single IPI would've been.
Yeah this is the concern, I looked at things that add cost to the
idle switch code and it gets hard to justify the scalability improvement
when you slow these fundmaental things down even a bit.
I still think working on the assumption that IPIs = scary expensive
might not be correct. An IPI itself is, but you only issue them when
you've left a lazy mm on another CPU which just isn't that often.
Thanks,
Nick
^ permalink raw reply
* [PATCH] ASoC: fsl_aud2htx: mark PM functions as __maybe_unused
From: Arnd Bergmann @ 2020-12-03 22:28 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Shengjiu Wang
Cc: alsa-devel, Arnd Bergmann, Fabio Estevam, linuxppc-dev,
linux-kernel, Shengjiu Wang
From: Arnd Bergmann <arnd@arndb.de>
When CONFIG_PM is disabled, we get a warning for unused functions:
sound/soc/fsl/fsl_aud2htx.c:261:12: error: unused function 'fsl_aud2htx_runtime_suspend' [-Werror,-Wunused-function]
static int fsl_aud2htx_runtime_suspend(struct device *dev)
sound/soc/fsl/fsl_aud2htx.c:271:12: error: unused function 'fsl_aud2htx_runtime_resume' [-Werror,-Wunused-function]
static int fsl_aud2htx_runtime_resume(struct device *dev)
Mark these as __maybe_unused to avoid the warning without adding
an #ifdef.
Fixes: 8a24c834c053 ("ASoC: fsl_aud2htx: Add aud2htx module driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
sound/soc/fsl/fsl_aud2htx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_aud2htx.c b/sound/soc/fsl/fsl_aud2htx.c
index 4091ccc7c3e9..d70d5e75a30c 100644
--- a/sound/soc/fsl/fsl_aud2htx.c
+++ b/sound/soc/fsl/fsl_aud2htx.c
@@ -258,7 +258,7 @@ static int fsl_aud2htx_remove(struct platform_device *pdev)
return 0;
}
-static int fsl_aud2htx_runtime_suspend(struct device *dev)
+static int __maybe_unused fsl_aud2htx_runtime_suspend(struct device *dev)
{
struct fsl_aud2htx *aud2htx = dev_get_drvdata(dev);
@@ -268,7 +268,7 @@ static int fsl_aud2htx_runtime_suspend(struct device *dev)
return 0;
}
-static int fsl_aud2htx_runtime_resume(struct device *dev)
+static int __maybe_unused fsl_aud2htx_runtime_resume(struct device *dev)
{
struct fsl_aud2htx *aud2htx = dev_get_drvdata(dev);
int ret;
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 0/6] Add documentation for Documentation/features at the built docs
From: Jonathan Corbet @ 2020-12-03 22:36 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Rich Felker, linux-ia64, Linux Doc Mailing List, x86, linux-mips,
James E.J. Bottomley, Paul Mackerras, H. Peter Anvin, linux-riscv,
Will Deacon, Thomas Gleixner, Jonas Bonn, linux-s390,
Yoshinori Sato, Helge Deller, linux-sh, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, Fenghua Yu, Albert Ou, Kees Cook,
Vasily Gorbik, Heiko Carstens, Jonathan Neuschäfer,
Stefan Kristiansson, Tony Luck, Borislav Petkov, Paul Walmsley,
Stafford Horne, Daniel W. S. Almeida, linux-arm-kernel,
Thomas Bogendoerfer, linux-parisc, Andrew Cooper, linux-kernel,
openrisc, Palmer Dabbelt, Masami Hiramatsu, Greg Kroah-Hartman,
linuxppc-dev
In-Reply-To: <cover.1606748711.git.mchehab+huawei@kernel.org>
On Mon, 30 Nov 2020 16:36:29 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> This series got already submitted last year:
>
> https://lore.kernel.org/lkml/cover.1561222784.git.mchehab+samsung@kernel.org/
>
> Yet, on that time, there were too many other patches related to ReST
> conversion floating around. So, at the end, I guess this one got missed.
>
> So, I did a rebase on the top of upstream, and added a few new changes.
OK, I've gone ahead and applied these; it gains me a new trivial conflict
with x86, but so be it...
That said, I think that the RST table formatting could be *way* improved.
The current tables are all white space and hard to make sense of. What if
we condensed the information? Just looking at the first entry in
Documentation/admin-guide/features.html, perhaps it could look like:
FEATURE KCONFIG/DESCRIPTION STATUS
cBPF-JIT HAVE_CBPF_JIT TODO: alpha, arc, arm...
ok: mips, powerpc, ...
arch supports cBPF JIT
optimizations
The result would be far more compact and easy to read, IMO. I may get
around to giving this a try if (hint :) nobody else gets there first.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_aud2htx: mark PM functions as __maybe_unused
From: Nicolin Chen @ 2020-12-03 22:36 UTC (permalink / raw)
To: Arnd Bergmann
Cc: alsa-devel, linuxppc-dev, Arnd Bergmann, Timur Tabi,
Liam Girdwood, Shengjiu Wang, Shengjiu Wang, Xiubo Li,
Takashi Iwai, Jaroslav Kysela, Mark Brown, Fabio Estevam,
linux-kernel
In-Reply-To: <20201203222900.1042578-1-arnd@kernel.org>
On Thu, Dec 03, 2020 at 11:28:47PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> When CONFIG_PM is disabled, we get a warning for unused functions:
>
> sound/soc/fsl/fsl_aud2htx.c:261:12: error: unused function 'fsl_aud2htx_runtime_suspend' [-Werror,-Wunused-function]
> static int fsl_aud2htx_runtime_suspend(struct device *dev)
> sound/soc/fsl/fsl_aud2htx.c:271:12: error: unused function 'fsl_aud2htx_runtime_resume' [-Werror,-Wunused-function]
> static int fsl_aud2htx_runtime_resume(struct device *dev)
>
> Mark these as __maybe_unused to avoid the warning without adding
> an #ifdef.
>
> Fixes: 8a24c834c053 ("ASoC: fsl_aud2htx: Add aud2htx module driver")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* Re: [PATCH kernel v2] vfio/pci/nvlink2: Do not attempt NPU2 setup on POWER8NVL NPU
From: Alex Williamson @ 2020-12-03 23:48 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, kvm-ppc, stable, Leonardo Augusto Guimarães Garcia,
linuxppc-dev, David Gibson
In-Reply-To: <20201122073950.15684-1-aik@ozlabs.ru>
On Sun, 22 Nov 2020 18:39:50 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> We execute certain NPU2 setup code (such as mapping an LPID to a device
> in NPU2) unconditionally if an Nvlink bridge is detected. However this
> cannot succeed on POWER8NVL machines as the init helpers return an error
> other than ENODEV which means the device is there is and setup failed so
> vfio_pci_enable() fails and pass through is not possible.
>
> This changes the two NPU2 related init helpers to return -ENODEV if
> there is no "memory-region" device tree property as this is
> the distinction between NPU and NPU2.
>
> Tested on
> - POWER9 pvr=004e1201, Ubuntu 19.04 host, Ubuntu 18.04 vm,
> NVIDIA GV100 10de:1db1 driver 418.39
> - POWER8 pvr=004c0100, RHEL 7.6 host, Ubuntu 16.10 vm,
> NVIDIA P100 10de:15f9 driver 396.47
>
> Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver")
> Cc: stable@vger.kernel.org # 5.0
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v2:
> * updated commit log with tested configs and replaced P8+ with POWER8NVL for clarity
> ---
> drivers/vfio/pci/vfio_pci_nvlink2.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
Thanks, applies to vfio next branch for v5.11.
Alex
>
> diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c
> index 65c61710c0e9..9adcf6a8f888 100644
> --- a/drivers/vfio/pci/vfio_pci_nvlink2.c
> +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c
> @@ -231,7 +231,7 @@ int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
> return -EINVAL;
>
> if (of_property_read_u32(npu_node, "memory-region", &mem_phandle))
> - return -EINVAL;
> + return -ENODEV;
>
> mem_node = of_find_node_by_phandle(mem_phandle);
> if (!mem_node)
> @@ -393,7 +393,7 @@ int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
> int ret;
> struct vfio_pci_npu2_data *data;
> struct device_node *nvlink_dn;
> - u32 nvlink_index = 0;
> + u32 nvlink_index = 0, mem_phandle = 0;
> struct pci_dev *npdev = vdev->pdev;
> struct device_node *npu_node = pci_device_to_OF_node(npdev);
> struct pci_controller *hose = pci_bus_to_host(npdev->bus);
> @@ -408,6 +408,9 @@ int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
> if (!pnv_pci_get_gpu_dev(vdev->pdev))
> return -ENODEV;
>
> + if (of_property_read_u32(npu_node, "memory-region", &mem_phandle))
> + return -ENODEV;
> +
> /*
> * NPU2 normally has 8 ATSD registers (for concurrency) and 6 links
> * so we can allocate one register per link, using nvlink index as
^ permalink raw reply
* Re: [MOCKUP] x86/mm: Lightweight lazy mm refcounting
From: Andy Lutomirski @ 2020-12-04 2:17 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linux-arch, X86 ML, Arnd Bergmann, Linux-MM, Peter Zijlstra,
Catalin Marinas, Rik van Riel, LKML, Dave Hansen, Will Deacon,
Mathieu Desnoyers, Andy Lutomirski, linuxppc-dev
In-Reply-To: <1607033145.hcppy9ndl4.astroid@bobo.none>
> On Dec 3, 2020, at 2:13 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Excerpts from Peter Zijlstra's message of December 3, 2020 6:44 pm:
>>> On Wed, Dec 02, 2020 at 09:25:51PM -0800, Andy Lutomirski wrote:
>>>
>>> power: same as ARM, except that the loop may be rather larger since
>>> the systems are bigger. But I imagine it's still faster than Nick's
>>> approach -- a cmpxchg to a remote cacheline should still be faster than
>>> an IPI shootdown.
>>
>> While a single atomic might be cheaper than an IPI, the comparison
>> doesn't work out nicely. You do the xchg() on every unlazy, while the
>> IPI would be once per process exit.
>>
>> So over the life of the process, it might do very many unlazies, adding
>> up to a total cost far in excess of what the single IPI would've been.
>
> Yeah this is the concern, I looked at things that add cost to the
> idle switch code and it gets hard to justify the scalability improvement
> when you slow these fundmaental things down even a bit.
v2 fixes this and is generally much nicer. I’ll send it out in a couple hours.
>
> I still think working on the assumption that IPIs = scary expensive
> might not be correct. An IPI itself is, but you only issue them when
> you've left a lazy mm on another CPU which just isn't that often.
>
> Thanks,
> Nick
^ permalink raw reply
* [PATCH 0/3] Extend Parsing "ibm, thread-groups" for Shared-L2 information
From: Gautham R. Shenoy @ 2020-12-04 4:48 UTC (permalink / raw)
To: Srikar Dronamraju, Anton Blanchard, Vaidyanathan Srinivasan,
Michael Ellerman, Michael Neuling, Nicholas Piggin, Nathan Lynch,
Peter Zijlstra, Valentin Schneider
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
The "ibm,thread-groups" device-tree property is an array that is used
to indicate if groups of threads within a core share certain
properties. It provides details of which property is being shared by
which groups of threads. This array can encode information about
multiple properties being shared by different thread-groups within the
core.
Example: Suppose,
"ibm,thread-groups" = [1,2,4,8,10,12,14,9,11,13,15,2,2,4,8,10,12,14,9,11,13,15]
This can be decomposed up into two consecutive arrays:
a) [1,2,4,8,10,12,14,9,11,13,15]
b) [2,2,4,8,10,12,14,9,11,13,15]
where in,
a) provides information of Property "1" being shared by "2" groups,
each with "4" threads each. The "ibm,ppc-interrupt-server#s" of the
first group is {8,10,12,14} and the "ibm,ppc-interrupt-server#s" of
the second group is {9,11,13,15}. Property "1" is indicative of
the thread in the group sharing L1 cache, translation cache and
Instruction Data flow.
b) provides information of Property "2" being shared by "2" groups,
each group with "4" threads. The "ibm,ppc-interrupt-server#s" of
the first group is {8,10,12,14} and the
"ibm,ppc-interrupt-server#s" of the second group is
{9,11,13,15}. Property "2" indicates that the threads in each group
share the L2-cache.
The existing code assumes that the "ibm,thread-groups" encodes
information about only one property. Hence even on platforms which
encode information about multiple properties being shared by the
corresponding groups of threads, the current code will only pick the
first one. (In the above example, it will only consider
[1,2,4,8,10,12,14,9,11,13,15] but not [2,2,4,8,10,12,14,9,11,13,15]).
Furthermore, currently on platforms where groups of threads share L2
cache, we incorrectly create an extra CACHE level sched-domain that
maps to all the threads of the core.
For example, if "ibm,thread-groups" is
00000001 00000002 00000004 00000000
00000002 00000004 00000006 00000001
00000003 00000005 00000007 00000002
00000002 00000004 00000000 00000002
00000004 00000006 00000001 00000003
00000005 00000007
then, the sub-array
[00000002 00000002 00000004
00000000 00000002 00000004 00000006
00000001 00000003 00000005 00000007]
indicates that L2 (Property "2") is shared only between the threads of a single
group. There are "2" groups of threads where each group contains "4"
threads each. The groups being {0,2,4,6} and {1,3,5,7}.
However, the sched-domain hierarchy for CPUs 0,1 is
CPU0 attaching sched-domain(s):
domain-0: span=0,2,4,6 level=SMT
domain-1: span=0-7 level=CACHE
domain-2: span=0-15,24-39,48-55 level=MC
domain-3: span=0-55 level=DIE
CPU1 attaching sched-domain(s):
domain-0: span=1,3,5,7 level=SMT
domain-1: span=0-7 level=CACHE
domain-2: span=0-15,24-39,48-55 level=MC
domain-3: span=0-55 level=DIE
where the CACHE domain reports that L2 is shared across the entire
core which is incorrect on such platforms.
This patchset remedies these issues by extending the parsing support
for "ibm,thread-groups" to discover information about multiple
properties being shared by the corresponding groups of threads. In
particular we cano now detect if the groups of threads within a core
share the L2-cache. On such platforms, we populate the populating the
cpu_l2_cache_mask of every CPU to the core-siblings which share L2
with the CPU as specified in the by the "ibm,thread-groups" property
array.
With the patchset, the sched-domain hierarchy is correctly
reported. For eg for CPUs 0,1, with the patchset
CPU0 attaching sched-domain(s):
domain-0: span=0,2,4,6 level=SMT
domain-1: span=0-15,24-39,48-55 level=MC
domain-2: span=0-55 level=DIE
CPU1 attaching sched-domain(s):
domain-0: span=1,3,5,7 level=SMT
domain-1: span=0-15,24-39,48-55 level=MC
domain-2: span=0-55 level=DIE
The CACHE domain with span=0,2,4,6 for CPU 0 (span=1,3,5,7 for CPU 1
resp.) gets degenerated into the SMT domain. Furthermore, the
last-level-cache domain gets correctly set to the SMT sched-domain.
Finally, this patchset reports the correct shared_cpu_map/list in the
sysfs for L2 cache on such platforms. With the patchset for CPUs0, 1,
for L2 cache we see the correct shared_cpu_map/list
/sys/devices/system/cpu/cpu0/cache/index2/shared_cpu_list:0,2,4,6
/sys/devices/system/cpu/cpu0/cache/index2/shared_cpu_map:000000,00000055
/sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_list:1,3,5,7
/sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map:000000,000000aa
The patchset has been tested on older platforms which encode only the
L1 sharing information via "ibm,thread-groups" and there is no
regression found.
Gautham R. Shenoy (3):
powerpc/smp: Parse ibm,thread-groups with multiple properties
powerpc/smp: Add support detecting thread-groups sharing L2 cache
powerpc/cacheinfo: Print correct cache-sibling map/list for L2 cache
arch/powerpc/kernel/cacheinfo.c | 7 ++
arch/powerpc/kernel/smp.c | 198 +++++++++++++++++++++++++++++-----------
2 files changed, 150 insertions(+), 55 deletions(-)
--
1.9.4
^ permalink raw reply
* [PATCH 2/3] powerpc/smp: Add support detecting thread-groups sharing L2 cache
From: Gautham R. Shenoy @ 2020-12-04 4:48 UTC (permalink / raw)
To: Srikar Dronamraju, Anton Blanchard, Vaidyanathan Srinivasan,
Michael Ellerman, Michael Neuling, Nicholas Piggin, Nathan Lynch,
Peter Zijlstra, Valentin Schneider
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
In-Reply-To: <1607057327-29822-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
On POWER systems, groups of threads within a core sharing the L2-cache
can be indicated by the "ibm,thread-groups" property array with the
identifier "2".
This patch adds support for detecting this, and when present, populate
the populating the cpu_l2_cache_mask of every CPU to the core-siblings
which share L2 with the CPU as specified in the by the
"ibm,thread-groups" property array.
On a platform with the following "ibm,thread-group" configuration
00000001 00000002 00000004 00000000
00000002 00000004 00000006 00000001
00000003 00000005 00000007 00000002
00000002 00000004 00000000 00000002
00000004 00000006 00000001 00000003
00000005 00000007
Without this patch, the sched-domain hierarchy for CPUs 0,1 would be
CPU0 attaching sched-domain(s):
domain-0: span=0,2,4,6 level=SMT
domain-1: span=0-7 level=CACHE
domain-2: span=0-15,24-39,48-55 level=MC
domain-3: span=0-55 level=DIE
CPU1 attaching sched-domain(s):
domain-0: span=1,3,5,7 level=SMT
domain-1: span=0-7 level=CACHE
domain-2: span=0-15,24-39,48-55 level=MC
domain-3: span=0-55 level=DIE
The CACHE domain at 0-7 is incorrect since the ibm,thread-groups
sub-array
[00000002 00000002 00000004
00000000 00000002 00000004 00000006
00000001 00000003 00000005 00000007]
indicates that L2 (Property "2") is shared only between the threads of a single
group. There are "2" groups of threads where each group contains "4"
threads each. The groups being {0,2,4,6} and {1,3,5,7}.
With this patch, the sched-domain hierarchy for CPUs 0,1 would be
CPU0 attaching sched-domain(s):
domain-0: span=0,2,4,6 level=SMT
domain-1: span=0-15,24-39,48-55 level=MC
domain-2: span=0-55 level=DIE
CPU1 attaching sched-domain(s):
domain-0: span=1,3,5,7 level=SMT
domain-1: span=0-15,24-39,48-55 level=MC
domain-2: span=0-55 level=DIE
The CACHE domain with span=0,2,4,6 for CPU 0 (span=1,3,5,7 for CPU 1
resp.) gets degenerated into the SMT domain. Furthermore, the
last-level-cache domain gets correctly set to the SMT sched-domain.
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 66 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 58 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 6a242a3..a116d2d 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -76,6 +76,7 @@
struct task_struct *secondary_current;
bool has_big_cores;
bool coregroup_enabled;
+bool thread_group_shares_l2;
DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
@@ -99,6 +100,7 @@ enum {
#define MAX_THREAD_LIST_SIZE 8
#define THREAD_GROUP_SHARE_L1 1
+#define THREAD_GROUP_SHARE_L2 2
struct thread_groups {
unsigned int property;
unsigned int nr_groups;
@@ -107,7 +109,7 @@ struct thread_groups {
};
/* Maximum number of properties that groups of threads within a core can share */
-#define MAX_THREAD_GROUP_PROPERTIES 1
+#define MAX_THREAD_GROUP_PROPERTIES 2
struct thread_groups_list {
unsigned int nr_properties;
@@ -121,6 +123,13 @@ struct thread_groups_list {
*/
DEFINE_PER_CPU(cpumask_var_t, cpu_l1_cache_map);
+/*
+ * On some big-cores system, thread_group_l2_cache_map for each CPU
+ * corresponds to the set its siblings within the core that share the
+ * L2-cache.
+ */
+DEFINE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
+
/* SMP operations for this machine */
struct smp_ops_t *smp_ops;
@@ -718,7 +727,9 @@ static void or_cpumasks_related(int i, int j, struct cpumask *(*srcmask)(int),
*
* ibm,thread-groups[i + 0] tells us the property based on which the
* threads are being grouped together. If this value is 1, it implies
- * that the threads in the same group share L1, translation cache.
+ * that the threads in the same group share L1, translation cache. If
+ * the value is 2, it implies that the threads in the same group share
+ * the same L2 cache.
*
* ibm,thread-groups[i+1] tells us how many such thread groups exist for the
* property ibm,thread-groups[i]
@@ -745,10 +756,10 @@ static void or_cpumasks_related(int i, int j, struct cpumask *(*srcmask)(int),
* 12}.
*
* b) there are 2 groups of 4 threads each, where each group of
- * threads share some property indicated by the first value 2. The
- * "ibm,ppc-interrupt-server#s" of the first group is {5,7,9,11}
- * and the "ibm,ppc-interrupt-server#s" of the second group is
- * {6,8,10,12} structure
+ * threads share some property indicated by the first value 2 (L2
+ * cache). The "ibm,ppc-interrupt-server#s" of the first group is
+ * {5,7,9,11} and the "ibm,ppc-interrupt-server#s" of the second
+ * group is {6,8,10,12} structure
*
* Returns 0 on success, -EINVAL if the property does not exist,
* -ENODATA if property does not have a value, and -EOVERFLOW if the
@@ -840,7 +851,8 @@ static int init_cpu_cache_map(int cpu, unsigned int cache_property)
if (!dn)
return -ENODATA;
- if (!(cache_property == THREAD_GROUP_SHARE_L1))
+ if (!(cache_property == THREAD_GROUP_SHARE_L1 ||
+ cache_property == THREAD_GROUP_SHARE_L2))
return -EINVAL;
if (!cpu_tgl->nr_properties) {
@@ -867,7 +879,10 @@ static int init_cpu_cache_map(int cpu, unsigned int cache_property)
goto out;
}
- mask = &per_cpu(cpu_l1_cache_map, cpu);
+ if (cache_property == THREAD_GROUP_SHARE_L1)
+ mask = &per_cpu(cpu_l1_cache_map, cpu);
+ else if (cache_property == THREAD_GROUP_SHARE_L2)
+ mask = &per_cpu(thread_group_l2_cache_map, cpu);
zalloc_cpumask_var_node(mask, GFP_KERNEL, cpu_to_node(cpu));
@@ -973,6 +988,16 @@ static int init_big_cores(void)
}
has_big_cores = true;
+
+ for_each_possible_cpu(cpu) {
+ int err = init_cpu_cache_map(cpu, THREAD_GROUP_SHARE_L2);
+
+ if (err)
+ return err;
+ }
+
+ thread_group_shares_l2 = true;
+ pr_info("Thread-groups in a core share L2-cache\n");
return 0;
}
@@ -1287,6 +1312,31 @@ static bool update_mask_by_l2(int cpu, cpumask_var_t *mask)
if (has_big_cores)
submask_fn = cpu_smallcore_mask;
+
+ /*
+ * If the threads in a thread-group share L2 cache, then then
+ * the L2-mask can be obtained from thread_group_l2_cache_map.
+ */
+ if (thread_group_shares_l2) {
+ /* Siblings that share L1 is a subset of siblings that share L2.*/
+ or_cpumasks_related(cpu, cpu, submask_fn, cpu_l2_cache_mask);
+ if (*mask) {
+ cpumask_andnot(*mask,
+ per_cpu(thread_group_l2_cache_map, cpu),
+ cpu_l2_cache_mask(cpu));
+ } else {
+ mask = &per_cpu(thread_group_l2_cache_map, cpu);
+ }
+
+ for_each_cpu(i, *mask) {
+ if (!cpu_online(i))
+ continue;
+ set_cpus_related(i, cpu, cpu_l2_cache_mask);
+ }
+
+ return true;
+ }
+
l2_cache = cpu_to_l2cache(cpu);
if (!l2_cache || !*mask) {
/* Assume only core siblings share cache with this CPU */
--
1.9.4
^ permalink raw reply related
* [PATCH 3/3] powerpc/cacheinfo: Print correct cache-sibling map/list for L2 cache
From: Gautham R. Shenoy @ 2020-12-04 4:48 UTC (permalink / raw)
To: Srikar Dronamraju, Anton Blanchard, Vaidyanathan Srinivasan,
Michael Ellerman, Michael Neuling, Nicholas Piggin, Nathan Lynch,
Peter Zijlstra, Valentin Schneider
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
In-Reply-To: <1607057327-29822-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
On POWER platforms where only some groups of threads within a core
share the L2-cache (indicated by the ibm,thread-groups device-tree
property), we currently print the incorrect shared_cpu_map/list for
L2-cache in the sysfs.
This patch reports the correct shared_cpu_map/list on such platforms.
Example:
On a platform with "ibm,thread-groups" set to
00000001 00000002 00000004 00000000
00000002 00000004 00000006 00000001
00000003 00000005 00000007 00000002
00000002 00000004 00000000 00000002
00000004 00000006 00000001 00000003
00000005 00000007
This indicates that threads {0,2,4,6} in the core share the L2-cache
and threads {1,3,5,7} in the core share the L2 cache.
However, without the patch, the shared_cpu_map/list for L2 for CPUs 0,
1 is reported in the sysfs as follows:
/sys/devices/system/cpu/cpu0/cache/index2/shared_cpu_list:0-7
/sys/devices/system/cpu/cpu0/cache/index2/shared_cpu_map:000000,000000ff
/sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_list:0-7
/sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map:000000,000000ff
With the patch, the shared_cpu_map/list for L2 cache for CPUs 0, 1 is
correctly reported as follows:
/sys/devices/system/cpu/cpu0/cache/index2/shared_cpu_list:0,2,4,6
/sys/devices/system/cpu/cpu0/cache/index2/shared_cpu_map:000000,00000055
/sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_list:1,3,5,7
/sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map:000000,000000aa
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/kernel/cacheinfo.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index 65ab9fc..1cc8f37 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -651,15 +651,22 @@ static unsigned int index_dir_to_cpu(struct cache_index_dir *index)
return dev->id;
}
+extern bool thread_group_shares_l2;
/*
* On big-core systems, each core has two groups of CPUs each of which
* has its own L1-cache. The thread-siblings which share l1-cache with
* @cpu can be obtained via cpu_smallcore_mask().
+ *
+ * On some big-core systems, the L2 cache is shared only between some
+ * groups of siblings. This is already parsed and encoded in
+ * cpu_l2_cache_mask().
*/
static const struct cpumask *get_big_core_shared_cpu_map(int cpu, struct cache *cache)
{
if (cache->level == 1)
return cpu_smallcore_mask(cpu);
+ if (cache->level == 2 && thread_group_shares_l2)
+ return cpu_l2_cache_mask(cpu);
return &cache->shared_cpu_map;
}
--
1.9.4
^ permalink raw reply related
* [PATCH 1/3] powerpc/smp: Parse ibm, thread-groups with multiple properties
From: Gautham R. Shenoy @ 2020-12-04 4:48 UTC (permalink / raw)
To: Srikar Dronamraju, Anton Blanchard, Vaidyanathan Srinivasan,
Michael Ellerman, Michael Neuling, Nicholas Piggin, Nathan Lynch,
Peter Zijlstra, Valentin Schneider
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel
In-Reply-To: <1607057327-29822-1-git-send-email-ego@linux.vnet.ibm.com>
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
The "ibm,thread-groups" device-tree property is an array that is used
to indicate if groups of threads within a core share certain
properties. It provides details of which property is being shared by
which groups of threads. This array can encode information about
multiple properties being shared by different thread-groups within the
core.
Example: Suppose,
"ibm,thread-groups" = [1,2,4,8,10,12,14,9,11,13,15,2,2,4,8,10,12,14,9,11,13,15]
This can be decomposed up into two consecutive arrays:
a) [1,2,4,8,10,12,14,9,11,13,15]
b) [2,2,4,8,10,12,14,9,11,13,15]
where in,
a) provides information of Property "1" being shared by "2" groups,
each with "4" threads each. The "ibm,ppc-interrupt-server#s" of the
first group is {8,10,12,14} and the "ibm,ppc-interrupt-server#s" of
the second group is {9,11,13,15}. Property "1" is indicative of
the thread in the group sharing L1 cache, translation cache and
Instruction Data flow.
b) provides information of Property "2" being shared by "2" groups,
each group with "4" threads. The "ibm,ppc-interrupt-server#s" of
the first group is {8,10,12,14} and the
"ibm,ppc-interrupt-server#s" of the second group is
{9,11,13,15}. Property "2" indicates that the threads in each group
share the L2-cache.
The existing code assumes that the "ibm,thread-groups" encodes
information about only one property. Hence even on platforms which
encode information about multiple properties being shared by the
corresponding groups of threads, the current code will only pick the
first one. (In the above example, it will only consider
[1,2,4,8,10,12,14,9,11,13,15] but not [2,2,4,8,10,12,14,9,11,13,15]).
This patch extends the parsing support on platforms which encode
information about multiple properties being shared by the
corresponding groups of threads.
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
arch/powerpc/kernel/smp.c | 146 +++++++++++++++++++++++++++++-----------------
1 file changed, 92 insertions(+), 54 deletions(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 8c2857c..6a242a3 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -106,6 +106,15 @@ struct thread_groups {
unsigned int thread_list[MAX_THREAD_LIST_SIZE];
};
+/* Maximum number of properties that groups of threads within a core can share */
+#define MAX_THREAD_GROUP_PROPERTIES 1
+
+struct thread_groups_list {
+ unsigned int nr_properties;
+ struct thread_groups property_tgs[MAX_THREAD_GROUP_PROPERTIES];
+};
+
+static struct thread_groups_list tgl[NR_CPUS] __initdata;
/*
* On big-cores system, cpu_l1_cache_map for each CPU corresponds to
* the set its siblings that share the L1-cache.
@@ -695,81 +704,94 @@ static void or_cpumasks_related(int i, int j, struct cpumask *(*srcmask)(int),
/*
* parse_thread_groups: Parses the "ibm,thread-groups" device tree
* property for the CPU device node @dn and stores
- * the parsed output in the thread_groups
- * structure @tg if the ibm,thread-groups[0]
- * matches @property.
+ * the parsed output in the thread_groups_list
+ * structure @tglp.
*
* @dn: The device node of the CPU device.
- * @tg: Pointer to a thread group structure into which the parsed
+ * @tglp: Pointer to a thread group list structure into which the parsed
* output of "ibm,thread-groups" is stored.
- * @property: The property of the thread-group that the caller is
- * interested in.
*
* ibm,thread-groups[0..N-1] array defines which group of threads in
* the CPU-device node can be grouped together based on the property.
*
- * ibm,thread-groups[0] tells us the property based on which the
+ * This array can represent thread groupings for multiple properties.
+ *
+ * ibm,thread-groups[i + 0] tells us the property based on which the
* threads are being grouped together. If this value is 1, it implies
* that the threads in the same group share L1, translation cache.
*
- * ibm,thread-groups[1] tells us how many such thread groups exist.
+ * ibm,thread-groups[i+1] tells us how many such thread groups exist for the
+ * property ibm,thread-groups[i]
*
- * ibm,thread-groups[2] tells us the number of threads in each such
+ * ibm,thread-groups[i+2] tells us the number of threads in each such
* group.
+ * Suppose k = (ibm,thread-groups[i+1] * ibm,thread-groups[i+2]), then,
*
- * ibm,thread-groups[3..N-1] is the list of threads identified by
+ * ibm,thread-groups[i+3..i+k+2] (is the list of threads identified by
* "ibm,ppc-interrupt-server#s" arranged as per their membership in
* the grouping.
*
- * Example: If ibm,thread-groups = [1,2,4,5,6,7,8,9,10,11,12] it
- * implies that there are 2 groups of 4 threads each, where each group
- * of threads share L1, translation cache.
+ * Example:
+ * If ibm,thread-groups = [1,2,4,5,6,7,8,9,10,11,12,2,2,4,5,7,9,11,6,8,10,12]
+ * This can be decomposed up into two consecutive arrays:
+ * a) [1,2,4,5,6,7,8,9,10,11,12]
+ * b) [2,2,4,5,7,9,11,6,8,10,12]
+ * where in,
*
- * The "ibm,ppc-interrupt-server#s" of the first group is {5,6,7,8}
- * and the "ibm,ppc-interrupt-server#s" of the second group is {9, 10,
- * 11, 12} structure
+ * a) there are 2 groups of 4 threads each, where each group of
+ * threads share Property 1 (L1, translation cache). The
+ * "ibm,ppc-interrupt-server#s" of the first group is {5,6,7,8} and
+ * the "ibm,ppc-interrupt-server#s" of the second group is {9, 10, 11,
+ * 12}.
+ *
+ * b) there are 2 groups of 4 threads each, where each group of
+ * threads share some property indicated by the first value 2. The
+ * "ibm,ppc-interrupt-server#s" of the first group is {5,7,9,11}
+ * and the "ibm,ppc-interrupt-server#s" of the second group is
+ * {6,8,10,12} structure
*
* Returns 0 on success, -EINVAL if the property does not exist,
* -ENODATA if property does not have a value, and -EOVERFLOW if the
* property data isn't large enough.
*/
static int parse_thread_groups(struct device_node *dn,
- struct thread_groups *tg,
- unsigned int property)
+ struct thread_groups_list *tglp)
{
- int i;
- u32 thread_group_array[3 + MAX_THREAD_LIST_SIZE];
+ int i = 0;
+ u32 *thread_group_array;
u32 *thread_list;
size_t total_threads;
- int ret;
+ int ret = 0, count;
+ unsigned int property_idx = 0;
+ count = of_property_count_u32_elems(dn, "ibm,thread-groups");
+ thread_group_array = kcalloc(count, sizeof(u32), GFP_KERNEL);
ret = of_property_read_u32_array(dn, "ibm,thread-groups",
- thread_group_array, 3);
+ thread_group_array, count);
if (ret)
- return ret;
-
- tg->property = thread_group_array[0];
- tg->nr_groups = thread_group_array[1];
- tg->threads_per_group = thread_group_array[2];
- if (tg->property != property ||
- tg->nr_groups < 1 ||
- tg->threads_per_group < 1)
- return -ENODATA;
+ goto out_free;
- total_threads = tg->nr_groups * tg->threads_per_group;
+ while (i < count && property_idx < MAX_THREAD_GROUP_PROPERTIES) {
+ int j;
+ struct thread_groups *tg = &tglp->property_tgs[property_idx++];
- ret = of_property_read_u32_array(dn, "ibm,thread-groups",
- thread_group_array,
- 3 + total_threads);
- if (ret)
- return ret;
+ tg->property = thread_group_array[i];
+ tg->nr_groups = thread_group_array[i + 1];
+ tg->threads_per_group = thread_group_array[i + 2];
+ total_threads = tg->nr_groups * tg->threads_per_group;
+
+ thread_list = &thread_group_array[i + 3];
- thread_list = &thread_group_array[3];
+ for (j = 0; j < total_threads; j++)
+ tg->thread_list[j] = thread_list[j];
+ i = i + 3 + total_threads;
+ }
- for (i = 0 ; i < total_threads; i++)
- tg->thread_list[i] = thread_list[i];
+ tglp->nr_properties = property_idx;
- return 0;
+out_free:
+ kfree(thread_group_array);
+ return ret;
}
/*
@@ -805,24 +827,39 @@ static int get_cpu_thread_group_start(int cpu, struct thread_groups *tg)
return -1;
}
-static int init_cpu_l1_cache_map(int cpu)
+static int init_cpu_cache_map(int cpu, unsigned int cache_property)
{
struct device_node *dn = of_get_cpu_node(cpu, NULL);
- struct thread_groups tg = {.property = 0,
- .nr_groups = 0,
- .threads_per_group = 0};
+ struct thread_groups *tg = NULL;
int first_thread = cpu_first_thread_sibling(cpu);
int i, cpu_group_start = -1, err = 0;
+ cpumask_var_t *mask;
+ struct thread_groups_list *cpu_tgl = &tgl[cpu];
if (!dn)
return -ENODATA;
- err = parse_thread_groups(dn, &tg, THREAD_GROUP_SHARE_L1);
- if (err)
- goto out;
+ if (!(cache_property == THREAD_GROUP_SHARE_L1))
+ return -EINVAL;
- cpu_group_start = get_cpu_thread_group_start(cpu, &tg);
+ if (!cpu_tgl->nr_properties) {
+ err = parse_thread_groups(dn, cpu_tgl);
+ if (err)
+ goto out;
+ }
+
+ for (i = 0; i < cpu_tgl->nr_properties; i++) {
+ if (cpu_tgl->property_tgs[i].property == cache_property) {
+ tg = &cpu_tgl->property_tgs[i];
+ break;
+ }
+ }
+
+ if (!tg)
+ return -EINVAL;
+
+ cpu_group_start = get_cpu_thread_group_start(cpu, tg);
if (unlikely(cpu_group_start == -1)) {
WARN_ON_ONCE(1);
@@ -830,11 +867,12 @@ static int init_cpu_l1_cache_map(int cpu)
goto out;
}
- zalloc_cpumask_var_node(&per_cpu(cpu_l1_cache_map, cpu),
- GFP_KERNEL, cpu_to_node(cpu));
+ mask = &per_cpu(cpu_l1_cache_map, cpu);
+
+ zalloc_cpumask_var_node(mask, GFP_KERNEL, cpu_to_node(cpu));
for (i = first_thread; i < first_thread + threads_per_core; i++) {
- int i_group_start = get_cpu_thread_group_start(i, &tg);
+ int i_group_start = get_cpu_thread_group_start(i, tg);
if (unlikely(i_group_start == -1)) {
WARN_ON_ONCE(1);
@@ -843,7 +881,7 @@ static int init_cpu_l1_cache_map(int cpu)
}
if (i_group_start == cpu_group_start)
- cpumask_set_cpu(i, per_cpu(cpu_l1_cache_map, cpu));
+ cpumask_set_cpu(i, *mask);
}
out:
@@ -924,7 +962,7 @@ static int init_big_cores(void)
int cpu;
for_each_possible_cpu(cpu) {
- int err = init_cpu_l1_cache_map(cpu);
+ int err = init_cpu_cache_map(cpu, THREAD_GROUP_SHARE_L1);
if (err)
return err;
--
1.9.4
^ permalink raw reply related
* [RFC v2 0/2] lazy mm refcounting
From: Andy Lutomirski @ 2020-12-04 5:26 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linux-arch, Nadav Amit, Arnd Bergmann, Rik van Riel, Will Deacon,
X86 ML, Dave Hansen, LKML, Linux-MM, Mathieu Desnoyers,
Andy Lutomirski, Catalin Marinas, Jann Horn, linuxppc-dev
This is part of a larger series here, but the beginning bit is irrelevant
to the current discussion:
https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/mm&id=203d39d11562575fd8bd6a094d97a3a332d8b265
This is IMO a lot better than v1. It's now almost entirely in generic
code. (It looks like it's 100% generic, but that's a lie -- the
generic code currently that all possible lazy mm refs are in
mm_cpumask(), and that's not true on all arches. So, if we take my
approach, we'll need to have a little arch hook to control this.)
Here's how I think it fits with various arches:
x86: On bare metal (i.e. paravirt flush unavailable), the loop won't do
much. The existing TLB shootdown when user tables are freed will
empty mm_cpumask of everything but the calling CPU. So x86 ends up
pretty close to as good as we can get short of reworking mm_cpumask() itself.
arm64: It needs the fixup above for correctness, but I think performance
should be pretty good. Compared to current kernels, we lose an mmgrab()
and mmdrop() on each lazy transition, and we add a reasonably fast loop
over all cpus on process exit. Someone (probably me) needs to make
sure we don't need some extra barriers.
power: Similar to x86.
s390x: Should be essentially the same as arm64.
Other arches: I don't know. Further research is required.
What do you all think?
Andy Lutomirski (2):
[NEEDS HELP] x86/mm: Handle unlazying membarrier core sync in the arch
code
[MOCKUP] sched/mm: Lightweight lazy mm refcounting
arch/x86/mm/tlb.c | 17 +++++-
kernel/fork.c | 4 ++
kernel/sched/core.c | 134 +++++++++++++++++++++++++++++++++++++------
kernel/sched/sched.h | 11 +++-
4 files changed, 145 insertions(+), 21 deletions(-)
--
2.28.0
^ permalink raw reply
* [RFC v2 1/2] [NEEDS HELP] x86/mm: Handle unlazying membarrier core sync in the arch code
From: Andy Lutomirski @ 2020-12-04 5:26 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linux-arch, Nadav Amit, Arnd Bergmann, Rik van Riel, Will Deacon,
X86 ML, Dave Hansen, LKML, Linux-MM, Mathieu Desnoyers,
Andy Lutomirski, Catalin Marinas, Jann Horn, linuxppc-dev
In-Reply-To: <cover.1607059162.git.luto@kernel.org>
The core scheduler isn't a great place for
membarrier_mm_sync_core_before_usermode() -- the core scheduler doesn't
actually know whether we are lazy. With the old code, if a CPU is
running a membarrier-registered task, goes idle, gets unlazied via a TLB
shootdown IPI, and switches back to the membarrier-registered task, it
will do an unnecessary core sync.
Conveniently, x86 is the only architecture that does anything in this
hook, so we can just move the code.
XXX: there are some comments in swich_mm_irqs_off() that seem to be
trying to document what barriers are expected, and it's not clear to me
that these barriers are actually present in all paths through the
code. So I think this change makes the code more comprehensible and
has no effect on the code's correctness, but I'm not at all convinced
that the code is correct.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/mm/tlb.c | 17 ++++++++++++++++-
kernel/sched/core.c | 14 +++++++-------
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 3338a1feccf9..23df035b80e8 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -8,6 +8,7 @@
#include <linux/export.h>
#include <linux/cpu.h>
#include <linux/debugfs.h>
+#include <linux/sched/mm.h>
#include <asm/tlbflush.h>
#include <asm/mmu_context.h>
@@ -496,6 +497,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
* from one thread in a process to another thread in the same
* process. No TLB flush required.
*/
+
+ // XXX: why is this okay wrt membarrier?
if (!was_lazy)
return;
@@ -508,12 +511,24 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
smp_mb();
next_tlb_gen = atomic64_read(&next->context.tlb_gen);
if (this_cpu_read(cpu_tlbstate.ctxs[prev_asid].tlb_gen) ==
- next_tlb_gen)
+ next_tlb_gen) {
+ /*
+ * We're reactivating an mm, and membarrier might
+ * need to serialize. Tell membarrier.
+ */
+
+ // XXX: I can't understand the logic in
+ // membarrier_mm_sync_core_before_usermode(). What's
+ // the mm check for?
+ membarrier_mm_sync_core_before_usermode(next);
return;
+ }
/*
* TLB contents went out of date while we were in lazy
* mode. Fall through to the TLB switching code below.
+ * No need for an explicit membarrier invocation -- the CR3
+ * write will serialize.
*/
new_asid = prev_asid;
need_flush = true;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2d95dc3f4644..6c4b76147166 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3619,22 +3619,22 @@ static struct rq *finish_task_switch(struct task_struct *prev)
kcov_finish_switch(current);
fire_sched_in_preempt_notifiers(current);
+
/*
* When switching through a kernel thread, the loop in
* membarrier_{private,global}_expedited() may have observed that
* kernel thread and not issued an IPI. It is therefore possible to
* schedule between user->kernel->user threads without passing though
* switch_mm(). Membarrier requires a barrier after storing to
- * rq->curr, before returning to userspace, so provide them here:
+ * rq->curr, before returning to userspace, and mmdrop() provides
+ * this barrier.
*
- * - a full memory barrier for {PRIVATE,GLOBAL}_EXPEDITED, implicitly
- * provided by mmdrop(),
- * - a sync_core for SYNC_CORE.
+ * XXX: I don't think mmdrop() actually does this. There's no
+ * smp_mb__before/after_atomic() in there.
*/
- if (mm) {
- membarrier_mm_sync_core_before_usermode(mm);
+ if (mm)
mmdrop(mm);
- }
+
if (unlikely(prev_state == TASK_DEAD)) {
if (prev->sched_class->task_dead)
prev->sched_class->task_dead(prev);
--
2.28.0
^ permalink raw reply related
* [RFC v2 2/2] [MOCKUP] sched/mm: Lightweight lazy mm refcounting
From: Andy Lutomirski @ 2020-12-04 5:26 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linux-arch, Nadav Amit, Arnd Bergmann, Rik van Riel, Will Deacon,
X86 ML, Dave Hansen, LKML, Linux-MM, Mathieu Desnoyers,
Andy Lutomirski, Catalin Marinas, Jann Horn, linuxppc-dev
In-Reply-To: <cover.1607059162.git.luto@kernel.org>
This is a mockup. It's designed to illustrate the algorithm and how the
code might be structured. There are several things blatantly wrong with
it:
The coding stype is not up to kernel standards. I have prototypes in the
wrong places and other hacks.
There's a problem with mm_cpumask() not being reliable.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
kernel/fork.c | 4 ++
kernel/sched/core.c | 128 +++++++++++++++++++++++++++++++++++++------
kernel/sched/sched.h | 11 +++-
3 files changed, 126 insertions(+), 17 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index da8d360fb032..0887a33cf84f 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1066,6 +1066,8 @@ struct mm_struct *mm_alloc(void)
return mm_init(mm, current, current_user_ns());
}
+extern void mm_fixup_lazy_refs(struct mm_struct *mm);
+
static inline void __mmput(struct mm_struct *mm)
{
VM_BUG_ON(atomic_read(&mm->mm_users));
@@ -1084,6 +1086,8 @@ static inline void __mmput(struct mm_struct *mm)
}
if (mm->binfmt)
module_put(mm->binfmt->module);
+
+ mm_fixup_lazy_refs(mm);
mmdrop(mm);
}
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 6c4b76147166..69dfdfe0e5b4 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3555,6 +3555,75 @@ prepare_task_switch(struct rq *rq, struct task_struct *prev,
prepare_arch_switch(next);
}
+static void drop_extra_mm_refs(struct rq *rq)
+{
+ unsigned long old_mm;
+
+ if (likely(!atomic_long_read(&rq->mm_to_mmdrop)))
+ return;
+
+ /*
+ * Slow path. This only happens when we recently stopped using
+ * an mm that is exiting.
+ */
+ old_mm = atomic_long_xchg_relaxed(&rq->mm_to_mmdrop, 0);
+ if (old_mm)
+ mmdrop((struct mm_struct *)old_mm);
+}
+
+/*
+ * This ensures that all lazy_mm refs to mm are converted to mm_count
+ * refcounts. Our caller holds an mm_count reference, so we don't need
+ * to worry about mm being freed out from under us.
+ */
+void mm_fixup_lazy_refs(struct mm_struct *mm)
+{
+ int cpu;
+
+ /*
+ * mm_users is zero, so no new lazy refs will be taken.
+ */
+ WARN_ON_ONCE(atomic_read(&mm->mm_users) != 0);
+
+ /*
+ * XXX: this is wrong on arm64 and possibly on other architectures.
+ * Maybe we need a config option for this? Or a
+ * for_each_possible_lazy_cpu(cpu, mm) helper?
+ */
+ for_each_cpu(cpu, mm_cpumask(mm)) {
+ struct rq *rq = cpu_rq(cpu);
+ unsigned long old;
+
+ if (READ_ONCE(rq->lazy_mm) != mm)
+ continue;
+
+ // XXX: we could optimize this by doing a big addition to
+ // mm_count up front instead of incrementing it separately
+ // for each CPU.
+ mmgrab(mm);
+
+ // XXX: could this be relaxed instead?
+ old = atomic_long_xchg(&rq->mm_to_mmdrop, (unsigned long)mm);
+
+ // At this point, mm can be mmdrop()ed at any time, probably
+ // by the target cpu.
+
+ if (!old)
+ continue; // All done!
+
+ WARN_ON_ONCE(old == (unsigned long)mm);
+
+ // Uh oh! We just stole an mm reference from the target CPU.
+ // Fortunately, we just observed the target's lazy_mm pointing
+ // to something other than old, and we observed this after
+ // bringing mm_users down to 0. This means that the remote
+ // cpu is definitely done with old. So we can drop it on the
+ // remote CPU's behalf.
+
+ mmdrop((struct mm_struct *)old);
+ }
+}
+
/**
* finish_task_switch - clean up after a task-switch
* @prev: the thread we just switched away from.
@@ -3578,7 +3647,6 @@ static struct rq *finish_task_switch(struct task_struct *prev)
__releases(rq->lock)
{
struct rq *rq = this_rq();
- struct mm_struct *mm = rq->prev_mm;
long prev_state;
/*
@@ -3597,8 +3665,6 @@ static struct rq *finish_task_switch(struct task_struct *prev)
current->comm, current->pid, preempt_count()))
preempt_count_set(FORK_PREEMPT_COUNT);
- rq->prev_mm = NULL;
-
/*
* A task struct has one reference for the use as "current".
* If a task dies, then it sets TASK_DEAD in tsk->state and calls
@@ -3629,11 +3695,28 @@ static struct rq *finish_task_switch(struct task_struct *prev)
* rq->curr, before returning to userspace, and mmdrop() provides
* this barrier.
*
- * XXX: I don't think mmdrop() actually does this. There's no
- * smp_mb__before/after_atomic() in there.
+ * XXX: I don't think mmdrop() actually did this. There's no
+ * smp_mb__before/after_atomic() in there. But mmdrop()
+ * certainly doesn't do this now, since we don't call mmdrop().
*/
- if (mm)
- mmdrop(mm);
+ if (current->mm && rq->lazy_mm) {
+ /*
+ * We are unlazying. Any remote CPU that observes our
+ * store to lazy_mm is permitted to free the mm if mm_users
+ * and mm_count are both zero.
+ */
+ WRITE_ONCE(rq->lazy_mm, NULL);
+ }
+
+ // Do this unconditionally. There's a race in which a remote CPU
+ // sees rq->lazy_mm != NULL and gives us an extra mm ref while we
+ // are executing this code and we don't notice. Instead of letting
+ // that ref sit around until the next time we unlazy, do it on every
+ // context switch.
+ //
+ // XXX: maybe we should do this at the beginning of a context switch
+ // instead?
+ drop_extra_mm_refs(rq);
if (unlikely(prev_state == TASK_DEAD)) {
if (prev->sched_class->task_dead)
@@ -3737,20 +3820,31 @@ context_switch(struct rq *rq, struct task_struct *prev,
arch_start_context_switch(prev);
/*
- * kernel -> kernel lazy + transfer active
- * user -> kernel lazy + mmgrab() active
+ * TODO: write a new comment!
*
- * kernel -> user switch + mmdrop() active
- * user -> user switch
+ * NB: none of this is kept in sync with the arch code.
+ * In particular, active_mm can point to an mm that is no longer
+ * in use by the arch mm code, and this condition can persist
+ * across multiple context switches. This isn't a problem
+ * per se, but it does mean that using active_mm for anything
+ * other than keeping an mm from being freed is a bit dubious.
*/
if (!next->mm) { // to kernel
enter_lazy_tlb(prev->active_mm, next);
next->active_mm = prev->active_mm;
- if (prev->mm) // from user
- mmgrab(prev->active_mm);
- else
+ if (prev->mm) { // from user
+ WARN_ON_ONCE(rq->lazy_mm);
+ WRITE_ONCE(rq->lazy_mm, next->active_mm);
+ /*
+ * barrier here? this needs to be visible to any
+ * remote CPU that starts executing __mmput(). That
+ * can't happen until either we call mmput() or until
+ * prev migrates elsewhere.
+ */
+ } else {
prev->active_mm = NULL;
+ }
} else { // to user
membarrier_switch_mm(rq, prev->active_mm, next->mm);
/*
@@ -3760,12 +3854,14 @@ context_switch(struct rq *rq, struct task_struct *prev,
* The below provides this either through switch_mm(), or in
* case 'prev->active_mm == next->mm' through
* finish_task_switch()'s mmdrop().
+ *
+ * XXX: mmdrop() didn't do this before, and the new
+ * code doesn't even call mmdrop().
*/
switch_mm_irqs_off(prev->active_mm, next->mm, next);
if (!prev->mm) { // from kernel
- /* will mmdrop() in finish_task_switch(). */
- rq->prev_mm = prev->active_mm;
+ /* will release lazy_mm in finish_task_switch(). */
prev->active_mm = NULL;
}
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 28709f6b0975..e0caee5f158e 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -950,7 +950,16 @@ struct rq {
struct task_struct *idle;
struct task_struct *stop;
unsigned long next_balance;
- struct mm_struct *prev_mm;
+
+ /*
+ * Hazard pointer for an mm that we might be using lazily.
+ */
+ struct mm_struct *lazy_mm;
+
+ /*
+ * An mm that needs mmdrop()ing.
+ */
+ atomic_long_t mm_to_mmdrop;
unsigned int clock_update_flags;
u64 clock;
--
2.28.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox