(Sorry for so many list cross-posting and big cc)
Please help testing !
The invalidate_page callback suffered from 2 pitfalls. First it used to
happen after page table lock was release and thus a new page might have
been setup for the virtual address before the call to invalidate_page().
This is in a weird way fixed by c7ab0d2fdc840266b39db94538f74207ec2afbf6
which moved the callback under the page table lock. Which also broke
several existing user of the mmu_notifier API that assumed they could
sleep inside this callback.
The second pitfall was invalidate_page being the only callback not taking
a range of address in respect to invalidation but was giving an address
and a page. Lot of the callback implementer assumed this could never be
THP and thus failed to invalidate the appropriate range for THP pages.
By killing this callback we unify the mmu_notifier callback API to always
take a virtual address range as input.
There is now 2 clear API (I am not mentioning the youngess API which is
seldomly used):
- invalidate_range_start()/end() callback (which allow you to sleep)
- invalidate_range() where you can not sleep but happen right after
page table update under page table lock
Note that a lot of existing user feels broken in respect to range_start/
range_end. Many user only have range_start() callback but there is nothing
preventing them to undo what was invalidated in their range_start() callback
after it returns but before any CPU page table update take place.
The code pattern use in kvm or umem odp is an example on how to properly
avoid such race. In a nutshell use some kind of sequence number and active
range invalidation counter to block anything that might undo what the
range_start() callback did.
If you do not care about keeping fully in sync with CPU page table (ie
you can live with CPU page table pointing to new different page for a
given virtual address) then you can take a reference on the pages inside
the range_start callback and drop it in range_end or when your driver
is done with those pages.
Last alternative is to use invalidate_range() if you can do invalidation
without sleeping as invalidate_range() callback happens under the CPU
page table spinlock right after the page table is updated.
Note this is barely tested. I intend to do more testing of next few days
but i do not have access to all hardware that make use of the mmu_notifier
API.
First 2 patches convert existing call of mmu_notifier_invalidate_page()
to mmu_notifier_invalidate_range() and bracket those call with call to
mmu_notifier_invalidate_range_start()/end().
The next 10 patches remove existing invalidate_page() callback as it can
no longer happen.
Finaly the last page remove it completely so it can RIP.
Jérôme Glisse (13):
dax: update to new mmu_notifier semantic
mm/rmap: update to new mmu_notifier semantic
powerpc/powernv: update to new mmu_notifier semantic
drm/amdgpu: update to new mmu_notifier semantic
IB/umem: update to new mmu_notifier semantic
IB/hfi1: update to new mmu_notifier semantic
iommu/amd: update to new mmu_notifier semantic
iommu/intel: update to new mmu_notifier semantic
misc/mic/scif: update to new mmu_notifier semantic
sgi-gru: update to new mmu_notifier semantic
xen/gntdev: update to new mmu_notifier semantic
KVM: update to new mmu_notifier semantic
mm/mmu_notifier: kill invalidate_page
Cc: Kirill A. Shutemov <redacted>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrea Arcangeli <redacted>
Cc: Joerg Roedel <redacted>
Cc: Dan Williams <redacted>
Cc: Sudeep Dutt <redacted>
Cc: Ashutosh Dixit <redacted>
Cc: Dimitri Sivanich <redacted>
Cc: Jack Steiner <redacted>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <redacted>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: dri-devel@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-rdma@vger.kernel.org
Cc: iommu@lists.linux-foundation.org
Cc: xen-devel@lists.xenproject.org
Cc: kvm@vger.kernel.org
arch/powerpc/platforms/powernv/npu-dma.c | 10 --------
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 31 ----------------------
drivers/infiniband/core/umem_odp.c | 19 --------------
drivers/infiniband/hw/hfi1/mmu_rb.c | 9 -------
drivers/iommu/amd_iommu_v2.c | 8 ------
drivers/iommu/intel-svm.c | 9 -------
drivers/misc/mic/scif/scif_dma.c | 11 --------
drivers/misc/sgi-gru/grutlbpurge.c | 12 ---------
drivers/xen/gntdev.c | 8 ------
fs/dax.c | 19 ++++++++------
include/linux/mm.h | 1 +
include/linux/mmu_notifier.h | 25 ------------------
mm/memory.c | 26 +++++++++++++++----
mm/mmu_notifier.c | 14 ----------
mm/rmap.c | 44 +++++++++++++++++++++++++++++---
virt/kvm/kvm_main.c | 42 ------------------------------
16 files changed, 74 insertions(+), 214 deletions(-)
--
2.13.5
Call to mmu_notifier_invalidate_page() are replaced by call to
mmu_notifier_invalidate_range() and thus call are bracketed by
call to mmu_notifier_invalidate_range_start()/end()
Remove now useless invalidate_page callback.
Signed-off-by: Jérôme Glisse <redacted>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Alistair Popple <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Kirill A. Shutemov <redacted>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrea Arcangeli <redacted>
---
arch/powerpc/platforms/powernv/npu-dma.c | 10 ----------
1 file changed, 10 deletions(-)
On Tue, Aug 29, 2017 at 4:54 PM, J=C3=A9r=C3=B4me Glisse <jglisse@redhat.co=
m> wrote:
Note this is barely tested. I intend to do more testing of next few days
but i do not have access to all hardware that make use of the mmu_notifie=
r
API.
Thanks for doing this.
First 2 patches convert existing call of mmu_notifier_invalidate_page()
to mmu_notifier_invalidate_range() and bracket those call with call to
mmu_notifier_invalidate_range_start()/end().
Ok, those two patches are a bit more complex than I was hoping for,
but not *too* bad.
And the final end result certainly looks nice:
Yeah, removing all those invalidate_page() notifiers certainly makes
for a nice patch.
And I actually think you missed some more lines that can now be
removed: kvm_arch_mmu_notifier_invalidate_page() should no longer be
needed either, so you can remove all of those too (most of them are
empty inline functions, but x86 has one that actually does something.
So there's an added 30 or so dead lines that should be removed in the
kvm patch, I think.
But from a _very_ quick read-through this looks fine. But it obviously
needs testing.
People - *especially* the people who saw issues under KVM - can you
try out J=C3=A9r=C3=B4me's patch-series? I aded some people to the cc, the =
full
series is on lkml. J=C3=A9r=C3=B4me - do you have a git branch for people t=
o
test that they could easily pull and try out?
Linus
On Tue, Aug 29, 2017 at 05:11:24PM -0700, Linus Torvalds wrote:
On Tue, Aug 29, 2017 at 4:54 PM, Jérôme Glisse [off-list ref] wrote:
quoted
Note this is barely tested. I intend to do more testing of next few days
but i do not have access to all hardware that make use of the mmu_notifier
API.
Thanks for doing this.
quoted
First 2 patches convert existing call of mmu_notifier_invalidate_page()
to mmu_notifier_invalidate_range() and bracket those call with call to
mmu_notifier_invalidate_range_start()/end().
Ok, those two patches are a bit more complex than I was hoping for,
but not *too* bad.
And the final end result certainly looks nice:
Yeah, removing all those invalidate_page() notifiers certainly makes
for a nice patch.
And I actually think you missed some more lines that can now be
removed: kvm_arch_mmu_notifier_invalidate_page() should no longer be
needed either, so you can remove all of those too (most of them are
empty inline functions, but x86 has one that actually does something.
So there's an added 30 or so dead lines that should be removed in the
kvm patch, I think.
Yes i missed that. I will wait for people to test and for result of my
own test before reposting if need be, otherwise i will post as separate
patch.
But from a _very_ quick read-through this looks fine. But it obviously
needs testing.
People - *especially* the people who saw issues under KVM - can you
try out Jérôme's patch-series? I aded some people to the cc, the full
series is on lkml. Jérôme - do you have a git branch for people to
test that they could easily pull and try out?
https://cgit.freedesktop.org/~glisse/linux mmu-notifier branch
git://people.freedesktop.org/~glisse/linux
(Sorry if that tree is bit big it has a lot of dead thing i need
to push a clean and slim one)
Jérôme
Looks good here.
I reproduced fairly quickly with RT host and 1 RT guest by just having
the guest do a parallel kbuild over NFS (the guest had to be restored
afterward, was corrupted). =C2=A0I'm currently flogging 2 guests as well as
the host, whimper free. =C2=A0I'll let the lot broil for while longer, but
at this point, smoke/flame appearance seems comfortingly unlikely.
-Mike
From: Adam Borowski <hidden> Date: 2017-08-30 14:58:19
On Tue, Aug 29, 2017 at 08:56:15PM -0400, Jerome Glisse wrote:
I will wait for people to test and for result of my own test before
reposting if need be, otherwise i will post as separate patch.
quoted
But from a _very_ quick read-through this looks fine. But it obviously
needs testing.
People - *especially* the people who saw issues under KVM - can you
try out Jérôme's patch-series? I aded some people to the cc, the full
series is on lkml. Jérôme - do you have a git branch for people to
test that they could easily pull and try out?
Tested your branch as of 10f07641, on a long list of guest VMs.
No earth-shattering kaboom.
Meow!
--
⢀⣴⠾⠻⢶⣦⠀
⣾⠁⢰⠒⠀⣿⡁ Vat kind uf sufficiently advanced technology iz dis!?
⢿⡄⠘⠷⠚⠋⠀ -- Genghis Ht'rok'din
⠈⠳⣄⠀⠀⠀⠀
From: Jeff Cook <hidden> Date: 2017-09-01 14:47:32
On Wed, Aug 30, 2017, at 10:57 AM, Adam Borowski wrote:
On Tue, Aug 29, 2017 at 08:56:15PM -0400, Jerome Glisse wrote:
quoted
I will wait for people to test and for result of my own test before
reposting if need be, otherwise i will post as separate patch.
quoted
But from a _very_ quick read-through this looks fine. But it obviously
needs testing.
=20
People - *especially* the people who saw issues under KVM - can you
try out J=C3=A9r=C3=B4me's patch-series? I aded some people to the cc=
, the full
quoted
quoted
series is on lkml. J=C3=A9r=C3=B4me - do you have a git branch for pe=
taskboxtester@gmail.com liked your message with Boxer for Android.
On Sep 1, 2017 10:48 AM, Jeff Cook [off-list ref] wrote:
On Wed, Aug 30, 2017, at 10:57 AM, Adam Borowski wrote:
On Tue, Aug 29, 2017 at 08:56:15PM -0400, Jerome Glisse wrote:
quoted
I will wait for people to test and for result of my own test before
reposting if need be, otherwise i will post as separate patch.
quoted
But from a _very_ quick read-through this looks fine. But it obviously
needs testing.
People - *especially* the people who saw issues under KVM - can you
try out Jérôme's patch-series? I aded some people to the cc, the full
series is on lkml. Jérôme - do you have a git branch for people to
test that they could easily pull and try out?
On Tue, Aug 29, 2017 at 07:54:34PM -0400, Jérôme Glisse wrote:
(Sorry for so many list cross-posting and big cc)
Ditto (trimmed it a bit already, feel free to limit replies as you see
fit).
Please help testing !
Kernels 4.13 and 4.14 (which both contain these patch series in its
final form) are affected by a bug triggering BSOD
(CRITICAL_STRUCTURE_CORRUPTION) in Windows 10/2016 VMs in Qemu under
certain conditions on certain hardware/microcode versions (see below for
details).
Testing this proved to be quite cumbersome, as only some systems are
affected and it took a while to find a semi-reliable test setup. Some
users reported that microcode updates made the problem disappear on some
affected systems[1].
Bisecting the 4.13 release cycle first pointed to
aac2fea94f7a3df8ad1eeb477eb2643f81fd5393 rmap: do not call mmu_notifier_invalidate_page() under ptl
as likely culprit (although it was not possible to bisect exactly down
to this commit).
It was reverted in 785373b4c38719f4af6775845df6be1dfaea120f after which
the symptoms disappeared until this series was merged, which contains
369ea8242c0fb5239b4ddf0dc568f694bd244de4 mm/rmap: update to new mmu_notifier semantic v2
We haven't bisected the individual commits of the series yet, but the
commit immediately preceding its merge exhibits no problems, while
everything after does. It is not known whether the bug is actually in
the series itself, or whether increasing the likelihood of triggering it
is just a side-effect. There is a similar report[2] concerning an
upgrade from 4.12.12 to 4.12.13, which does not contain this series in
any form AFAICT but might be worth another look as well.
Our test setup consists of the following:
CPU: Intel(R) Xeon(R) CPU D-1528 @ 1.90GHz (single socket)
Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat
pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb
rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology
nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx
est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid dca sse4_1 sse4_2 x2apic
movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm
3dnowprefetch cpuid_fault epb cat_l3 cdp_l3 intel_ppin intel_pt
tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 hle avx2
smep bmi2 erms invpcid rtm cqm rdt_a rdseed adx smap xsaveopt cqm_llc
cqm_occup_llc cqm_mbm_total cqm_mbm_local dtherm arat pln pts
microcode: 0x700000e
Mainboard: Supermicro X10SDV-6C+-TLN4F
RAM: 64G DDR4 [3]
Swap: 8G on an LV
Qemu: 2.9.1 with some patches on top ([4])
OS: Debian Stretch based (PVE 5.1)
KSM is enabled, but turning it off just increases the number of test
iterations needed to trigger the bug.
Kernel config: [5]
VMs:
A: Windows 2016 with virtio-blk disks, 6G RAM
B: Windows 10 with (virtual) IDE disks, 6G RAM
C: Debian Stretch, ~55G RAM
fio config:
[global]
thread=2
runtime=1800
[write1]
ioengine=windowsaio
sync=0
direct=0
bs=4k
size=30G
rw=randwrite
iodepth=16
[read1]
ioengine=windowsaio
sync=0
direct=0
bs=4k
size=30G
rw=randread
iodepth=16
Test run:
- Start all three VMs
- run 'stress-ng --vm-bytes 1G --vm 52 -t 6000' in VM C
- wait until swap is (almost) full and KSM starts to merge pages
- start fio in VM A and B
- stop stress-ng in VM C, power off VM C
- run 'swapoff -av' on host
- wait until swap content has been swapped in again (this takes a while)
- observe BSOD in at least one of A / B around 30% of the time
While this test case is pretty artifical, the BSOD issue does affect
users in the wild running regular work loads (where it can take from
multiple hours up to several days to trigger).
We have reverted this patch series in our 4.13 based kernel for now,
with positive feedback from users and our own testing. If more detailed
traces or data from a test run on an affected system is needed, we will
of course provide it.
Any further input / pointers are highly appreciated!
1: https://forum.proxmox.com/threads/blue-screen-with-5-1.37664/
2: http://www.spinics.net/lists/kvm/msg159179.htmlhttps://bugs.launchpad.net/qemu/+bug/1728256https://bugzilla.kernel.org/show_bug.cgi?id=197951
3: http://www.samsung.com/semiconductor/products/dram/server-dram/ddr4-registered-dimm/M393A2K40BB1?ia=2503
5: https://git.proxmox.com/?p=pve-qemu.git;a=tree;f=debian/patches;h=2c516be8e69a033d14809b17e8a661b3808257f7;hb=8d4a2d3f5569817221c19a91f763964c40e00292
6: https://gist.github.com/Fabian-Gruenbichler/5c3af22ac7e6faae46840bdcebd7df14
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2017-11-30 11:20:54
On 30/11/2017 10:33, Fabian Grünbichler wrote:
It was reverted in 785373b4c38719f4af6775845df6be1dfaea120f after which
the symptoms disappeared until this series was merged, which contains
369ea8242c0fb5239b4ddf0dc568f694bd244de4 mm/rmap: update to new mmu_notifier semantic v2
We haven't bisected the individual commits of the series yet, but the
commit immediately preceding its merge exhibits no problems, while
everything after does. It is not known whether the bug is actually in
the series itself, or whether increasing the likelihood of triggering it
is just a side-effect. There is a similar report[2] concerning an
upgrade from 4.12.12 to 4.12.13, which does not contain this series in
any form AFAICT but might be worth another look as well.
I know of one issue in this series (invalidate_page was removed from KVM
without reimplementing it as invalidate_range). I'll try to prioritize
the fix, but I don't think I can do it before Monday.
Thanks,
Paolo
It was reverted in 785373b4c38719f4af6775845df6be1dfaea120f after which
the symptoms disappeared until this series was merged, which contains
369ea8242c0fb5239b4ddf0dc568f694bd244de4 mm/rmap: update to new mmu_notifier semantic v2
We haven't bisected the individual commits of the series yet, but the
commit immediately preceding its merge exhibits no problems, while
everything after does. It is not known whether the bug is actually in
the series itself, or whether increasing the likelihood of triggering it
is just a side-effect. There is a similar report[2] concerning an
upgrade from 4.12.12 to 4.12.13, which does not contain this series in
any form AFAICT but might be worth another look as well.
I know of one issue in this series (invalidate_page was removed from KVM
without reimplementing it as invalidate_range). I'll try to prioritize
the fix, but I don't think I can do it before Monday.
The series also dropped the reloading of the APIC access page and we
never had it in invalidate_range_start ... I'll look into it today.