LinuxLists.cc - Re: [mm 4.15-rc7] Random oopses under memory pressure.

[permalink] [raw]

Subject: Re: [mm 4.15-rc7] Random oopses under memory pressure.

On Sun, Jan 14, 2018 at 3:54 AM, Tetsuo Handa
<[email protected]> wrote:
> This memory corruption bug occurs even on CONFIG_SMP=n CONFIG_PREEMPT_NONE=y
> kernel. This bug highly depends on timing and thus too difficult to bisect.
> This bug seems to exist at least since Linux 4.8 (judging from the traces, though
> the cause might be different). None of debugging configuration gives me a clue.
> So far only CONFIG_HIGHMEM=y CONFIG_DEBUG_PAGEALLOC=y kernel (with RAM enough to
> use HighMem: zone) seems to hit this bug, but it might be just by chance caused
> by timings. Thus, there is no evidence that 64bit kernels are not affected by
> this bug. But I can't narrow down any more. Thus, I call for developers who can
> narrow down / identify where the memory corruption bug is.

Hmm.

I guess I'm still hung up on the "it does not look like a valid
'struct page *'" thing.

Can you reproduce this with CONFIG_FLATMEM=y instead of CONFIG_SPARSEMEM?

Because if you can, I think we can easily add a few more pfn and
'struct page' validation debug statements. With SPARSEMEM, it gets
pretty complicated because the whole struct page setup is much more
complex.

Linus

2018-01-16 01:15:37

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

Linus Torvalds wrote:
> On Sun, Jan 14, 2018 at 3:54 AM, Tetsuo Handa
> <[email protected]> wrote:
> > This memory corruption bug occurs even on CONFIG_SMP=n CONFIG_PREEMPT_NONE=y
> > kernel. This bug highly depends on timing and thus too difficult to bisect.
> > This bug seems to exist at least since Linux 4.8 (judging from the traces, though
> > the cause might be different). None of debugging configuration gives me a clue.
> > So far only CONFIG_HIGHMEM=y CONFIG_DEBUG_PAGEALLOC=y kernel (with RAM enough to
> > use HighMem: zone) seems to hit this bug, but it might be just by chance caused
> > by timings. Thus, there is no evidence that 64bit kernels are not affected by
> > this bug. But I can't narrow down any more. Thus, I call for developers who can
> > narrow down / identify where the memory corruption bug is.
>
> Hmm.
>
> I guess I'm still hung up on the "it does not look like a valid
> 'struct page *'" thing.
>
> Can you reproduce this with CONFIG_FLATMEM=y instead of CONFIG_SPARSEMEM?
>
> Because if you can, I think we can easily add a few more pfn and
> 'struct page' validation debug statements. With SPARSEMEM, it gets
> pretty complicated because the whole struct page setup is much more
> complex.

I can't reproduce this with CONFIG_FLATMEM=y . But I'm not sure whether
we are hitting a bug in CONFIG_SPARSEMEM=y code, for the bug is highly
timing dependent.

----------
# diff .config.old .config
372a373
> CONFIG_X86_SUPPORTS_MEMORY_FAILURE=y
462d462
< CONFIG_NEED_NODE_MEMMAP_SIZE=y
468,471c468,471
< # CONFIG_FLATMEM_MANUAL is not set
< CONFIG_SPARSEMEM_MANUAL=y
< CONFIG_SPARSEMEM=y
< CONFIG_HAVE_MEMORY_PRESENT=y
---
> CONFIG_FLATMEM_MANUAL=y
> # CONFIG_SPARSEMEM_MANUAL is not set
> CONFIG_FLATMEM=y
> CONFIG_FLAT_NODE_MEM_MAP=y
478d477
< # CONFIG_MEMORY_HOTPLUG is not set
486a486,487
> CONFIG_ARCH_SUPPORTS_MEMORY_FAILURE=y
> # CONFIG_MEMORY_FAILURE is not set
----------

----------
[ 0.000000] Linux version 4.15.0-rc8 ([email protected]) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)) #381 Tue Jan 16 09:38:22 JST 2018
[ 0.000000] x86/fpu: x87 FPU will use FXSAVE
[ 0.000000] e820: BIOS-provided physical RAM map:
[ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
[ 0.000000] BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
[ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007fffbfff] usable
[ 0.000000] BIOS-e820: [mem 0x000000007fffc000-0x000000007fffffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
[ 0.000000] Notice: NX (Execute Disable) protection missing in CPU!
[ 0.000000] random: fast init done
[ 0.000000] SMBIOS 2.4 present.
[ 0.000000] DMI: Red Hat KVM, BIOS 0.5.1 01/01/2011
[ 0.000000] e820: last_pfn = 0x7fffc max_arch_pfn = 0x100000
[ 0.000000] x86/PAT: Configuration [0-7]: WB WC UC- UC WB WC UC- UC
[ 0.000000] found SMP MP-table at [mem 0x000f7300-0x000f730f] mapped at [(ptrval)]
[ 0.000000] Scanning 1 areas for low memory corruption
[ 0.000000] ACPI: Early table checksum verification disabled
[ 0.000000] ACPI: RSDP 0x00000000000F7160 000014 (v00 BOCHS )
[ 0.000000] ACPI: RSDT 0x000000007FFFFA9B 000030 (v01 BOCHS BXPCRSDT 00000001 BXPC 00000001)
[ 0.000000] ACPI: FACP 0x000000007FFFF177 000074 (v01 BOCHS BXPCFACP 00000001 BXPC 00000001)
[ 0.000000] ACPI: DSDT 0x000000007FFFE040 001137 (v01 BOCHS BXPCDSDT 00000001 BXPC 00000001)
[ 0.000000] ACPI: FACS 0x000000007FFFE000 000040
[ 0.000000] ACPI: SSDT 0x000000007FFFF1EB 000838 (v01 BOCHS BXPCSSDT 00000001 BXPC 00000001)
[ 0.000000] ACPI: APIC 0x000000007FFFFA23 000078 (v01 BOCHS BXPCAPIC 00000001 BXPC 00000001)
[ 0.000000] 1159MB HIGHMEM available.
[ 0.000000] 887MB LOWMEM available.
[ 0.000000] mapped low ram: 0 - 377fe000
[ 0.000000] low ram: 0 - 377fe000
[ 0.000000] tsc: Fast TSC calibration using PIT
[ 0.000000] Zone ranges:
[ 0.000000] DMA [mem 0x0000000000001000-0x0000000000ffffff]
[ 0.000000] Normal [mem 0x0000000001000000-0x00000000377fdfff]
[ 0.000000] HighMem [mem 0x00000000377fe000-0x000000007fffbfff]
[ 0.000000] Movable zone start for each node
[ 0.000000] Early memory node ranges
[ 0.000000] node 0: [mem 0x0000000000001000-0x000000000009efff]
[ 0.000000] node 0: [mem 0x0000000000100000-0x000000007fffbfff]
[ 0.000000] Initmem setup node 0 [mem 0x0000000000001000-0x000000007fffbfff]
[ 0.000000] Reserved but unavailable: 98 pages
[ 0.000000] Using APIC driver default
[ 0.000000] ACPI: PM-Timer IO Port: 0x608
[ 0.000000] ACPI: LAPIC_NMI (acpi_id[0xff] dfl dfl lint[0x1])
[ 0.000000] IOAPIC[0]: apic_id 0 already used, trying 1
[ 0.000000] IOAPIC[0]: apic_id 1, version 17, address 0xfec00000, GSI 0-23
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 5 global_irq 5 high level)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 10 global_irq 10 high level)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 11 global_irq 11 high level)
[ 0.000000] Using ACPI (MADT) for SMP configuration information
[ 0.000000] e820: [mem 0x80000000-0xfffbffff] available for PCI devices
[ 0.000000] clocksource: refined-jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1910969940391419 ns
[ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 521966
[ 0.000000] Kernel command line: ro console=ttyS0,115200n8 root=/dev/vda init=/init
[ 0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
[ 0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
[ 0.000000] Initializing CPU#0
[ 0.000000] Initializing HighMem for node 0 (000377fe:0007fffc)
[ 0.000000] Initializing Movable for node 0 (00000000:00000000)
[ 0.000000] Memory: 2064192K/2096744K available (3353K kernel code, 256K rwdata, 924K rodata, 376K init, 5308K bss, 32552K reserved, 0K cma-reserved, 1187832K highmem)
[ 0.000000] virtual kernel memory layout:
[ 0.000000] fixmap : 0xfffa4000 - 0xfffff000 ( 364 kB)
[ 0.000000] cpu_entry : 0xffc00000 - 0xffc28000 ( 160 kB)
[ 0.000000] pkmap : 0xff800000 - 0xffc00000 (4096 kB)
[ 0.000000] vmalloc : 0xf7ffe000 - 0xff7fe000 ( 120 MB)
[ 0.000000] lowmem : 0xc0000000 - 0xf77fe000 ( 887 MB)
[ 0.000000] .init : 0xc147e000 - 0xc14dc000 ( 376 kB)
[ 0.000000] .data : 0xc13466c8 - 0xc14713e0 (1195 kB)
[ 0.000000] .text : 0xc1000000 - 0xc13466c8 (3353 kB)
[ 0.000000] Checking if this processor honours the WP bit even in supervisor mode...Ok.
[ 0.000000] SLUB: HWalign=128, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[ 0.000000] NR_IRQS: 2304, nr_irqs: 256, preallocated irqs: 16
[ 0.000000] Console: colour VGA+ 80x25
[ 0.000000] console [ttyS0] enabled
[ 0.000000] ACPI: Core revision 20170831
[ 0.000000] ACPI: 2 ACPI AML tables successfully acquired and loaded
[ 0.001000] APIC: Switch to symmetric I/O mode setup
[ 0.002000] Enabling APIC mode: Flat. Using 1 I/O APICs
[ 0.002000] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
[ 0.007000] tsc: Fast TSC calibration using PIT
[ 0.008000] tsc: Detected 1995.399 MHz processor
[ 0.008000] Calibrating delay loop (skipped), value calculated using timer frequency.. 3990.79 BogoMIPS (lpj=1995399)
[ 0.008000] pid_max: default: 32768 minimum: 301
[ 0.009788] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes)
[ 0.010000] Mountpoint-cache hash table entries: 2048 (order: 1, 8192 bytes)
[ 0.017000] mce: CPU supports 10 MCE banks
[ 0.017478] Last level iTLB entries: 4KB 0, 2MB 0, 4MB 0
[ 0.017644] Last level dTLB entries: 4KB 0, 2MB 0, 4MB 0, 1GB 0
[ 0.017792] CPU: Intel Common 32-bit KVM processor (family: 0xf, model: 0x6, stepping: 0x1)
[ 0.018000] Spectre V2 mitigation: Vulnerable: Minimal generic ASM retpoline
[ 0.024000] Performance Events: unsupported Netburst CPU model 6 no PMU driver, software events only.
[ 0.027000] APIC calibration not consistent with PM-Timer: 205ms instead of 100ms
[ 0.027000] APIC delta adjusted to PM-Timer: 6250051 (12854429)
[ 0.029058] devtmpfs: initialized
[ 0.032000] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275000 ns
[ 0.032000] futex hash table entries: 256 (order: -1, 3072 bytes)
[ 0.035992] cpuidle: using governor menu
[ 0.037000] ACPI: bus type PCI registered
[ 0.039000] PCI: PCI BIOS revision 2.10 entry at 0xfd54b, last bus=0
[ 0.039000] PCI: Using configuration type 1 for base access
[ 0.048000] HugeTLB registered 4.00 MiB page size, pre-allocated 0 pages
[ 0.050000] ACPI: Added _OSI(Module Device)
[ 0.050000] ACPI: Added _OSI(Processor Device)
[ 0.050000] ACPI: Added _OSI(3.0 _SCP Extensions)
[ 0.050000] ACPI: Added _OSI(Processor Aggregator Device)
[ 0.071000] ACPI: Interpreter enabled
[ 0.071000] ACPI: (supports S0 S5)
[ 0.071000] ACPI: Using IOAPIC for interrupt routing
[ 0.071409] PCI: Using host bridge windows from ACPI; if necessary, use "pci=nocrs" and report a bug
[ 0.073000] ACPI: Enabled 16 GPEs in block 00 to 0F
[ 0.100000] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
[ 0.100000] acpi PNP0A03:00: _OSC: OS supports [ASPM ClockPM Segments MSI]
[ 0.100314] acpi PNP0A03:00: _OSC failed (AE_NOT_FOUND); disabling ASPM
[ 0.100785] acpi PNP0A03:00: fail to add MMCONFIG information, can't access extended PCI configuration space under this bridge.
[ 0.102000] PCI host bridge to bus 0000:00
[ 0.102156] pci_bus 0000:00: root bus resource [io 0x0000-0x0cf7 window]
[ 0.102378] pci_bus 0000:00: root bus resource [io 0x0d00-0xffff window]
[ 0.102602] pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000bffff window]
[ 0.102784] pci_bus 0000:00: root bus resource [mem 0x80000000-0xfebfffff window]
[ 0.103000] pci_bus 0000:00: root bus resource [bus 00-ff]
[ 0.108493] pci 0000:00:01.1: legacy IDE quirk: reg 0x10: [io 0x01f0-0x01f7]
[ 0.108716] pci 0000:00:01.1: legacy IDE quirk: reg 0x14: [io 0x03f6]
[ 0.108905] pci 0000:00:01.1: legacy IDE quirk: reg 0x18: [io 0x0170-0x0177]
[ 0.109000] pci 0000:00:01.1: legacy IDE quirk: reg 0x1c: [io 0x0376]
[ 0.110000] pci 0000:00:01.3: quirk: [io 0x0600-0x063f] claimed by PIIX4 ACPI
[ 0.110000] pci 0000:00:01.3: quirk: [io 0x0700-0x070f] claimed by PIIX4 SMB
[ 0.129000] ACPI: PCI Interrupt Link [LNKA] (IRQs 5 *10 11)
[ 0.130000] ACPI: PCI Interrupt Link [LNKB] (IRQs 5 *10 11)
[ 0.130000] ACPI: PCI Interrupt Link [LNKC] (IRQs 5 10 *11)
[ 0.131000] ACPI: PCI Interrupt Link [LNKD] (IRQs 5 10 *11)
[ 0.131000] ACPI: PCI Interrupt Link [LNKS] (IRQs *9)
[ 0.135000] pci 0000:00:02.0: vgaarb: setting as boot VGA device
[ 0.135000] pci 0000:00:02.0: vgaarb: VGA device added: decodes=io+mem,owns=io+mem,locks=none
[ 0.135000] pci 0000:00:02.0: vgaarb: bridge control possible
[ 0.135000] vgaarb: loaded
[ 0.136000] SCSI subsystem initialized
[ 0.136735] PCI: Using ACPI for IRQ routing
[ 0.137652] clocksource: Switched to clocksource refined-jiffies
[ 0.138999] ACPI: Failed to create genetlink family for ACPI event
[ 0.138999] pnp: PnP ACPI init
[ 0.142999] pnp: PnP ACPI: found 5 devices
[ 0.167133] clocksource: acpi_pm: mask: 0xffffff max_cycles: 0xffffff, max_idle_ns: 2085701024 ns
[ 0.167414] clocksource: Switched to clocksource acpi_pm
[ 0.167995] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
[ 0.167995] pci 0000:00:01.0: PIIX3: Enabling Passive Release
[ 0.167995] pci 0000:00:01.0: Activating ISA DMA hang workarounds
[ 0.170867] pci 0000:00:02.0: Video device with shadowed ROM at [mem 0x000c0000-0x000dffff]
[ 0.179490] Scanning for low memory corruption every 60 seconds
[ 0.185037] workingset: timestamp_bits=14 max_order=19 bucket_order=5
[ 0.209598] zbud: loaded
[ 0.214606] SGI XFS with ACLs, security attributes, no debug enabled
[ 0.226422] bounce: pool size: 64 pages
[ 0.226745] io scheduler noop registered (default)
[ 0.239185] atomic64_test: passed for i586+ platform with CX8 and with SSE
[ 0.245995] input: Power Button as /devices/LNXSYSTM:00/LNXPWRBN:00/input/input0
[ 0.246746] ACPI: Power Button [PWRF]
[ 0.259029] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
[ 0.259249] virtio-pci 0000:00:04.0: virtio_pci: leaving for legacy driver
[ 0.263351] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[ 0.286776] 00:04: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A
[ 0.293723] Non-volatile memory driver v1.3
[ 0.330628] i8042: PNP: PS/2 Controller [PNP0303:KBD,PNP0f13:MOU] at 0x60,0x64 irq 1,12
[ 0.335343] serio: i8042 KBD port at 0x60,0x64 irq 1
[ 0.335644] serio: i8042 AUX port at 0x60,0x64 irq 12
[ 0.338091] Using IPI Shortcut mode
[ 0.338091] sched_clock: Marking stable (337091080, 0)->(767486575, -430395495)
[ 0.341662] page_owner is disabled
[ 0.373592] XFS (vda): Mounting V5 Filesystem
[ 0.440300] XFS (vda): Ending clean mount
[ 0.463216] VFS: Mounted root (xfs filesystem) readonly on device 254:0.
[ 0.465860] devtmpfs: mounted
[ 0.466592] debug: unmapping init [mem 0xc147e000-0xc14dbfff]
[ 0.468584] Write protecting the kernel text: 3356k
[ 0.469119] Write protecting the kernel read-only data: 936k
Starting a.out
[ 1.184517] tsc: Refined TSC clocksource calibration: 1995.468 MHz
[ 1.184906] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x3986e9f5ad2, max_idle_ns: 881590531620 ns
[ 2.259952] clocksource: Switched to clocksource tsc
----------

I dont know why but selecting CONFIG_FLATMEM=y seems to avoid a different bug
where bootup of qemu randomly fails at

----------
[ 0.000000] Linux version 4.15.0-rc8 ([email protected]) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)) #380 Tue Jan 16 09:25:52 JST 2018
[ 0.000000] x86/fpu: x87 FPU will use FXSAVE
[ 0.000000] e820: BIOS-provided physical RAM map:
[ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
[ 0.000000] BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
[ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007fffbfff] usable
[ 0.000000] BIOS-e820: [mem 0x000000007fffc000-0x000000007fffffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
[ 0.000000] Notice: NX (Execute Disable) protection missing in CPU!
[ 0.000000] random: fast init done
[ 0.000000] SMBIOS 2.4 present.
[ 0.000000] DMI: Red Hat KVM, BIOS 0.5.1 01/01/2011
[ 0.000000] e820: last_pfn = 0x7fffc max_arch_pfn = 0x100000
[ 0.000000] x86/PAT: Configuration [0-7]: WB WC UC- UC WB WC UC- UC
[ 0.000000] found SMP MP-table at [mem 0x000f7300-0x000f730f] mapped at [(ptrval)]
[ 0.000000] Scanning 1 areas for low memory corruption
[ 0.000000] ACPI: Early table checksum verification disabled
[ 0.000000] ACPI: RSDP 0x00000000000F7160 000014 (v00 BOCHS )
[ 0.000000] ACPI: RSDT 0x000000007FFFFA9B 000030 (v01 BOCHS BXPCRSDT 00000001 BXPC 00000001)
[ 0.000000] ACPI: FACP 0x000000007FFFF177 000074 (v01 BOCHS BXPCFACP 00000001 BXPC 00000001)
[ 0.000000] ACPI: DSDT 0x000000007FFFE040 001137 (v01 BOCHS BXPCDSDT 00000001 BXPC 00000001)
[ 0.000000] ACPI: FACS 0x000000007FFFE000 000040
[ 0.000000] ACPI: SSDT 0x000000007FFFF1EB 000838 (v01 BOCHS BXPCSSDT 00000001 BXPC 00000001)
[ 0.000000] ACPI: APIC 0x000000007FFFFA23 000078 (v01 BOCHS BXPCAPIC 00000001 BXPC 00000001)
[ 0.000000] 1159MB HIGHMEM available.
[ 0.000000] 887MB LOWMEM available.
[ 0.000000] mapped low ram: 0 - 377fe000
[ 0.000000] low ram: 0 - 377fe000
[ 0.000000] tsc: Unable to calibrate against PIT
[ 0.000000] tsc: No reference (HPET/PMTIMER) available
[ 0.000000] Zone ranges:
[ 0.000000] DMA [mem 0x0000000000001000-0x0000000000ffffff]
[ 0.000000] Normal [mem 0x0000000001000000-0x00000000377fdfff]
[ 0.000000] HighMem [mem 0x00000000377fe000-0x000000007fffbfff]
[ 0.000000] Movable zone start for each node
[ 0.000000] Early memory node ranges
[ 0.000000] node 0: [mem 0x0000000000001000-0x000000000009efff]
[ 0.000000] node 0: [mem 0x0000000000100000-0x000000007fffbfff]
[ 0.000000] Initmem setup node 0 [mem 0x0000000000001000-0x000000007fffbfff]
[ 0.000000] Reserved but unavailable: 98 pages
[ 0.000000] Using APIC driver default
[ 0.000000] ACPI: PM-Timer IO Port: 0x608
[ 0.000000] ACPI: LAPIC_NMI (acpi_id[0xff] dfl dfl lint[0x1])
[ 0.000000] IOAPIC[0]: apic_id 0 already used, trying 1
[ 0.000000] IOAPIC[0]: apic_id 1, version 17, address 0xfec00000, GSI 0-23
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 5 global_irq 5 high level)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 10 global_irq 10 high level)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 11 global_irq 11 high level)
[ 0.000000] Using ACPI (MADT) for SMP configuration information
[ 0.000000] e820: [mem 0x80000000-0xfffbffff] available for PCI devices
[ 0.000000] clocksource: refined-jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1910969940391419 ns
[ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 521966
[ 0.000000] Kernel command line: ro console=ttyS0,115200n8 root=/dev/vda init=/init
[ 0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
[ 0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
[ 0.000000] Initializing CPU#0
[ 0.000000] Initializing HighMem for node 0 (000377fe:0007fffc)
[ 0.000000] Initializing Movable for node 0 (00000000:00000000)
[ 0.000000] Memory: 2064188K/2096744K available (3357K kernel code, 252K rwdata, 924K rodata, 380K init, 5308K bss, 32556K reserved, 0K cma-reserved, 1187832K highmem)
[ 0.000000] virtual kernel memory layout:
[ 0.000000] fixmap : 0xfffa4000 - 0xfffff000 ( 364 kB)
[ 0.000000] cpu_entry : 0xffc00000 - 0xffc28000 ( 160 kB)
[ 0.000000] pkmap : 0xff800000 - 0xffc00000 (4096 kB)
[ 0.000000] vmalloc : 0xf7ffe000 - 0xff7fe000 ( 120 MB)
[ 0.000000] lowmem : 0xc0000000 - 0xf77fe000 ( 887 MB)
[ 0.000000] .init : 0xc147e000 - 0xc14dd000 ( 380 kB)
[ 0.000000] .data : 0xc13476a8 - 0xc14713e0 (1191 kB)
[ 0.000000] .text : 0xc1000000 - 0xc13476a8 (3357 kB)
[ 0.000000] Checking if this processor honours the WP bit even in supervisor mode...Ok.
[ 0.000000] SLUB: HWalign=128, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[ 0.000000] NR_IRQS: 2304, nr_irqs: 256, preallocated irqs: 16
[ 0.000000] Console: colour VGA+ 80x25
[ 0.000000] console [ttyS0] enabled
[ 0.000000] ACPI: Core revision 20170831
[ 0.000000] ACPI: 2 ACPI AML tables successfully acquired and loaded
[ 0.000000] APIC: Switch to symmetric I/O mode setup
[ 0.000000] Enabling APIC mode: Flat. Using 1 I/O APICs
[ 0.000000] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
[ 0.000000] ..MP-BIOS bug: 8254 timer not connected to IO-APIC
[ 0.000000] ...trying to set up timer (IRQ0) through the 8259A ...
[ 0.000000] ..... (found apic 0 pin 2) ...
[ 0.000000] ....... failed.
[ 0.000000] ...trying to set up timer as Virtual Wire IRQ...
[ 0.000000] ..... failed.
[ 0.000000] ...trying to set up timer as ExtINT IRQ...
[ 0.000000] ..... failed :(.
[ 0.000000] Kernel panic - not syncing: IO-APIC + timer doesn't work! Boot with apic=debug and send a report. Then try booting with the 'noapic' option.
[ 0.000000]
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.15.0-rc8 #380
[ 0.000000] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
[ 0.000000] Call Trace:
[ 0.000000] dump_stack+0x16/0x24
[ 0.000000] panic+0x87/0x1a2
[ 0.000000] ? vprintk_func+0x23/0x60
[ 0.000000] setup_IO_APIC+0x4dd/0x6f8
[ 0.000000] ? clear_IO_APIC+0x29/0x50
[ 0.000000] ? lapic_get_maxlvt+0x2e/0x40
[ 0.000000] ? end_local_APIC_setup+0x9d/0x130
[ 0.000000] apic_bsp_setup+0x70/0x75
[ 0.000000] apic_intr_mode_init+0x10c/0x10e
[ 0.000000] ? hpet_time_init+0x1e/0x20
[ 0.000000] x86_late_time_init+0xf/0x16
[ 0.000000] start_kernel+0x2ef/0x360
[ 0.000000] ? set_init_arg+0x52/0x52
[ 0.000000] i386_start_kernel+0x13d/0x140
[ 0.000000] startup_32_smp+0x164/0x168
[ 0.000000] Rebooting in 1 seconds..
----------

or hangs at

----------
[ 0.000000] Linux version 4.15.0-rc8 ([email protected]) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)) #380 Tue Jan 16 09:25:52 JST 2018
[ 0.000000] x86/fpu: x87 FPU will use FXSAVE
[ 0.000000] e820: BIOS-provided physical RAM map:
[ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
[ 0.000000] BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
[ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007fffbfff] usable
[ 0.000000] BIOS-e820: [mem 0x000000007fffc000-0x000000007fffffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
[ 0.000000] Notice: NX (Execute Disable) protection missing in CPU!
[ 0.000000] random: fast init done
[ 0.000000] SMBIOS 2.4 present.
[ 0.000000] DMI: Red Hat KVM, BIOS 0.5.1 01/01/2011
[ 0.000000] e820: last_pfn = 0x7fffc max_arch_pfn = 0x100000
[ 0.000000] x86/PAT: Configuration [0-7]: WB WC UC- UC WB WC UC- UC
[ 0.000000] found SMP MP-table at [mem 0x000f7300-0x000f730f] mapped at [(ptrval)]
[ 0.000000] Scanning 1 areas for low memory corruption
[ 0.000000] ACPI: Early table checksum verification disabled
[ 0.000000] ACPI: RSDP 0x00000000000F7160 000014 (v00 BOCHS )
[ 0.000000] ACPI: RSDT 0x000000007FFFFA9B 000030 (v01 BOCHS BXPCRSDT 00000001 BXPC 00000001)
[ 0.000000] ACPI: FACP 0x000000007FFFF177 000074 (v01 BOCHS BXPCFACP 00000001 BXPC 00000001)
[ 0.000000] ACPI: DSDT 0x000000007FFFE040 001137 (v01 BOCHS BXPCDSDT 00000001 BXPC 00000001)
[ 0.000000] ACPI: FACS 0x000000007FFFE000 000040
[ 0.000000] ACPI: SSDT 0x000000007FFFF1EB 000838 (v01 BOCHS BXPCSSDT 00000001 BXPC 00000001)
[ 0.000000] ACPI: APIC 0x000000007FFFFA23 000078 (v01 BOCHS BXPCAPIC 00000001 BXPC 00000001)
[ 0.000000] 1159MB HIGHMEM available.
[ 0.000000] 887MB LOWMEM available.
[ 0.000000] mapped low ram: 0 - 377fe000
[ 0.000000] low ram: 0 - 377fe000
[ 0.000000] tsc: Unable to calibrate against PIT
[ 0.000000] tsc: No reference (HPET/PMTIMER) available
[ 0.000000] Zone ranges:
[ 0.000000] DMA [mem 0x0000000000001000-0x0000000000ffffff]
[ 0.000000] Normal [mem 0x0000000001000000-0x00000000377fdfff]
[ 0.000000] HighMem [mem 0x00000000377fe000-0x000000007fffbfff]
[ 0.000000] Movable zone start for each node
[ 0.000000] Early memory node ranges
[ 0.000000] node 0: [mem 0x0000000000001000-0x000000000009efff]
[ 0.000000] node 0: [mem 0x0000000000100000-0x000000007fffbfff]
[ 0.000000] Initmem setup node 0 [mem 0x0000000000001000-0x000000007fffbfff]
[ 0.000000] Reserved but unavailable: 98 pages
[ 0.000000] Using APIC driver default
[ 0.000000] ACPI: PM-Timer IO Port: 0x608
[ 0.000000] ACPI: LAPIC_NMI (acpi_id[0xff] dfl dfl lint[0x1])
[ 0.000000] IOAPIC[0]: apic_id 0 already used, trying 1
[ 0.000000] IOAPIC[0]: apic_id 1, version 17, address 0xfec00000, GSI 0-23
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 5 global_irq 5 high level)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 10 global_irq 10 high level)
[ 0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 11 global_irq 11 high level)
[ 0.000000] Using ACPI (MADT) for SMP configuration information
[ 0.000000] e820: [mem 0x80000000-0xfffbffff] available for PCI devices
[ 0.000000] clocksource: refined-jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1910969940391419 ns
[ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 521966
[ 0.000000] Kernel command line: ro console=ttyS0,115200n8 root=/dev/vda init=/init
[ 0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
[ 0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
[ 0.000000] Initializing CPU#0
[ 0.000000] Initializing HighMem for node 0 (000377fe:0007fffc)
[ 0.000000] Initializing Movable for node 0 (00000000:00000000)
[ 0.000000] Memory: 2064188K/2096744K available (3357K kernel code, 252K rwdata, 924K rodata, 380K init, 5308K bss, 32556K reserved, 0K cma-reserved, 1187832K highmem)
[ 0.000000] virtual kernel memory layout:
[ 0.000000] fixmap : 0xfffa4000 - 0xfffff000 ( 364 kB)
[ 0.000000] cpu_entry : 0xffc00000 - 0xffc28000 ( 160 kB)
[ 0.000000] pkmap : 0xff800000 - 0xffc00000 (4096 kB)
[ 0.000000] vmalloc : 0xf7ffe000 - 0xff7fe000 ( 120 MB)
[ 0.000000] lowmem : 0xc0000000 - 0xf77fe000 ( 887 MB)
[ 0.000000] .init : 0xc147e000 - 0xc14dd000 ( 380 kB)
[ 0.000000] .data : 0xc13476a8 - 0xc14713e0 (1191 kB)
[ 0.000000] .text : 0xc1000000 - 0xc13476a8 (3357 kB)
[ 0.000000] Checking if this processor honours the WP bit even in supervisor mode...Ok.
[ 0.000000] SLUB: HWalign=128, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[ 0.000000] NR_IRQS: 2304, nr_irqs: 256, preallocated irqs: 16
[ 0.000000] Console: colour VGA+ 80x25
[ 0.000000] console [ttyS0] enabled
[ 0.000000] ACPI: Core revision 20170831
[ 0.000000] ACPI: 2 ACPI AML tables successfully acquired and loaded
[ 0.001000] APIC: Switch to symmetric I/O mode setup
[ 0.001000] Enabling APIC mode: Flat. Using 1 I/O APICs
[ 0.001000] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
[ 0.003000] ..MP-BIOS bug: 8254 timer not connected to IO-APIC
[ 0.003000] ...trying to set up timer (IRQ0) through the 8259A ...
[ 0.003000] ..... (found apic 0 pin 2) ...
[ 0.005000] ....... failed.
[ 0.005000] ...trying to set up timer as Virtual Wire IRQ...
[ 13.120000] random: crng init done
----------

which has been bothering me when testing using qemu. Note that the 13.120000 part is
incorrect. There is no delay between "...trying to set up timer as Virtual Wire IRQ..."
line and "random: crng init done" line.

2018-01-16 02:14:53

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Mon, Jan 15, 2018 at 5:15 PM, Tetsuo Handa
<[email protected]> wrote:
>
> I can't reproduce this with CONFIG_FLATMEM=y . But I'm not sure whether
> we are hitting a bug in CONFIG_SPARSEMEM=y code, for the bug is highly
> timing dependent.

Hmm. Maybe. But sparsemem really also generates *much* more complex
code particularly for the pfn_to_page() case.

It also has much less testing. For example, on x86-64 we do use
sparsemem, but we use the VMEMMAP version of sparsemem: the version
that does *not* play really odd and complex games with that whole
pfn_to_page().

I've always felt like sparsemem was really damn complicated. The
whole "section_mem_map" encoding is really subtle and odd.

And considering that we're getting what appears to be a invalid page,
in one of the more complicated sequences that very much does that
whole pfn_to_page(), I really wonder.

I wonder if somebody could add some VM_BUG_ON() checks to the
non-vmemmap case of sparsemem in include/asm-generic/memory_model.h.

Because this:

#define __pfn_to_page(pfn) \
({ unsigned long __pfn = (pfn); \
struct mem_section *__sec = __pfn_to_section(__pfn); \
__section_mem_map_addr(__sec) + __pfn; \
})

is really subtle, and if we have some case where we pass in an
out-of-range pfn, or some case where we get the section wrong (because
the pfn is between sections or whatever due to some subtle setup bug),
things will really go sideways.

The reason I was hoping you could do this for FLATMEM is that it's
much easier to verify the pfn range in that case. The sparsemem cases
really makes it much nastier.

That said, all of that code is really old. Most of it goes back to
-05/06 or so. But since you seem to be able to reproduce at least back
to 4.8, I guess this bug does back years too.

But I'm adding Dave Hansen explicitly to the cc, in case he has any
ideas. Not because I blame him, but he's touched the sparsemem code
fairly recently, so maybe he'd have some idea on adding sanity
checking to the sparsemem version of pfn_to_page().

> I dont know why but selecting CONFIG_FLATMEM=y seems to avoid a different bug
> where bootup of qemu randomly fails at

Hmm. That looks very different indeed. But if CONFIG_SPARSEMEM
(presumably together with HIGHMEM) has some odd off-by-one corner case
or similar, who knows *what* issues it could trigger.

Linus

2018-01-16 08:07:05

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On 01/15/2018 06:14 PM, Linus Torvalds wrote:
> But I'm adding Dave Hansen explicitly to the cc, in case he has any
> ideas. Not because I blame him, but he's touched the sparsemem code
> fairly recently, so maybe he'd have some idea on adding sanity
> checking to the sparsemem version of pfn_to_page().

I swear I haven't touched it lately!

I'm not sure I'd go after pfn_to_page(). *Maybe* if we were close to
the places where we've done a pfn_to_page(), but I'm not seeing those.
These, for instance (from the January 5th post) have sane (~500MB) PFNs
and all BUG_ON() because of seeing the page being locked at free:

[ 192.152510] BUG: Bad page state in process a.out pfn:18566
[ 77.872133] BUG: Bad page state in process a.out pfn:1873a
[ 188.992549] BUG: Bad page state in process a.out pfn:197ea

and the page in all those cases came off a list, not out of a pte or
something that would need pfn_to_page(). The page fault path leading up
to the "EIP is at page_cache_tree_insert+0xbe/0xc0" probably doesn't
have a pfn_to_page() anywhere in there at all.

Did anyone else notice the

[ 31.068198] ? vmalloc_sync_all+0x150/0x150

present in a bunch of the stack traces? That should be pretty uncommon.
Is it just part of the normal do_page_fault() stack and the stack
dumper picks up on it?

A few things from earlier in this thread:

> [ 44.103192] page:5a5a0697 count:-1055023618 mapcount:-1055030029 mapping:26f4be11 index:0xc11d7c83
> [ 44.103196] flags: 0xc10528fe(waiters|error|referenced|uptodate|dirty|lru|active|reserved|private_2|mappedtodisk|swapbacked)
> [ 44.103200] raw: c10528fe c114fff7 c11d7c83 c11d84f2 c11d9dfe c11daa34 c11daaa0 c13e65df
> [ 44.103201] raw: c13e4a1c c13e4c62
> [ 44.103202] page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) <= 0)
> [ 44.103203] page->mem_cgroup:35401b27

Isn't that 'page:' a non-aligned address in userspace? It's also weird
that you start dumping out kernel-looking addresses that came from
userspace addresses. Which VM_SPLIT option are you running with, btw?

I'm still pretty stumped, though.

2018-01-16 08:37:45

by Ingo Molnar

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

* Dave Hansen <[email protected]> wrote:

> Did anyone else notice the
>
> [ 31.068198] ? vmalloc_sync_all+0x150/0x150
>
> present in a bunch of the stack traces? That should be pretty uncommon.

I thikn that's pretty unusual:

> Is it just part of the normal do_page_fault() stack and the stack
> dumper picks up on it?

No, it should only be called by register_die_notifier(), which is not part of the
regular stack dump, AFAICS.

Thanks,

Ingo

2018-01-16 17:33:26

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

Linus Torvalds wrote:
> On Mon, Jan 15, 2018 at 5:15 PM, Tetsuo Handa
> <[email protected]> wrote:
> >
> > I can't reproduce this with CONFIG_FLATMEM=y . But I'm not sure whether
> > we are hitting a bug in CONFIG_SPARSEMEM=y code, for the bug is highly
> > timing dependent.
>
> Hmm. Maybe. But sparsemem really also generates *much* more complex
> code particularly for the pfn_to_page() case.

Since I got a faster reproducer, I tried full bisection between 4.11 and 4.12-rc1.
But I have no idea why bisection arrives at c0332694903a37cf.

----------
gcc -Wall -O3 -m32 -o /mnt/a.out -x c - << "EOF"
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char *argv[])
{
if (argc != 1) {
unsigned long long size;
char *buf = NULL;
unsigned long long i;
for (size = 1048576; size < 512ULL * (1 << 30); size += 1048576) {
char *cp = realloc(buf, size);
if (!cp) {
size -= 1048576;
break;
}
buf = cp;
}
for (i = 0; i < size; i += 4096)
buf[i] = 0;
_exit(0);
} else
while (1) {
if (fork() == 0) {
execlp(argv[0], argv[0], "", NULL);
_exit(0);
}
sleep(1);
}
return 0;
}
EOF
----------

----------
# bad: [2ea659a9ef488125eb46da6eb571de5eae5c43f6] Linux 4.12-rc1
# good: [a351e9b9fc24e982ec2f0e76379a49826036da12] Linux 4.11
git bisect start 'HEAD' 'v4.11'
# good: [221656e7c4ce342b99c31eca96c1cbb6d1dce45f] Merge tag 'sound-4.12-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good 221656e7c4ce342b99c31eca96c1cbb6d1dce45f
# good: [c6a677c6f37bb7abc85ba7e3465e82b9f7eb1d91] Merge tag 'staging-4.12-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
git bisect good c6a677c6f37bb7abc85ba7e3465e82b9f7eb1d91
# bad: [0ff4c01b279a590a2826ade9321ad8c7ca5a1b6c] Merge tag 'armsoc-arm64' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
git bisect bad 0ff4c01b279a590a2826ade9321ad8c7ca5a1b6c
# bad: [8f3207c7eab9d885cc64c778416537034a7d9c5b] Merge tag 'tty-4.12-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty
git bisect bad 8f3207c7eab9d885cc64c778416537034a7d9c5b
# bad: [9c6ee01ed5bb1ee489d580eaa60d7eb5a8ede336] Merge branch 'for-linus' of git://git.armlinux.org.uk/~rmk/linux-arm
git bisect bad 9c6ee01ed5bb1ee489d580eaa60d7eb5a8ede336
# bad: [fe7a719b30dfdb4d55680461954b99b257ebe671] Merge branch 'for-next' of git://git.samba.org/sfrench/cifs-2.6
git bisect bad fe7a719b30dfdb4d55680461954b99b257ebe671
# good: [d557d1b58b3546bab2c5bc2d624c5709840e6b10] refcount: change EXPORT_SYMBOL markings
git bisect good d557d1b58b3546bab2c5bc2d624c5709840e6b10
# good: [8f720d9f892e0e223dab8400f03130bc208c72e7] xfs: publish UUID in struct super_block
git bisect good 8f720d9f892e0e223dab8400f03130bc208c72e7
# bad: [d173a25165c124442182f6b21d0c2ec381a0eebe] blk-mq: move debugfs declarations to a separate header file
git bisect bad d173a25165c124442182f6b21d0c2ec381a0eebe
# bad: [2719aa217e0d025dbfce74ac777815776ccec072] blk-mq: don't use sync workqueue flushing from drivers
git bisect bad 2719aa217e0d025dbfce74ac777815776ccec072
# bad: [9f2779bff2f178496fb00b89797734ee245d2c93] blk-mq-sched: remove hack that bypasses scheduler for reserved requests
git bisect bad 9f2779bff2f178496fb00b89797734ee245d2c93
# bad: [8afdd94c74e416de74a8ee61d79e4bf93466420b] mtip32xx: kill atomic argument to mtip_quiesce_io()
git bisect bad 8afdd94c74e416de74a8ee61d79e4bf93466420b
# bad: [0f6422a2c57c6afcf66ead903dc3fa6641184aa4] mtip32xx: get rid of 'atomic' argument to mtip_exec_internal_command()
git bisect bad 0f6422a2c57c6afcf66ead903dc3fa6641184aa4
# bad: [c0332694903a37cf8ecdc9102d5c9e09cf8643d0] block: Remove elevator_change()
git bisect bad c0332694903a37cf8ecdc9102d5c9e09cf8643d0
# first bad commit: [c0332694903a37cf8ecdc9102d5c9e09cf8643d0] block: Remove elevator_change()
----------

I tried different routes from mm/sparse.c , but I feel I can't find the culprit.

----------
# bad: [7660a6fddcbae344de8583aa4092071312f110c3] mm: allow slab_nomerge to be set at build time
# good: [60a7a88dbb9fc9adcca78a10a3ecf36966b5a45c] mm/sparse: refine usemap_size() a little
git bisect start '7660a6fddcbae344de8583aa4092071312f110c3' '60a7a88dbb9fc9adcca78a10a3ecf36966b5a45c'
# bad: [9786e34e0a6055dbd1b46e16dfa791ac2b3da289] Merge tag 'for-linus-20170510' of git://git.infradead.org/linux-mtd
git bisect bad 9786e34e0a6055dbd1b46e16dfa791ac2b3da289
# good: [1062ae4982cabbf60f89b4e069fbb7def7edc8f7] Merge tag 'drm-forgot-about-tegra-for-v4.12-rc1' of git://people.freedesktop.org/~airlied/linux
git bisect good 1062ae4982cabbf60f89b4e069fbb7def7edc8f7
# bad: [bf5f89463f5b3109a72ed13ca62b57e90213387d] Merge branch 'akpm' (patches from Andrew)
git bisect bad bf5f89463f5b3109a72ed13ca62b57e90213387d
# good: [2b0d92b265324cfe42839a23cb46677bb0112c2c] staging: ks7010: remove cast from netdev_priv()
git bisect good 2b0d92b265324cfe42839a23cb46677bb0112c2c
# bad: [d484467c860dab3e17893d23b2238e1f581460fa] Merge tag 'xfs-4.12-merge-7' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
git bisect bad d484467c860dab3e17893d23b2238e1f581460fa
# good: [e8357cdec3d1b6b42566ce3bc960e5e10c2b3787] [media] media: stk1160: Add Kconfig help on snd-usb-audio requirement
git bisect good e8357cdec3d1b6b42566ce3bc960e5e10c2b3787
# good: [53ef7d0e208fa38c3f63d287e0c3ab174f1e1235] Merge tag 'libnvdimm-for-4.12' of git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm
git bisect good 53ef7d0e208fa38c3f63d287e0c3ab174f1e1235
# good: [044f1daaaaf7c86bc4fcf433848b7baae236946b] Merge branch 'for-linus' of git://git.kernel.dk/linux-block
git bisect good 044f1daaaaf7c86bc4fcf433848b7baae236946b
# good: [bf8eadbacb24e321c99bbdd901589942712810d1] xfs: remove xfs_bmap_remap_alloc
git bisect good bf8eadbacb24e321c99bbdd901589942712810d1
# good: [e2a641922a3592b5ea226624d5abeb13eb49622c] xfs: corruption needs to respect endianess too!
git bisect good e2a641922a3592b5ea226624d5abeb13eb49622c
# good: [3c3781951c9a155a56e5eed567349118374cc315] xfs: Allow user to kill fstrim process
git bisect good 3c3781951c9a155a56e5eed567349118374cc315
# good: [ae2c4ac2dd39b23a87ddb14ceddc3f2872c6aef5] xfs: update ag iterator to support wait on new inodes
git bisect good ae2c4ac2dd39b23a87ddb14ceddc3f2872c6aef5
# good: [fe0be23e68200573de027de9b8cc2b27e7fce35e] xfs: reserve enough blocks to handle btree splits when remapping
git bisect good fe0be23e68200573de027de9b8cc2b27e7fce35e
# good: [161f55efba5ddccc690139fae9373cafc3447a97] xfs: fix use-after-free in xfs_finish_page_writeback
git bisect good 161f55efba5ddccc690139fae9373cafc3447a97
# first bad commit: [d484467c860dab3e17893d23b2238e1f581460fa] Merge tag 'xfs-4.12-merge-7' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux

# bad: [7660a6fddcbae344de8583aa4092071312f110c3] mm: allow slab_nomerge to be set at build time
# good: [60a7a88dbb9fc9adcca78a10a3ecf36966b5a45c] mm/sparse: refine usemap_size() a little
git bisect start 'HEAD' '60a7a88dbb9fc9adcca78a10a3ecf36966b5a45c' 'mm/'
# bad: [499118e966f1d2150bd66647c8932343c4e9a0b8] mm: introduce memalloc_noreclaim_{save,restore}
git bisect bad 499118e966f1d2150bd66647c8932343c4e9a0b8
# good: [5e82cd120382ad7bbcc82298e34a034538b4384c] kasan: introduce helper functions for determining bug type
git bisect good 5e82cd120382ad7bbcc82298e34a034538b4384c
# bad: [f25ba6dccc3bfe7e1524f4498a171be038507c45] mm, compaction: reorder fields in struct compact_control
git bisect bad f25ba6dccc3bfe7e1524f4498a171be038507c45
# good: [b19385993623c1a18a686b6b271cd24d5aa96f52] kasan: separate report parts by empty lines
git bisect good b19385993623c1a18a686b6b271cd24d5aa96f52
# good: [ab182e67ec99ea0c8d7435a32a4a1ed9bb02559a] Merge tag 'arm64-upstream' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux
git bisect good ab182e67ec99ea0c8d7435a32a4a1ed9bb02559a
# good: [e4231bcda72daef497af45e195a33daa0f9357d0] cma: Introduce cma_for_each_area
git bisect good e4231bcda72daef497af45e195a33daa0f9357d0
# good: [c6a677c6f37bb7abc85ba7e3465e82b9f7eb1d91] Merge tag 'staging-4.12-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
git bisect good c6a677c6f37bb7abc85ba7e3465e82b9f7eb1d91
# first bad commit: [f25ba6dccc3bfe7e1524f4498a171be038507c45] mm, compaction: reorder fields in struct compact_control
----------

>
> It also has much less testing. For example, on x86-64 we do use
> sparsemem, but we use the VMEMMAP version of sparsemem: the version
> that does *not* play really odd and complex games with that whole
> pfn_to_page().
>
> I've always felt like sparsemem was really damn complicated. The
> whole "section_mem_map" encoding is really subtle and odd.
>
> And considering that we're getting what appears to be a invalid page,
> in one of the more complicated sequences that very much does that
> whole pfn_to_page(), I really wonder.
>
> I wonder if somebody could add some VM_BUG_ON() checks to the
> non-vmemmap case of sparsemem in include/asm-generic/memory_model.h.
>
> Because this:
>
> #define __pfn_to_page(pfn) \
> ({ unsigned long __pfn = (pfn); \
> struct mem_section *__sec = __pfn_to_section(__pfn); \
> __section_mem_map_addr(__sec) + __pfn; \
> })
>
> is really subtle, and if we have some case where we pass in an
> out-of-range pfn, or some case where we get the section wrong (because
> the pfn is between sections or whatever due to some subtle setup bug),
> things will really go sideways.
>
> The reason I was hoping you could do this for FLATMEM is that it's
> much easier to verify the pfn range in that case. The sparsemem cases
> really makes it much nastier.
>
> That said, all of that code is really old. Most of it goes back to
> -05/06 or so. But since you seem to be able to reproduce at least back
> to 4.8, I guess this bug does back years too.

I feel that the bug in 4.8 is a different one, though the reproducer is the same.

>
> But I'm adding Dave Hansen explicitly to the cc, in case he has any
> ideas. Not because I blame him, but he's touched the sparsemem code
> fairly recently, so maybe he'd have some idea on adding sanity
> checking to the sparsemem version of pfn_to_page().
>
> > I dont know why but selecting CONFIG_FLATMEM=y seems to avoid a different bug
> > where bootup of qemu randomly fails at
>
> Hmm. That looks very different indeed. But if CONFIG_SPARSEMEM
> (presumably together with HIGHMEM) has some odd off-by-one corner case
> or similar, who knows *what* issues it could trigger.

It turned out that CONFIG_FLATMEM was irrelevant. I just did not hit it.

2018-01-16 19:30:58

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Tue, Jan 16, 2018 at 12:06 AM, Dave Hansen
<[email protected]> wrote:
> On 01/15/2018 06:14 PM, Linus Torvalds wrote:
>> But I'm adding Dave Hansen explicitly to the cc, in case he has any
>> ideas. Not because I blame him, but he's touched the sparsemem code
>> fairly recently, so maybe he'd have some idea on adding sanity
>> checking to the sparsemem version of pfn_to_page().
>
> I swear I haven't touched it lately!

Heh. I did

git blame -C mm/sparse.c | grep 2017

and your name shows up at the beginning a lot because of commit
c4e1be9ec113 ("mm, sparsemem: break out of loops early").

And Michal Hocko (who shows up even more) was already on the cc.

> I'm not sure I'd go after pfn_to_page(). *Maybe* if we were close to
> the places where we've done a pfn_to_page(), but I'm not seeing those.

Fair enough. I just wanted to add debugging, looked at Tetsuo's
config, and went "no way am I adding debugging to the sparsemem case
because it's so confusing".

That said, I also started looking at "kmap_to_page()". That's
something that is *really* different with HIGHMEM, and while most of
the users are in random drivers that do crazy things, I do note that
one of the users is in mm/swap.c.

That thing goes back to commit 5a178119b0fb ("mm: add support for
direct_IO to highmem pages") and was only used for swap_writepage(),
if I read this right.

That swap_writepage() use of kmap()'ed patches was removed some time
later in commit 62a8067a7f35 ("bio_vec-backed iov_iter"), but the
crazy kmap_to_page() thing remained.

I see nothing actively wrong in there, but it really feels like a
"that is all bogus" thing to me.

> Did anyone else notice the
>
> [ 31.068198] ? vmalloc_sync_all+0x150/0x150
>
> present in a bunch of the stack traces? That should be pretty uncommon.

No, didn't notice that. And yes, vmalloc_sync_all() might be interesting.

> Is it just part of the normal do_page_fault() stack and the stack
> dumper picks up on it?

I don't think so. It should *not* happen normally. The fact that it
shows up in the trace means it happened that time.

It doesn't seem HIGHMEM-related, though. Or maybe the highmem signal
is bogus too, and it's just that Tetsuo's reproducer needs magical
timing.

Linus

2018-01-16 19:34:50

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Tue, Jan 16, 2018 at 9:33 AM, Tetsuo Handa
<[email protected]> wrote:
>
> Since I got a faster reproducer, I tried full bisection between 4.11 and 4.12-rc1.
> But I have no idea why bisection arrives at c0332694903a37cf.

I don't think your reproducer is 100% reliable.

And bisection is great because it's very aggressive and optimal when
it comes to testing. But that also implies that if *any* of the
good/bad choices were incorrect, then the end result is pure garbage
and isn't even *close* to the right commit.

> It turned out that CONFIG_FLATMEM was irrelevant. I just did not hit it.

So have you actually been able to see the problem with FLATMEM, or is
this based on the bisect? Because I really think the bisect is pretty
much guaranteed to be wrong.

Linus

2018-01-17 11:09:01

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

Linus Torvalds wrote:
> > It turned out that CONFIG_FLATMEM was irrelevant. I just did not hit it.
>
> So have you actually been able to see the problem with FLATMEM, or is
> this based on the bisect? Because I really think the bisect is pretty
> much guaranteed to be wrong.

Oops, this "it" is "a different bug where bootup of qemu randomly hangs at

[ 0.001000] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
[ 0.003000] ..MP-BIOS bug: 8254 timer not connected to IO-APIC
[ 0.003000] ...trying to set up timer (IRQ0) through the 8259A ...
[ 0.003000] ..... (found apic 0 pin 2) ...
[ 0.005000] ....... failed.
[ 0.005000] ...trying to set up timer as Virtual Wire IRQ...
[ 13.120000] random: crng init done

". This bug occurs with both CONFIG_FLATMEM=y or CONFIG_SPARSEMEM=y .

> On Tue, Jan 16, 2018 at 9:33 AM, Tetsuo Handa
> <[email protected]> wrote:
> >
> > Since I got a faster reproducer, I tried full bisection between 4.11 and 4.12-rc1.
> > But I have no idea why bisection arrives at c0332694903a37cf.
>
> I don't think your reproducer is 100% reliable.
>
> And bisection is great because it's very aggressive and optimal when
> it comes to testing. But that also implies that if *any* of the
> good/bad choices were incorrect, then the end result is pure garbage
> and isn't even *close* to the right commit.

OK. I missed the mark. I overlooked that 4.11 already has this problem.

----------
[ 40.272926] BUG: unable to handle kernel paging request at f6d74b44
[ 40.272934] IP: page_remove_rmap+0x7/0x2c0
[ 40.272935] *pde = 3732c067
[ 40.272936] *pte = 36d74062
[ 40.272936]
[ 40.272938] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 40.272939] Modules linked in: xfs libcrc32c sr_mod cdrom sd_mod ata_generic pata_acpi serio_raw mptspi scsi_transport_spi mptscsih e1000 mptbase ata_piix libata
[ 40.272952] CPU: 6 PID: 719 Comm: b.out Not tainted 4.11.0 #266
[ 40.272952] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
[ 40.272953] task: ef4c1e40 task.stack: ef578000
[ 40.272955] EIP: page_remove_rmap+0x7/0x2c0
[ 40.272956] EFLAGS: 00010246 CPU: 6
[ 40.272957] EAX: f6d74b30 EBX: f6d74b30 ECX: 00000000 EDX: 00000000
[ 40.272958] ESI: ef7d9640 EDI: 00000093 EBP: ef579a78 ESP: ef579a70
[ 40.272959] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[ 40.272961] CR0: 80050033 CR2: f6d74b44 CR3: 2f4a8000 CR4: 000406d0
[ 40.273046] Call Trace:
[ 40.273050] try_to_unmap_one+0x206/0x4f0
[ 40.273055] rmap_walk_file+0x13c/0x270
[ 40.273057] rmap_walk+0x32/0x60
[ 40.273058] try_to_unmap+0xad/0x150
[ 40.273060] ? page_remove_rmap+0x2c0/0x2c0
[ 40.273062] ? page_not_mapped+0x10/0x10
[ 40.273063] ? page_get_anon_vma+0x90/0x90
[ 40.273066] shrink_page_list+0x37a/0xd10
[ 40.273069] shrink_inactive_list+0x173/0x370
[ 40.273072] shrink_node_memcg+0x572/0x7d0
[ 40.273074] ? shrink_slab+0x1a0/0x2e0
[ 40.273077] shrink_node+0xb3/0x2c0
[ 40.273079] do_try_to_free_pages+0xb2/0x2b0
[ 40.273081] try_to_free_pages+0x1a4/0x2f0
[ 40.273085] ? schedule_timeout+0x142/0x200
[ 40.273088] __alloc_pages_slowpath+0x360/0x7e6
[ 40.273091] __alloc_pages_nodemask+0x1a4/0x1b0
[ 40.273093] do_anonymous_page+0xcb/0x500
[ 40.273120] ? xfs_filemap_fault+0x36/0x40 [xfs]
[ 40.273122] handle_mm_fault+0x52f/0x990
[ 40.273125] __do_page_fault+0x19c/0x460
[ 40.273127] ? __do_page_fault+0x460/0x460
[ 40.273129] do_page_fault+0x1a/0x20
[ 40.273131] common_exception+0x6c/0x72
[ 40.273133] EIP: 0x8048437
[ 40.273133] EFLAGS: 00010202 CPU: 6
[ 40.273134] EAX: 001f8000 EBX: 7ff00000 ECX: 37803008 EDX: 00000000
[ 40.273135] ESI: 7ff00000 EDI: 00000000 EBP: bfeffa68 ESP: bfeffa30
[ 40.273137] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b
[ 40.273137] Code: ff ff ba 78 50 7a c1 89 d8 e8 a6 f8 fe ff 0f 0b 83 e8 01 e9 66 ff ff ff 8d b6 00 00 00 00 8d bf 00 00 00 00 55 89 e5 56 53 89 c3 <8b> 40 14 a8 01 0f 85 a4 01 00 00 89 d8 f6 40 04 01 74 5e 84 d2
[ 40.273161] EIP: page_remove_rmap+0x7/0x2c0 SS:ESP: 0068:ef579a70
[ 40.273162] CR2: 00000000f6d74b44
[ 40.273164] ---[ end trace 902077077bed43fd ]---
----------

I needed to bisect between 4.10 and 4.11, and I got plausible culprit.

----------
# bad: [a351e9b9fc24e982ec2f0e76379a49826036da12] Linux 4.11
# good: [c470abd4fde40ea6a0846a2beab642a578c0b8cd] Linux 4.10
# good: [69973b830859bc6529a7a0468ba0d80ee5117826] Linux 4.9
# good: [c8d2bc9bc39ebea8437fd974fdbc21847bb897a3] Linux 4.8
# good: [523d939ef98fd712632d93a5a2b588e477a7565e] Linux 4.7
# good: [2dcd0af568b0cf583645c8a317dd12e344b1c72a] Linux 4.6
# good: [b562e44f507e863c6792946e4e1b1449fbbac85d] Linux 4.5
# good: [afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc] Linux 4.4
# good: [6a13feb9c82803e2b815eca72fa7a9f5561d7861] Linux 4.3
# good: [64291f7db5bd8150a74ad2036f1037e6a0428df2] Linux 4.2
# good: [b953c0d234bc72e8489d3bf51a276c5c4ec85345] Linux 4.1
# good: [39a8804455fb23f09157341d3ba7db6d7ae6ee76] Linux 4.0
git bisect start 'v4.11' 'v4.10' 'v4.9' 'v4.8' 'v4.7' 'v4.6' 'v4.5' 'v4.4' 'v4.3' 'v4.2' 'v4.1' 'v4.0' 'mm/'
# good: [1b096e5ae9f7181c770d59c6895f23a76c63adee] z3fold: extend compaction function
git bisect good 1b096e5ae9f7181c770d59c6895f23a76c63adee
# good: [8703e8a465b1e9cadc3680b4b1248f5987e54518] sched/headers: Prepare for new header dependencies before moving code to <linux/sched/user.h>
git bisect good 8703e8a465b1e9cadc3680b4b1248f5987e54518
# bad: [8fe3ccaed080a7804bc459c42e0419253973be92] Merge branch 'akpm' (patches from Andrew)
git bisect bad 8fe3ccaed080a7804bc459c42e0419253973be92
# good: [1827adb11ad26b2290dc9fe2aaf54976b2439865] Merge branch 'WIP.sched-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 1827adb11ad26b2290dc9fe2aaf54976b2439865
# bad: [8346242a7e32c4c93316684ad8f45473932586b9] rmap: fix NULL-pointer dereference on THP munlocking
git bisect bad 8346242a7e32c4c93316684ad8f45473932586b9
# bad: [ce9311cf95ad8baf044a014738d76973d93b739a] mm/vmstats: add thp_split_pud event for clarity
git bisect bad ce9311cf95ad8baf044a014738d76973d93b739a
# good: [590dce2d4934fb909b112cd80c80486362337744] Merge branch 'rebased-statx' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
git bisect good 590dce2d4934fb909b112cd80c80486362337744
# bad: [b4fb8f66f1ae2e167d06c12d018025a8d4d3ba7e] mm, page_alloc: Add missing check for memory holes
git bisect bad b4fb8f66f1ae2e167d06c12d018025a8d4d3ba7e
# first bad commit: [b4fb8f66f1ae2e167d06c12d018025a8d4d3ba7e] mm, page_alloc: Add missing check for memory holes
----------

I haven't completed bisecting between b4fb8f66f1ae2e16 and c470abd4fde40ea6, but
b4fb8f66f1ae2e16 ("mm, page_alloc: Add missing check for memory holes") and
13ad59df67f19788 ("mm, page_alloc: avoid page_to_pfn() when merging buddies")
are talking about memory holes, which matches the situation that I'm trivially
hitting the bug if CONFIG_SPARSEMEM=y .

----------
# bad: [b4fb8f66f1ae2e167d06c12d018025a8d4d3ba7e] mm, page_alloc: Add missing check for memory holes
# good: [590dce2d4934fb909b112cd80c80486362337744] Merge branch 'rebased-statx' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
# good: [1827adb11ad26b2290dc9fe2aaf54976b2439865] Merge branch 'WIP.sched-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
# good: [8703e8a465b1e9cadc3680b4b1248f5987e54518] sched/headers: Prepare for new header dependencies before moving code to <linux/sched/user.h>
# good: [1b096e5ae9f7181c770d59c6895f23a76c63adee] z3fold: extend compaction function
# good: [c470abd4fde40ea6a0846a2beab642a578c0b8cd] Linux 4.10
# good: [69973b830859bc6529a7a0468ba0d80ee5117826] Linux 4.9
# good: [c8d2bc9bc39ebea8437fd974fdbc21847bb897a3] Linux 4.8
# good: [523d939ef98fd712632d93a5a2b588e477a7565e] Linux 4.7
# good: [2dcd0af568b0cf583645c8a317dd12e344b1c72a] Linux 4.6
# good: [b562e44f507e863c6792946e4e1b1449fbbac85d] Linux 4.5
# good: [afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc] Linux 4.4
# good: [6a13feb9c82803e2b815eca72fa7a9f5561d7861] Linux 4.3
# good: [64291f7db5bd8150a74ad2036f1037e6a0428df2] Linux 4.2
# good: [b953c0d234bc72e8489d3bf51a276c5c4ec85345] Linux 4.1
# good: [39a8804455fb23f09157341d3ba7db6d7ae6ee76] Linux 4.0
# bad: [8a9172356f747dc3734cc8043a44bbe158f44749] Merge branch 'timers-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
# bad: [04bb94b13c02e9dbc92d622cddf88937127bd7ed] overlayfs: remove now unnecessary header file include
# bad: [bd0f9b356d00aa241ced36fb075a07041c28d3b8] sched/headers: fix up header file dependency on <linux/sched/signal.h>
# bad: [ec3b93ae0bf4742e9cbb40e1964129926c1464e0] Merge branch 'x86-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
----------

----------
[ 30.997908] a.out invoked oom-killer: gfp_mask=0x14280ca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), nodemask=(null), order=0, oom_score_adj=0
[ 31.002850] CPU: 0 PID: 274 Comm: a.out Not tainted 4.11.0-rc1-00068-g8a91723 #341
[ 31.005963] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
[ 31.010320] Call Trace:
[ 31.011391] dump_stack+0x58/0x73
[ 31.012774] dump_header+0x92/0x202
[ 31.014319] ? ___ratelimit+0x83/0xf0
[ 31.015854] oom_kill_process+0x1e5/0x3b0
[ 31.017682] ? has_capability_noaudit+0x1f/0x30
[ 31.019629] ? oom_badness+0xcf/0x130
[ 31.021182] ? oom_evaluate_task+0x14/0xe0
[ 31.022909] out_of_memory+0xe1/0x280
[ 31.024460] __alloc_pages_nodemask+0x70d/0x9d0
[ 31.026347] ? vma_gap_callbacks_rotate+0x14/0x20
[ 31.028292] handle_mm_fault+0x599/0xd90
[ 31.029926] __do_page_fault+0x197/0x450
[ 31.031480] ? __do_page_fault+0x450/0x450
[ 31.032999] do_page_fault+0x1a/0x20
[ 31.034639] common_exception+0x6c/0x72
[ 31.036266] EIP: 0x804858f
[ 31.037430] EFLAGS: 00010202 CPU: 0
[ 31.038921] EAX: 007bc000 EBX: 376aa008 ECX: 37e66008 EDX: 00000000
[ 31.041530] ESI: 7ff00000 EDI: 00000000 EBP: bfcc7368 ESP: bfcc7330
[ 31.044118] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b
[ 31.046387] Mem-Info:
[ 31.047394] active_anon:656090 inactive_anon:0 isolated_anon:0
[ 31.047394] active_file:0 inactive_file:0 isolated_file:3
[ 31.047394] unevictable:0 dirty:0 writeback:0 unstable:0
[ 31.047394] slab_reclaimable:173 slab_unreclaimable:20805
[ 31.047394] mapped:4 shmem:0 pagetables:38151 bounce:0
[ 31.047394] free:31109 free_pcp:171 free_cma:0
[ 31.060114] Node 0 active_anon:2624360kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB isolated(file):12kB mapped:16kB dirty:0kB writeback:0kB shmem:0kB shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB unstable:0kB pages_scanned:151 all_unreclaimable? yes
[ 31.071561] DMA free:12720kB min:788kB low:984kB high:1180kB active_anon:3196kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB present:15992kB managed:15916kB mlocked:0kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
[ 31.080790] BUG: unable to handle kernel paging request at bc25106a
[ 31.080799] IP: page_remove_rmap+0x17/0x260
[ 31.080800] *pde = 00000000
[ 31.080801]
[ 31.080803] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 31.080804] Modules linked in:
[ 31.080807] CPU: 1 PID: 61 Comm: kswapd0 Not tainted 4.11.0-rc1-00068-g8a91723 #341
[ 31.080808] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
[ 31.080809] task: f517e300 task.stack: f5346000
[ 31.080812] EIP: page_remove_rmap+0x17/0x260
[ 31.080812] EFLAGS: 00010286 CPU: 1
[ 31.080814] EAX: bc251066 EBX: f3f2dbf0 ECX: 01de4080 EDX: 00000000
[ 31.080815] ESI: f3f2dbf0 EDI: f5970080 EBP: f5347c24 ESP: f5347c18
[ 31.080816] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[ 31.080817] CR0: 80050033 CR2: bc25106a CR3: 0170f000 CR4: 000406d0
[ 31.080897] Call Trace:
[ 31.080902] try_to_unmap_one+0x210/0x4b0
[ 31.080905] rmap_walk_file+0xf0/0x200
[ 31.080907] rmap_walk+0x32/0x60
[ 31.080909] try_to_unmap+0x95/0x120
[ 31.080910] ? page_remove_rmap+0x260/0x260
[ 31.080912] ? page_not_mapped+0x10/0x10
[ 31.080914] ? page_get_anon_vma+0x90/0x90
[ 31.080916] shrink_page_list+0x387/0xbf0
[ 31.080919] shrink_inactive_list+0x173/0x370
[ 31.080921] shrink_node_memcg+0x572/0x7d0
[ 31.080924] ? __list_lru_count_one.isra.7+0x14/0x40
[ 31.080927] ? tick_program_event+0x3b/0x70
[ 31.080929] shrink_node+0xb3/0x2c0
[ 31.080931] kswapd+0x27f/0x5a0
[ 31.080935] kthread+0xd1/0x100
[ 31.080936] ? mem_cgroup_shrink_node+0xa0/0xa0
[ 31.080938] ? kthread_park+0x70/0x70
[ 31.080942] ret_from_fork+0x21/0x2c
[ 31.080943] Code: 83 e8 01 90 e9 7a ff ff ff 83 e8 01 e9 60 ff ff ff 8d 76 00 55 89 e5 56 53 89 c3 83 ec 04 8b 40 14 a8 01 0f 85 00 02 00 00 89 d8 <f6> 40 04 01 74 6b 84 d2 0f 85 3b 01 00 00 f0 83 43 0c ff 78 0c
[ 31.080981] EIP: page_remove_rmap+0x17/0x260 SS:ESP: 0068:f5347c18
[ 31.080981] CR2: 00000000bc25106a
[ 31.080983] ---[ end trace 53061eeda268128d ]---
[ 31.159021] lowmem_reserve[]: 0 827 3011 3011
[ 31.160883] Normal free:111096kB min:42088kB low:52608kB high:63128kB active_anon:385820kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB present:892920kB managed:847288kB mlocked:0kB slab_reclaimable:692kB slab_unreclaimable:83220kB kernel_stack:74800kB pagetables:152604kB bounce:0kB free_pcp:680kB local_pcp:120kB free_cma:0kB
[ 31.173443] lowmem_reserve[]: 0 0 17471 17471
[ 31.175185] HighMem free:620kB min:512kB low:28284kB high:56056kB active_anon:2233384kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB present:2236360kB managed:2236360kB mlocked:0kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
[ 31.186860] lowmem_reserve[]: 0 0 0 0
[ 31.188445] DMA: 2*4kB (UM) 1*8kB (U) 0*16kB 1*32kB (U) 2*64kB (U) 2*128kB (UM) 2*256kB (UM) 1*512kB (M) 1*1024kB (U) 1*2048kB (U) 2*4096kB (M) = 12720kB
[ 31.194038] Normal: 41*4kB (UME) 114*8kB (UME) 58*16kB (UE) 23*32kB (UME) 22*64kB (UME) 11*128kB (UME) 6*256kB (UM) 2*512kB (UE) 1*1024kB (U) 1*2048kB (E) 24*4096kB (M) = 109492kB
[ 31.200324] HighMem: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
[ 31.204414] Node 0 hugepages_total=0 hugepages_free=0 hugepages_surp=0 hugepages_size=4096kB
[ 31.207586] 3 total pagecache pages
[ 31.209085] 0 pages in swap cache
[ 31.210527] Swap cache stats: add 0, delete 0, find 0/0
[ 31.212706] Free swap = 0kB
[ 31.213952] Total swap = 0kB
[ 31.215203] 786318 pages RAM
[ 31.216439] 559090 pages HighMem/MovableOnly
[ 31.218281] 11427 pages reserved
[ 31.219486] 0 pages cma reserved
[ 31.220862] Out of memory: Kill process 129 (a.out) score 5 or sacrifice child
[ 31.223843] Killed process 180 (a.out) total-vm:2108kB, anon-rss:56kB, file-rss:0kB, shmem-rss:0kB
[ 31.231024] oom_reaper: reaped process 180 (a.out), now anon-rss:0kB, file-rss:0kB, shmem-rss:0kB
----------

Thus, I call for an attention by speculative execution. ;-)

2018-01-17 21:40:33

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Wed, Jan 17, 2018 at 3:08 AM, Tetsuo Handa
<[email protected]> wrote:
>
> I needed to bisect between 4.10 and 4.11, and I got plausible culprit.
> [...]
> git bisect bad b4fb8f66f1ae2e167d06c12d018025a8d4d3ba7e
> # first bad commit: [b4fb8f66f1ae2e167d06c12d018025a8d4d3ba7e] mm, page_alloc: Add missing check for memory holes

Ok, that is indeed much more likely, and very much matches the whole
"this problem only happens with sparsemem" issue.

In fact, the whole

pfn_valid_within(buddy_pfn)

test looks very odd. Maybe the pfn of the buddy is valid, but it's not
in the same zone? Then we'd combine the two pages in two different
zones into one combined page.

Maybe that's why HIGHMEM matters? The low DMA zone is obviously
aligned in the whole PAGE_ORDER range. But the highmem zone might not
be. I used to know the highmem code, but I've happily forgotten
everything. But I think we end up deciding on some random non-aligned
number in the 900MB range as being the limit between the regular zone
and the HIGHMEM zone.

So maybe something like this to test the theory?

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 76c9688b6a0a..f919a5548943 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -756,6 +756,8 @@ static inline void rmv_page_order(struct page *page)
static inline int page_is_buddy(struct page *page, struct page *buddy,
unsigned int order)
{
+ if (WARN_ON_ONCE(page_zone(page) != page_zone(buddy)))
+ return 0;
if (page_is_guard(buddy) && page_order(buddy) == order) {
if (page_zone_id(page) != page_zone_id(buddy))
return 0;

I don't know. Does that warning trigger for you?

The above is completely untested. It might not compile. If it compiles
it might not work. And even if it "works", it might not matter,
because perhaps the boundary between regular memory and HIGHMEM is
already sufficiently aligned.

Comments?

Linus

2018-01-17 21:52:36

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Wed, Jan 17, 2018 at 1:39 PM, Linus Torvalds
<[email protected]> wrote:
>
> In fact, the whole
>
> pfn_valid_within(buddy_pfn)
>
> test looks very odd. Maybe the pfn of the buddy is valid, but it's not
> in the same zone? Then we'd combine the two pages in two different
> zones into one combined page.

It might also be the same allocation zone, but if the pfn's are in
different sparsemem sections that would also be problematic.

But I hope/assume that all sparsemem sections are always aligned to
(PAGE_SIZE << MAXORDER).

In contrast, the ZONE_HIGHMEM limit really does seems to be
potentially not aligned to anything, ie

arch/x86/include/asm/pgtable_32_types.h:
#define MAXMEM (VMALLOC_END - PAGE_OFFSET - __VMALLOC_RESERVE)

which I have no idea what the alignment is, but VMALLOC_END at least
does not seem to have any MAXORDER alignment.

So it really does look like the zone for two page orders that would
otherwise be buddies might actually be different.

Interesting if this really is the case. Because afaik, if that
WARN_ON_ONCE actually triggers, it does seem like this bug could go
back pretty much forever.

In fact, it seems to be such a fundamental bug that I suspect I'm
entirely wrong, and full of shit. So it's an interesting and not
_obviously_ incorrect theory, but I suspect I must be missing
something.

Linus

2018-01-17 22:03:07

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On 01/17/2018 01:39 PM, Linus Torvalds wrote:
>
> So maybe something like this to test the theory?
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 76c9688b6a0a..f919a5548943 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -756,6 +756,8 @@ static inline void rmv_page_order(struct page *page)
> static inline int page_is_buddy(struct page *page, struct page *buddy,
> unsigned int order)
> {
> + if (WARN_ON_ONCE(page_zone(page) != page_zone(buddy)))
> + return 0;
> if (page_is_guard(buddy) && page_order(buddy) == order) {
> if (page_zone_id(page) != page_zone_id(buddy))
> return 0;

I thought that page_zone_id() stuff was there to prevent this kind of
cross-zone stuff from happening.

2018-01-17 22:05:40

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On 01/17/2018 01:51 PM, Linus Torvalds wrote:
> In fact, it seems to be such a fundamental bug that I suspect I'm
> entirely wrong, and full of shit. So it's an interesting and not
> _obviously_ incorrect theory, but I suspect I must be missing
> something.

I'll just note that a few of the pfns I decoded were smack in the middle
of the zone, not near either the high or low end of ZONE_NORMAL where we
would expect this cross-zone stuff to happen.

But I guess we could get similar wonkiness where 'struct page' is
screwed up in so many different ways if during buddy joining you do:

list_del(&buddy->lru);

and 'buddy' is off in another zone for which you do not hold the
spinlock. If we are somehow missing some locking, or double-allocating
a page, something like this would help:

static inline void rmv_page_order(struct page *page)
{
+ WARN_ON_ONCE(!PageBuddy(page));
__ClearPageBuddy(page);
set_page_private(page, 0);
}

2018-01-17 22:17:34

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Wed, Jan 17, 2018 at 2:00 PM, Dave Hansen
<[email protected]> wrote:
>
> I thought that page_zone_id() stuff was there to prevent this kind of
> cross-zone stuff from happening.

Ahh, that was the part I missed. Yeah looks like that checks things
properly. Although the mask generation is *so* confusing that I
stopped following it and will just take your word for it ;)

Linus

2018-01-18 08:13:28

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

Tetsuo Handa wrote:
> OK. I missed the mark. I overlooked that 4.11 already has this problem.
>
> I needed to bisect between 4.10 and 4.11, and I got plausible culprit.
>
> I haven't completed bisecting between b4fb8f66f1ae2e16 and c470abd4fde40ea6, but
> b4fb8f66f1ae2e16 ("mm, page_alloc: Add missing check for memory holes") and
> 13ad59df67f19788 ("mm, page_alloc: avoid page_to_pfn() when merging buddies")
> are talking about memory holes, which matches the situation that I'm trivially
> hitting the bug if CONFIG_SPARSEMEM=y .
>
> Thus, I call for an attention by speculative execution. ;-)

Speculative execution failed. I was confused by jiffies precision bug.
The final culprit is c7ab0d2fdc840266 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()").

----------------------------------------
[ 103.132986] BUG: unable to handle kernel paging request at b6c00171
[ 103.132996] IP: lock_page_memcg+0x3a/0x70
[ 103.132997] *pde = 00000000
[ 103.132997]
[ 103.132999] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 103.133000] Modules linked in: pcspkr shpchp serio_raw
[ 103.133006] CPU: 3 PID: 62 Comm: kswapd0 Not tainted 4.10.0-09628-gc7ab0d2-dirty #359
[ 103.133007] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
[ 103.133008] task: f4559b00 task.stack: f2c74000
[ 103.133011] EIP: lock_page_memcg+0x3a/0x70
[ 103.133012] EFLAGS: 00010282 CPU: 3
[ 103.133013] EAX: f3108060 EBX: b6c00001 ECX: 01c8e5a0 EDX: 00000000
[ 103.133014] ESI: f4afe5a0 EDI: f3108060 EBP: f2c75c10 ESP: f2c75c04
[ 103.133015] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[ 103.133017] CR0: 80050033 CR2: b6c00171 CR3: 0170a000 CR4: 000406d0
[ 103.133098] Call Trace:
[ 103.133104] page_remove_rmap+0x92/0x260
[ 103.133106] try_to_unmap_one+0x210/0x4b0
[ 103.133108] rmap_walk_file+0xf0/0x200
[ 103.133111] rmap_walk+0x32/0x60
[ 103.133112] try_to_unmap+0x95/0x120
[ 103.133114] ? page_remove_rmap+0x260/0x260
[ 103.133116] ? page_not_mapped+0x10/0x10
[ 103.133118] ? page_get_anon_vma+0x90/0x90
[ 103.133120] shrink_page_list+0x3af/0xc40
[ 103.133123] shrink_inactive_list+0x173/0x370
[ 103.133125] shrink_node_memcg+0x572/0x7d0
[ 103.133128] ? __list_lru_count_one.isra.5+0x14/0x40
[ 103.133130] shrink_node+0xb3/0x2c0
[ 103.133132] kswapd+0x27f/0x5a0
[ 103.133137] kthread+0xd1/0x100
[ 103.133139] ? mem_cgroup_shrink_node+0xa0/0xa0
[ 103.133140] ? kthread_park+0x70/0x70
[ 103.133144] ret_from_fork+0x21/0x2c
[ 103.133145] Code: 20 85 db 75 26 eb 2e 66 90 8d b3 74 01 00 00 89 f0 e8 9b 5a 37 00 3b 5f 20 74 26 89 c2 89 f0 e8 3d 57 37 00 8b 5f 20 85 db 74 0a <8b> 83 70 01 00 00 85 c0 7f d4 5b 5e 5f 5d f3 c3 8d b6 00 00 00
[ 103.133171] EIP: lock_page_memcg+0x3a/0x70 SS:ESP: 0068:f2c75c04
[ 103.133172] CR2: 00000000b6c00171
[ 103.133175] ---[ end trace fa59c5a5ab752d7a ]---
----------------------------------------

----------------------------------------
# bad: [86292b33d4b79ee03e2f43ea0381ef85f077c760] Merge branch 'akpm' (patches from Andrew)
# good: [13ad59df67f19788f6c22985b1a33e466eceb643] mm, page_alloc: avoid page_to_pfn() when merging buddies
git bisect start '86292b33d4b79ee03e2f43ea0381ef85f077c760' '13ad59df67f19788' 'mm/'
# good: [0262d9c845ec349edf93f69688a5129c36cc2232] memblock: embed memblock type name within struct memblock_type
git bisect good 0262d9c845ec349edf93f69688a5129c36cc2232
# good: [0262d9c845ec349edf93f69688a5129c36cc2232] memblock: embed memblock type name within struct memblock_type
git bisect good 0262d9c845ec349edf93f69688a5129c36cc2232
# bad: [897ab3e0c49e24b62e2d54d165c7afec6bbca65b] userfaultfd: non-cooperative: add event for memory unmaps
git bisect bad 897ab3e0c49e24b62e2d54d165c7afec6bbca65b
# good: [c791ace1e747371658237f0d30234fef56c39669] mm: replace FAULT_FLAG_SIZE with parameter to huge_fault
git bisect good c791ace1e747371658237f0d30234fef56c39669
# good: [8eaedede825a02dbe2420b9c9be9b5b2d7515496] mm: fix handling PTE-mapped THPs in page_referenced()
git bisect good 8eaedede825a02dbe2420b9c9be9b5b2d7515496
# bad: [36eaff3364e8cd35552a77ee426a8170f4f5fde9] mm, ksm: convert write_protect_page() to use page_vma_mapped_walk()
git bisect bad 36eaff3364e8cd35552a77ee426a8170f4f5fde9
# good: [a8fa41ad2f6f7ca08edd1afcf8149ae5a4dcf654] mm, rmap: check all VMAs that PTE-mapped THP can be part of
git bisect good a8fa41ad2f6f7ca08edd1afcf8149ae5a4dcf654
# bad: [c7ab0d2fdc840266b39db94538f74207ec2afbf6] mm: convert try_to_unmap_one() to use page_vma_mapped_walk()
git bisect bad c7ab0d2fdc840266b39db94538f74207ec2afbf6
# good: [f27176cfc363d395eea8dc5c4a26e5d6d7d65eaf] mm: convert page_mkclean_one() to use page_vma_mapped_walk()
git bisect good f27176cfc363d395eea8dc5c4a26e5d6d7d65eaf
# first bad commit: [c7ab0d2fdc840266b39db94538f74207ec2afbf6] mm: convert try_to_unmap_one() to use page_vma_mapped_walk()

bad 4.10.0-10531-g86292b3-dirty # 86292b33d4b79ee0 ("Merge branch 'akpm' (patches from Andrew)")
bad 4.10.0-09635-g897ab3e-dirty
bad 4.10.0-09629-g36eaff3-dirty
bad 4.10.0-09628-gc7ab0d2-dirty
good 4.10.0-09627-gf27176c-dirty
good 4.10.0-09626-ga8fa41ad-dirty
good 4.10.0-09624-g8eaeded-dirty
good 4.10.0-09611-gc791ace-dirty
good 4.10.0-09588-g0262d9c-dirty
good 4.10.0-06032-g13ad59d-dirty # 13ad59df67f19788 ("mm, page_alloc: avoid page_to_pfn() when merging buddies")
----------------------------------------

The "-dirty" part is due to testing with below patch (jiffies fix,
warn_alloc() lockup avoidance and disabling the OOM reaper).

----------------------------------------
--- a/kernel/time/jiffies.c
+++ b/kernel/time/jiffies.c
@@ -125,7 +125,7 @@ int register_refined_jiffies(long cycles_per_second)
shift_hz += cycles_per_tick/2;
do_div(shift_hz, cycles_per_tick);
/* Calculate nsec_per_tick using shift_hz */
- nsec_per_tick = (u64)TICK_NSEC << 8;
+ nsec_per_tick = (u64)NSEC_PER_SEC << 8;
nsec_per_tick += (u32)shift_hz/2;
do_div(nsec_per_tick, (u32)shift_hz);

--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -553,7 +553,7 @@ static void oom_reap_task(struct task_struct *tsk)
struct mm_struct *mm = tsk->signal->oom_mm;

/* Retry the down_read_trylock(mmap_sem) a few times */
- while (attempts++ < MAX_OOM_REAP_RETRIES && !__oom_reap_task_mm(tsk, mm))
+ //while (attempts++ < MAX_OOM_REAP_RETRIES && !__oom_reap_task_mm(tsk, mm))
schedule_timeout_idle(HZ/10);

if (attempts <= MAX_OOM_REAP_RETRIES)
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3765,7 +3765,7 @@ bool gfp_pfmemalloc_allowed(gfp_t gfp_mask)
goto nopage;

/* Make sure we know about allocations which stall for too long */
- if (time_after(jiffies, alloc_start + stall_timeout)) {
+ if (0 && time_after(jiffies, alloc_start + stall_timeout)) {
warn_alloc(gfp_mask, ac->nodemask,
"page allocation stalls for %ums, order:%u",
jiffies_to_msecs(jiffies-alloc_start), order);
----------------------------------------

Michal Hocko wrote:
> Yes, this looks like somebody is clobbering the page. I've seen one with
> refcount 0 so I though this would be a ref count issue. But the one
> below looks definitely like a memory corruption. A nasty one to debug :/

Yes, I guess that c7ab0d2fdc840266 is somewhere doing wrong ref count check.

2018-01-18 13:02:20

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 05:12:45PM +0900, Tetsuo Handa wrote:
> Tetsuo Handa wrote:
> > OK. I missed the mark. I overlooked that 4.11 already has this problem.
> >
> > I needed to bisect between 4.10 and 4.11, and I got plausible culprit.
> >
> > I haven't completed bisecting between b4fb8f66f1ae2e16 and c470abd4fde40ea6, but
> > b4fb8f66f1ae2e16 ("mm, page_alloc: Add missing check for memory holes") and
> > 13ad59df67f19788 ("mm, page_alloc: avoid page_to_pfn() when merging buddies")
> > are talking about memory holes, which matches the situation that I'm trivially
> > hitting the bug if CONFIG_SPARSEMEM=y .
> >
> > Thus, I call for an attention by speculative execution. ;-)
>
> Speculative execution failed. I was confused by jiffies precision bug.
> The final culprit is c7ab0d2fdc840266 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()").

I think I've tracked it down. check_pte() in mm/page_vma_mapped.c doesn't
work as intended.

I've added instrumentation below to prove it.

The BUG() triggers with following output:

[ 10.084024] diff: -858690919
[ 10.084258] hpage_nr_pages: 1
[ 10.084386] check1: 0
[ 10.084478] check2: 0

Basically, pte_page(*pvmw->pte) is below pvmw->page, but
(pte_page(*pvmw->pte) < pvmw->page) doesn't catch it.

Well, I can see how C lawyer can argue that you can only compare pointers
of the same memory object which is not the case here. But this is kinda
insane.

Any suggestions how to rewrite it in a way that compiler would
understand?

diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index d22b84310f6d..57b4397f1ea5 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -70,6 +70,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
}
if (pte_page(*pvmw->pte) < pvmw->page)
return false;
+
+ if (pte_page(*pvmw->pte) - pvmw->page) {
+ printk("diff: %d\n", pte_page(*pvmw->pte) - pvmw->page);
+ printk("hpage_nr_pages: %d\n", hpage_nr_pages(pvmw->page));
+ printk("check1: %d\n", pte_page(*pvmw->pte) - pvmw->page < 0);
+ printk("check2: %d\n", pte_page(*pvmw->pte) - pvmw->page >= hpage_nr_pages(pvmw->page));
+ BUG();
+ }
}

return true;
--
Kirill A. Shutemov

2018-01-18 14:35:33

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 04:12:10PM +0300, Kirill A. Shutemov wrote:
> On Thu, Jan 18, 2018 at 03:25:50PM +0300, Kirill A. Shutemov wrote:
> > On Thu, Jan 18, 2018 at 05:12:45PM +0900, Tetsuo Handa wrote:
> > > Tetsuo Handa wrote:
> > > > OK. I missed the mark. I overlooked that 4.11 already has this problem.
> > > >
> > > > I needed to bisect between 4.10 and 4.11, and I got plausible culprit.
> > > >
> > > > I haven't completed bisecting between b4fb8f66f1ae2e16 and c470abd4fde40ea6, but
> > > > b4fb8f66f1ae2e16 ("mm, page_alloc: Add missing check for memory holes") and
> > > > 13ad59df67f19788 ("mm, page_alloc: avoid page_to_pfn() when merging buddies")
> > > > are talking about memory holes, which matches the situation that I'm trivially
> > > > hitting the bug if CONFIG_SPARSEMEM=y .
> > > >
> > > > Thus, I call for an attention by speculative execution. ;-)
> > >
> > > Speculative execution failed. I was confused by jiffies precision bug.
> > > The final culprit is c7ab0d2fdc840266 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()").
> >
> > I think I've tracked it down. check_pte() in mm/page_vma_mapped.c doesn't
> > work as intended.
> >
> > I've added instrumentation below to prove it.
> >
> > The BUG() triggers with following output:
> >
> > [ 10.084024] diff: -858690919
> > [ 10.084258] hpage_nr_pages: 1
> > [ 10.084386] check1: 0
> > [ 10.084478] check2: 0
> >
> > Basically, pte_page(*pvmw->pte) is below pvmw->page, but
> > (pte_page(*pvmw->pte) < pvmw->page) doesn't catch it.
> >
> > Well, I can see how C lawyer can argue that you can only compare pointers
> > of the same memory object which is not the case here. But this is kinda
> > insane.
> >
> > Any suggestions how to rewrite it in a way that compiler would
> > understand?
>
> The patch below makes the crash go away for me.
>
> But this is situation is scary. So we cannot compare arbitrary pointers in
> kernel?
>
> Don't we rely on this for lock ordering in some cases? Like in
> mutex_lock_double()?
>
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index d22b84310f6d..1f0f512fd127 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -51,6 +51,8 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
> WARN_ON_ONCE(1);
> #endif
> } else {
> + unsigned long ptr1, ptr2;
> +
> if (is_swap_pte(*pvmw->pte)) {
> swp_entry_t entry;
>
> @@ -63,12 +65,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
> if (!pte_present(*pvmw->pte))
> return false;
>
> - /* THP can be referenced by any subpage */
> - if (pte_page(*pvmw->pte) - pvmw->page >=
> - hpage_nr_pages(pvmw->page)) {
> + ptr1 = (unsigned long)pte_page(*pvmw->pte);
> + ptr2 = (unsigned long)pvmw->page;
> +
> + if (ptr1 < ptr2)
> return false;
> - }
> - if (pte_page(*pvmw->pte) < pvmw->page)
> +
> + /* THP can be referenced by any subpage */
> + if (ptr1 - ptr2 >= hpage_nr_pages(pvmw->page))

Arghhh.. It has to be

if (ptr1 - ptr2 >= hpage_nr_pages(pvmw->page) * sizeof(*pvmw->page))

--
Kirill A. Shutemov

2018-01-18 14:40:19

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On 01/18/2018 05:12 AM, Kirill A. Shutemov wrote:
> - if (pte_page(*pvmw->pte) - pvmw->page >=
> - hpage_nr_pages(pvmw->page)) {

Is ->pte guaranteed to map a page which is within the same section as
pvmw->page? Otherwise, with sparsemem (non-vmemmap), the pointer
arithmetic won't work.

WARN_ON_ONCE(page_to_section(pvmw->page) !=
page_to_section(pte_page(*pvmw->pte));

Will detect that.

2018-01-18 14:45:45

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On 01/18/2018 04:25 AM, Kirill A. Shutemov wrote:
> [ 10.084024] diff: -858690919
> [ 10.084258] hpage_nr_pages: 1
> [ 10.084386] check1: 0
> [ 10.084478] check2: 0
...
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index d22b84310f6d..57b4397f1ea5 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -70,6 +70,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
> }
> if (pte_page(*pvmw->pte) < pvmw->page)
> return false;
> +
> + if (pte_page(*pvmw->pte) - pvmw->page) {
> + printk("diff: %d\n", pte_page(*pvmw->pte) - pvmw->page);
> + printk("hpage_nr_pages: %d\n", hpage_nr_pages(pvmw->page));
> + printk("check1: %d\n", pte_page(*pvmw->pte) - pvmw->page < 0);
> + printk("check2: %d\n", pte_page(*pvmw->pte) - pvmw->page >= hpage_nr_pages(pvmw->page));
> + BUG();
> + }

This says that pte_page(*pvmw->pte) and pvmw->page are roughly 4GB away
from each other (858690919*4=0xccba559c0). That's not the compiler
being wonky, it just means that the virtual addresses of the memory
sections are that far apart.

This won't happen when you have vmemmap or flatmem because the mem_map[]
is virtually contiguous and pointer arithmetic just works against all
'struct page' pointers. But with classic sparsemem, it doesn't.

You need to make sure that the PFNs are in the same section before you
can do the math that you want to do here.

2018-01-18 14:46:12

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 06:38:10AM -0800, Dave Hansen wrote:
> On 01/18/2018 05:12 AM, Kirill A. Shutemov wrote:
> > - if (pte_page(*pvmw->pte) - pvmw->page >=
> > - hpage_nr_pages(pvmw->page)) {
>
> Is ->pte guaranteed to map a page which is within the same section as
> pvmw->page? Otherwise, with sparsemem (non-vmemmap), the pointer
> arithmetic won't work.

No, it's not guaranteed. It can be arbitrary page.

The arithmetic won't work because they are different "memory objects"?

--
Kirill A. Shutemov

2018-01-18 14:53:15

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On 01/18/2018 06:45 AM, Kirill A. Shutemov wrote:
> On Thu, Jan 18, 2018 at 06:38:10AM -0800, Dave Hansen wrote:
>> On 01/18/2018 05:12 AM, Kirill A. Shutemov wrote:
>>> - if (pte_page(*pvmw->pte) - pvmw->page >=
>>> - hpage_nr_pages(pvmw->page)) {
>> Is ->pte guaranteed to map a page which is within the same section as
>> pvmw->page? Otherwise, with sparsemem (non-vmemmap), the pointer
>> arithmetic won't work.
> No, it's not guaranteed. It can be arbitrary page.
>
> The arithmetic won't work because they are different "memory objects"?

No, because sections' mem_map[]s can be allocated non-contiguously.
Section 1's might be a lower virtual address than Section 0's.

They're allocated not unlike this:

mem_section[0]->section_mem_map = kmalloc(SECTION_SIZE);
mem_section[1]->section_mem_map = kmalloc(SECTION_SIZE);
...

The first pfn in section 1 and the last pfn in section 0 are adjacent
PFNs, but their 'struct page' might have virtual addresses that are TB
apart.

2018-01-18 14:59:46

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 03:25:50PM +0300, Kirill A. Shutemov wrote:
> On Thu, Jan 18, 2018 at 05:12:45PM +0900, Tetsuo Handa wrote:
> > Tetsuo Handa wrote:
> > > OK. I missed the mark. I overlooked that 4.11 already has this problem.
> > >
> > > I needed to bisect between 4.10 and 4.11, and I got plausible culprit.
> > >
> > > I haven't completed bisecting between b4fb8f66f1ae2e16 and c470abd4fde40ea6, but
> > > b4fb8f66f1ae2e16 ("mm, page_alloc: Add missing check for memory holes") and
> > > 13ad59df67f19788 ("mm, page_alloc: avoid page_to_pfn() when merging buddies")
> > > are talking about memory holes, which matches the situation that I'm trivially
> > > hitting the bug if CONFIG_SPARSEMEM=y .
> > >
> > > Thus, I call for an attention by speculative execution. ;-)
> >
> > Speculative execution failed. I was confused by jiffies precision bug.
> > The final culprit is c7ab0d2fdc840266 ("mm: convert try_to_unmap_one() to use page_vma_mapped_walk()").
>
> I think I've tracked it down. check_pte() in mm/page_vma_mapped.c doesn't
> work as intended.
>
> I've added instrumentation below to prove it.
>
> The BUG() triggers with following output:
>
> [ 10.084024] diff: -858690919
> [ 10.084258] hpage_nr_pages: 1
> [ 10.084386] check1: 0
> [ 10.084478] check2: 0
>
> Basically, pte_page(*pvmw->pte) is below pvmw->page, but
> (pte_page(*pvmw->pte) < pvmw->page) doesn't catch it.
>
> Well, I can see how C lawyer can argue that you can only compare pointers
> of the same memory object which is not the case here. But this is kinda
> insane.
>
> Any suggestions how to rewrite it in a way that compiler would
> understand?

The patch below makes the crash go away for me.

But this is situation is scary. So we cannot compare arbitrary pointers in
kernel?

Don't we rely on this for lock ordering in some cases? Like in
mutex_lock_double()?

diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index d22b84310f6d..1f0f512fd127 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -51,6 +51,8 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
WARN_ON_ONCE(1);
#endif
} else {
+ unsigned long ptr1, ptr2;
+
if (is_swap_pte(*pvmw->pte)) {
swp_entry_t entry;

@@ -63,12 +65,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
if (!pte_present(*pvmw->pte))
return false;

- /* THP can be referenced by any subpage */
- if (pte_page(*pvmw->pte) - pvmw->page >=
- hpage_nr_pages(pvmw->page)) {
+ ptr1 = (unsigned long)pte_page(*pvmw->pte);
+ ptr2 = (unsigned long)pvmw->page;
+
+ if (ptr1 < ptr2)
return false;
- }
- if (pte_page(*pvmw->pte) < pvmw->page)
+
+ /* THP can be referenced by any subpage */
+ if (ptr1 - ptr2 >= hpage_nr_pages(pvmw->page))
return false;
}

--
Kirill A. Shutemov

2018-01-18 15:01:13

by Andrea Arcangeli

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

2018-01-18 16:29:49

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 06:45:00AM -0800, Dave Hansen wrote:
> On 01/18/2018 04:25 AM, Kirill A. Shutemov wrote:
> > [ 10.084024] diff: -858690919
> > [ 10.084258] hpage_nr_pages: 1
> > [ 10.084386] check1: 0
> > [ 10.084478] check2: 0
> ...
> > diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> > index d22b84310f6d..57b4397f1ea5 100644
> > --- a/mm/page_vma_mapped.c
> > +++ b/mm/page_vma_mapped.c
> > @@ -70,6 +70,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
> > }
> > if (pte_page(*pvmw->pte) < pvmw->page)
> > return false;
> > +
> > + if (pte_page(*pvmw->pte) - pvmw->page) {
> > + printk("diff: %d\n", pte_page(*pvmw->pte) - pvmw->page);
> > + printk("hpage_nr_pages: %d\n", hpage_nr_pages(pvmw->page));
> > + printk("check1: %d\n", pte_page(*pvmw->pte) - pvmw->page < 0);
> > + printk("check2: %d\n", pte_page(*pvmw->pte) - pvmw->page >= hpage_nr_pages(pvmw->page));
> > + BUG();
> > + }
>
> This says that pte_page(*pvmw->pte) and pvmw->page are roughly 4GB away
> from each other (858690919*4=0xccba559c0). That's not the compiler
> being wonky, it just means that the virtual addresses of the memory
> sections are that far apart.
>
> This won't happen when you have vmemmap or flatmem because the mem_map[]
> is virtually contiguous and pointer arithmetic just works against all
> 'struct page' pointers. But with classic sparsemem, it doesn't.
>
> You need to make sure that the PFNs are in the same section before you
> can do the math that you want to do here.

Something like this?

From 251e124630da82482e8b320c73162ce89af04d5d Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <[email protected]>
Date: Thu, 18 Jan 2018 18:24:07 +0300
Subject: [PATCH] mm, page_vma_mapped: Fix pointer arithmetics in check_pte()

Tetsuo reported random crashes under memory pressure on 32-bit x86
system and tracked down to change that introduced
page_vma_mapped_walk().

The root cause of the issue is the faulty pointer math in check_pte().
As ->pte may point to an arbitrary page we have to check that they are
belong to the section before doing math. Otherwise it may lead to weird
results.

It wasn't noticed until now as mem_map[] is virtually contiguous on flatmem or
vmemmap sparsemem. Pointer arithmetic just works against all 'struct page'
pointers. But with classic sparsemem, it doesn't.

Let's restructure code a bit and add necessary check.

Signed-off-by: Kirill A. Shutemov <[email protected]>
Reported-by: Tetsuo Handa <[email protected]>
Fixes: ace71a19cec5 ("mm: introduce page_vma_mapped_walk()")
Cc: [email protected]
---
mm/page_vma_mapped.c | 66 +++++++++++++++++++++++++++++++++++-----------------
1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index d22b84310f6d..de195dcdfbd8 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -30,8 +30,28 @@ static bool map_pte(struct page_vma_mapped_walk *pvmw)
return true;
}

+/**
+ * check_pte - check if @pvmw->page is mapped at the @pvmw->pte
+ *
+ * page_vma_mapped_walk() found a place where @pvmw->page is *potentially*
+ * mapped. check_pte() has to validate this.
+ *
+ * @pvmw->pte may point to empty PTE, swap PTE or PTE pointing to arbitrary
+ * page.
+ *
+ * If PVMW_MIGRATION flag is set, returns true if @pvmw->pte contains migration
+ * entry that points to @pvmw->page or any subpage in case of THP.
+ *
+ * If PVMW_MIGRATION flag is not set, returns true if @pvmw->pte points to
+ * @pvmw->page or any subpage in case of THP.
+ *
+ * Otherwise, return false.
+ *
+ */
static bool check_pte(struct page_vma_mapped_walk *pvmw)
{
+ struct page *page;
+
if (pvmw->flags & PVMW_MIGRATION) {
#ifdef CONFIG_MIGRATION
swp_entry_t entry;
@@ -41,37 +61,41 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)

if (!is_migration_entry(entry))
return false;
- if (migration_entry_to_page(entry) - pvmw->page >=
- hpage_nr_pages(pvmw->page)) {
- return false;
- }
- if (migration_entry_to_page(entry) < pvmw->page)
- return false;
+
+ page = migration_entry_to_page(entry);
#else
WARN_ON_ONCE(1);
#endif
- } else {
- if (is_swap_pte(*pvmw->pte)) {
- swp_entry_t entry;
+ } else if (is_swap_pte(*pvmw->pte)) {
+ swp_entry_t entry;

- entry = pte_to_swp_entry(*pvmw->pte);
- if (is_device_private_entry(entry) &&
- device_private_entry_to_page(entry) == pvmw->page)
- return true;
- }
+ /* Handle un-addressable ZONE_DEVICE memory */
+ entry = pte_to_swp_entry(*pvmw->pte);
+ if (!is_device_private_entry(entry))
+ return false;

+ page = device_private_entry_to_page(entry);
+ } else {
if (!pte_present(*pvmw->pte))
return false;

- /* THP can be referenced by any subpage */
- if (pte_page(*pvmw->pte) - pvmw->page >=
- hpage_nr_pages(pvmw->page)) {
- return false;
- }
- if (pte_page(*pvmw->pte) < pvmw->page)
- return false;
+ page = pte_page(*pvmw->pte);
}

+ /*
+ * Make sure that pages are in the same section before doing pointer
+ * arithmetics.
+ */
+ if (page_to_section(pvmw->page) != page_to_section(page))
+ return false;
+
+ if (page < pvmw->page)
+ return false;
+
+ /* THP can be referenced by any subpage */
+ if (page - pvmw->page >= hpage_nr_pages(pvmw->page))
+ return false;
+
return true;
}

--
Kirill A. Shutemov

2018-01-18 16:59:43

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 03:58:30PM +0100, Andrea Arcangeli wrote:
> On Thu, Jan 18, 2018 at 06:45:00AM -0800, Dave Hansen wrote:
> > On 01/18/2018 04:25 AM, Kirill A. Shutemov wrote:
> > > [ 10.084024] diff: -858690919
> > > [ 10.084258] hpage_nr_pages: 1
> > > [ 10.084386] check1: 0
> > > [ 10.084478] check2: 0
> > ...
> > > diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> > > index d22b84310f6d..57b4397f1ea5 100644
> > > --- a/mm/page_vma_mapped.c
> > > +++ b/mm/page_vma_mapped.c
> > > @@ -70,6 +70,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
> > > }
> > > if (pte_page(*pvmw->pte) < pvmw->page)
> > > return false;
> > > +
> > > + if (pte_page(*pvmw->pte) - pvmw->page) {
> > > + printk("diff: %d\n", pte_page(*pvmw->pte) - pvmw->page);
> > > + printk("hpage_nr_pages: %d\n", hpage_nr_pages(pvmw->page));
> > > + printk("check1: %d\n", pte_page(*pvmw->pte) - pvmw->page < 0);
> > > + printk("check2: %d\n", pte_page(*pvmw->pte) - pvmw->page >= hpage_nr_pages(pvmw->page));
> > > + BUG();
> > > + }
> >
> > This says that pte_page(*pvmw->pte) and pvmw->page are roughly 4GB away
> > from each other (858690919*4=0xccba559c0). That's not the compiler
> > being wonky, it just means that the virtual addresses of the memory
> > sections are that far apart.
> >
> > This won't happen when you have vmemmap or flatmem because the mem_map[]
> > is virtually contiguous and pointer arithmetic just works against all
> > 'struct page' pointers. But with classic sparsemem, it doesn't.
> >
> > You need to make sure that the PFNs are in the same section before you
> > can do the math that you want to do here.
>
> Isn't it simply that pvmw->page isn't a page or pte_page(*pvmw->pte)
> isn't a page?
>
> The distance cannot matter, MMU isn't involved, this is pure 64bit
> aritmetics, 1giga 1 terabyte, 48bits 5level pagetables are meaningless
> in this comparison.

Note, it's 32-bit.

>
> #include <stdio.h>
>
> int main()
> {
> volatile long i;
> struct x { char a[4000000000]; };
> for (i = 0; i < 4000000000*3; i += 4000000000) {
> printf("%ld\n", ((struct x *)0)-((((struct x *)i))));
> }
> printf("xxxx\n");
> for (i = 0; i < 4000000000; i += 1) {
> if (i==4)
> i = 4000000000;
> printf("%ld\n", ((struct x *)0)-((((struct x *)i))));
> }
> return 0;
> }
>
> You need to add two debug checks on "pte_page(*pvmw->pte) % 64" and
> same for pvmw->page to find out the one of the two that isn't a page.
>
> If both are real pages there's a bug that allocates page structs not
> naturally aligned.

Both are real page. But why do you expect pages to be 64-byte alinged?
Both are aligned to 64-bit as they suppose to be IIUC.

I can't say I fully grasp how 'diff' got this value and how it leads to both
checks being false.

[ 10.209657] page:f6e4cc38 count:8 mapcount:6 mapping:f5818f94 index:0x0
[ 10.209989] flags: 0x3501004d(locked|referenced|uptodate|active|mappedtodisk)
[ 10.210496] raw: 3501004d f5818f94 00000000 00000005 00000008 00000100 00000200 00000000
[ 10.210742] raw: f5c06600 00000000
[ 10.210863] page dumped because: pvmw->page
[ 10.210992] page->mem_cgroup:f5c06600
[ 10.211192] page:f74749d8 count:1 mapcount:1 mapping:f54612d1 index:0x0
[ 10.211381] flags: 0x15040068(uptodate|lru|active|swapbacked)
[ 10.211530] raw: 15040068 f54612d1 00000000 00000000 00000001 f74749c4 f75b9014 00000000
[ 10.211729] raw: f5c06600 00000000
[ 10.211806] page dumped because: pte_page(*pvmw->pte)
[ 10.211920] page->mem_cgroup:f5c06600
[ 10.212079] diff: -858832092
[ 10.212184] hpage_nr_pages: 1
[ 10.212284] check1: 0
[ 10.212384] check2: 0

--
Kirill A. Shutemov

2018-01-18 17:00:33

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 6:38 AM, Dave Hansen
<[email protected]> wrote:
> On 01/18/2018 05:12 AM, Kirill A. Shutemov wrote:
>> - if (pte_page(*pvmw->pte) - pvmw->page >=
>> - hpage_nr_pages(pvmw->page)) {
>
> Is ->pte guaranteed to map a page which is within the same section as
> pvmw->page? Otherwise, with sparsemem (non-vmemmap), the pointer
> arithmetic won't work.

Lovely.

Finally a reason for this bug that actually seems to make sense.

Thanks guys.

Tetsuo - does Kirill's latest patch fix this for you? The one with

Subject: [PATCH] mm, page_vma_mapped: Fix pointer arithmetics in check_pte()

in the body of the email? I'm really hoping it does, since this seems
to make sense.

Linus

2018-01-18 17:25:01

by Michal Hocko

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu 18-01-18 18:40:26, Kirill A. Shutemov wrote:
[...]
> + /*
> + * Make sure that pages are in the same section before doing pointer
> + * arithmetics.
> + */
> + if (page_to_section(pvmw->page) != page_to_section(page))
> + return false;

OK, THPs shouldn't cross memory sections AFAIK. My brain is meltdown
these days so this might be a completely stupid question. But why don't
you simply compare pfns? This would be just simpler, no?

> +
> + if (page < pvmw->page)
> + return false;
> +
> + /* THP can be referenced by any subpage */
> + if (page - pvmw->page >= hpage_nr_pages(pvmw->page))
> + return false;
> +
> return true;
> }
>
> --
> Kirill A. Shutemov

--
Michal Hocko
SUSE Labs

2018-01-18 17:27:02

by Luck, Tony

[permalink] [raw]

Subject: RE: [mm 4.15-rc8] Random oopses under memory pressure.

> Both are real page. But why do you expect pages to be 64-byte alinged?
> Both are aligned to 64-bit as they suppose to be IIUC.

On a 64-bit kernel sizeof struct page == 64 (after much work by people to
trim out excess stuff). So I thought we made sure to align the base address
of blocks of "struct page" so that every one neatly fits into one cache line.

-Tony

2018-01-18 17:27:08

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 8:56 AM, Kirill A. Shutemov
<[email protected]> wrote:
>
> I can't say I fully grasp how 'diff' got this value and how it leads to both
> checks being false.

I think the problem is that page difference when they are in different sections.

When you do

pte_page(*pvmw->pte) - pvmw->page

then the compiler takes the pointer difference, and then divides by
the size of "struct page" to get an index.

But - and this is important - it does so knowing that the division it
does will have no modulus: the two 'struct page *' pointers are really
in the same array, and they really are 'n*sizeof(struct page)' apart
for some 'n'.

That means that the compiler can optimize the division. In fact, for
this case, gcc will generate

subl %ebx, %eax
sarl $3, %eax
imull $-858993459, %eax, %eax

because 'struct page' is 40 bytes in size, and that magic sequence
happens to divide by 40 (first divide by 8, then that magical "imull"
will divide by 5 *IFF* the thing is evenly divisible by 5 (and not too
big - but the shift guarantees that).

Basically, it's a magic trick, because real divides are very
expensive, but you can fake them more quickly if you can limit the
input domain.

But what does it mean if the two "struct page *" are not in the same
array, and the two arrays were allocated not aligned exactly 40 bytes
away, but some random number of pages away?

You get *COMPLETE*GARBAGE* when you do the above optimized divide.
Suddenly the divide had a modulus (because the base of the two arrays
weren't 40-byte aligned), and the "trick" doesn't work.

So that's why you can't do pointer diffs between two arrays. Not
because you can't subtract the two pointers, but because the
*division* part of the C pointer diff rules leads to issues.

Linus

2018-01-18 17:29:49

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 9:26 AM, Luck, Tony <[email protected]> wrote:
>> Both are real page. But why do you expect pages to be 64-byte alinged?
>> Both are aligned to 64-bit as they suppose to be IIUC.
>
> On a 64-bit kernel sizeof struct page == 64 (after much work by people to
> trim out excess stuff). So I thought we made sure to align the base address
> of blocks of "struct page" so that every one neatly fits into one cache line.

The bug happens on 32-bit, and a 'struct page' is not 64-byte aligned
there at all.

See my other email about the explanation of why "page1 - page2"
doesn't work when they aren't mutually aligned to the actual size of
the 'struct page'.

Linus

2018-01-18 23:51:48

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 09:26:25AM -0800, Linus Torvalds wrote:
> On Thu, Jan 18, 2018 at 8:56 AM, Kirill A. Shutemov
> <[email protected]> wrote:
> >
> > I can't say I fully grasp how 'diff' got this value and how it leads to both
> > checks being false.
>
> I think the problem is that page difference when they are in different sections.
>
> When you do
>
> pte_page(*pvmw->pte) - pvmw->page
>
> then the compiler takes the pointer difference, and then divides by
> the size of "struct page" to get an index.
>
> But - and this is important - it does so knowing that the division it
> does will have no modulus: the two 'struct page *' pointers are really
> in the same array, and they really are 'n*sizeof(struct page)' apart
> for some 'n'.
>
> That means that the compiler can optimize the division. In fact, for
> this case, gcc will generate
>
> subl %ebx, %eax
> sarl $3, %eax
> imull $-858993459, %eax, %eax
>
> because 'struct page' is 40 bytes in size, and that magic sequence
> happens to divide by 40 (first divide by 8, then that magical "imull"
> will divide by 5 *IFF* the thing is evenly divisible by 5 (and not too
> big - but the shift guarantees that).
>
> Basically, it's a magic trick, because real divides are very
> expensive, but you can fake them more quickly if you can limit the
> input domain.
>
> But what does it mean if the two "struct page *" are not in the same
> array, and the two arrays were allocated not aligned exactly 40 bytes
> away, but some random number of pages away?
>
> You get *COMPLETE*GARBAGE* when you do the above optimized divide.
> Suddenly the divide had a modulus (because the base of the two arrays
> weren't 40-byte aligned), and the "trick" doesn't work.
>
> So that's why you can't do pointer diffs between two arrays. Not
> because you can't subtract the two pointers, but because the
> *division* part of the C pointer diff rules leads to issues.

Thanks a lot for the explanation!

I wounder if this may be a problem in other places?

For instance, perf uses address of a mutex to determinate the lock
ordering. See mutex_lock_double(). The mutex is embedded into struct
perf_event_context, which is allocated with kzalloc() so I don't see how
we can presume that alignment is consistent between them.

I don't think it's the only example in kernel. Are we just lucky?

--
Kirill A. Shutemov

2018-01-19 02:02:44

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

Kirill A. Shutemov wrote:
> Something like this?
>
>
> From 251e124630da82482e8b320c73162ce89af04d5d Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <[email protected]>
> Date: Thu, 18 Jan 2018 18:24:07 +0300
> Subject: [PATCH] mm, page_vma_mapped: Fix pointer arithmetics in check_pte()
>
> Tetsuo reported random crashes under memory pressure on 32-bit x86
> system and tracked down to change that introduced
> page_vma_mapped_walk().
>
> The root cause of the issue is the faulty pointer math in check_pte().
> As ->pte may point to an arbitrary page we have to check that they are
> belong to the section before doing math. Otherwise it may lead to weird
> results.
>
> It wasn't noticed until now as mem_map[] is virtually contiguous on flatmem or
> vmemmap sparsemem. Pointer arithmetic just works against all 'struct page'
> pointers. But with classic sparsemem, it doesn't.
>
> Let's restructure code a bit and add necessary check.
>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
> Reported-by: Tetsuo Handa <[email protected]>
> Fixes: ace71a19cec5 ("mm: introduce page_vma_mapped_walk()")
> Cc: [email protected]

This patch solves the problem. Thank you.

Tested-by: Tetsuo Handa <[email protected]>

> ---
> mm/page_vma_mapped.c | 66 +++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 45 insertions(+), 21 deletions(-)
>
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index d22b84310f6d..de195dcdfbd8 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -30,8 +30,28 @@ static bool map_pte(struct page_vma_mapped_walk *pvmw)
> return true;
> }
>
> +/**
> + * check_pte - check if @pvmw->page is mapped at the @pvmw->pte
> + *
> + * page_vma_mapped_walk() found a place where @pvmw->page is *potentially*
> + * mapped. check_pte() has to validate this.
> + *
> + * @pvmw->pte may point to empty PTE, swap PTE or PTE pointing to arbitrary
> + * page.
> + *
> + * If PVMW_MIGRATION flag is set, returns true if @pvmw->pte contains migration
> + * entry that points to @pvmw->page or any subpage in case of THP.
> + *
> + * If PVMW_MIGRATION flag is not set, returns true if @pvmw->pte points to
> + * @pvmw->page or any subpage in case of THP.
> + *
> + * Otherwise, return false.
> + *
> + */
> static bool check_pte(struct page_vma_mapped_walk *pvmw)
> {
> + struct page *page;
> +
> if (pvmw->flags & PVMW_MIGRATION) {
> #ifdef CONFIG_MIGRATION
> swp_entry_t entry;
> @@ -41,37 +61,41 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
>
> if (!is_migration_entry(entry))
> return false;
> - if (migration_entry_to_page(entry) - pvmw->page >=
> - hpage_nr_pages(pvmw->page)) {
> - return false;
> - }
> - if (migration_entry_to_page(entry) < pvmw->page)
> - return false;
> +
> + page = migration_entry_to_page(entry);
> #else
> WARN_ON_ONCE(1);
> #endif
> - } else {
> - if (is_swap_pte(*pvmw->pte)) {
> - swp_entry_t entry;
> + } else if (is_swap_pte(*pvmw->pte)) {
> + swp_entry_t entry;
>
> - entry = pte_to_swp_entry(*pvmw->pte);
> - if (is_device_private_entry(entry) &&
> - device_private_entry_to_page(entry) == pvmw->page)
> - return true;
> - }
> + /* Handle un-addressable ZONE_DEVICE memory */
> + entry = pte_to_swp_entry(*pvmw->pte);
> + if (!is_device_private_entry(entry))
> + return false;
>
> + page = device_private_entry_to_page(entry);
> + } else {
> if (!pte_present(*pvmw->pte))
> return false;
>
> - /* THP can be referenced by any subpage */
> - if (pte_page(*pvmw->pte) - pvmw->page >=
> - hpage_nr_pages(pvmw->page)) {
> - return false;
> - }
> - if (pte_page(*pvmw->pte) < pvmw->page)
> - return false;
> + page = pte_page(*pvmw->pte);
> }
>
> + /*
> + * Make sure that pages are in the same section before doing pointer
> + * arithmetics.
> + */
> + if (page_to_section(pvmw->page) != page_to_section(page))
> + return false;
> +
> + if (page < pvmw->page)
> + return false;
> +
> + /* THP can be referenced by any subpage */
> + if (page - pvmw->page >= hpage_nr_pages(pvmw->page))
> + return false;
> +
> return true;
> }

2018-01-19 10:04:30

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Thu, Jan 18, 2018 at 06:22:13PM +0100, Michal Hocko wrote:
> On Thu 18-01-18 18:40:26, Kirill A. Shutemov wrote:
> [...]
> > + /*
> > + * Make sure that pages are in the same section before doing pointer
> > + * arithmetics.
> > + */
> > + if (page_to_section(pvmw->page) != page_to_section(page))
> > + return false;
>
> OK, THPs shouldn't cross memory sections AFAIK. My brain is meltdown
> these days so this might be a completely stupid question. But why don't
> you simply compare pfns? This would be just simpler, no?

In original code, we already had pvmw->page around and I thought it would
be easier to get page for the pte intead of looking for pfn for both
sides.

We these changes it's no longer the case.

Do you care enough to send a patch? :)

--
Kirill A. Shutemov

2018-01-19 10:34:37

by Michal Hocko

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Fri 19-01-18 13:02:59, Kirill A. Shutemov wrote:
> On Thu, Jan 18, 2018 at 06:22:13PM +0100, Michal Hocko wrote:
> > On Thu 18-01-18 18:40:26, Kirill A. Shutemov wrote:
> > [...]
> > > + /*
> > > + * Make sure that pages are in the same section before doing pointer
> > > + * arithmetics.
> > > + */
> > > + if (page_to_section(pvmw->page) != page_to_section(page))
> > > + return false;
> >
> > OK, THPs shouldn't cross memory sections AFAIK. My brain is meltdown
> > these days so this might be a completely stupid question. But why don't
> > you simply compare pfns? This would be just simpler, no?
>
> In original code, we already had pvmw->page around and I thought it would
> be easier to get page for the pte intead of looking for pfn for both
> sides.
>
> We these changes it's no longer the case.
>
> Do you care enough to send a patch? :)

Well, memory sections are sparsemem concept IIRC. Unless I've missed
something page_to_section is quarded by SECTION_IN_PAGE_FLAGS and that
is conditional to CONFIG_SPARSEMEM. THP is a generic code so using it
there is wrong unless I miss some subtle detail here.

Comparing pfn should be generic enough.
--
Michal Hocko
SUSE Labs

2018-01-19 11:50:11

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Fri, Jan 19, 2018 at 11:33:42AM +0100, Michal Hocko wrote:
> On Fri 19-01-18 13:02:59, Kirill A. Shutemov wrote:
> > On Thu, Jan 18, 2018 at 06:22:13PM +0100, Michal Hocko wrote:
> > > On Thu 18-01-18 18:40:26, Kirill A. Shutemov wrote:
> > > [...]
> > > > + /*
> > > > + * Make sure that pages are in the same section before doing pointer
> > > > + * arithmetics.
> > > > + */
> > > > + if (page_to_section(pvmw->page) != page_to_section(page))
> > > > + return false;
> > >
> > > OK, THPs shouldn't cross memory sections AFAIK. My brain is meltdown
> > > these days so this might be a completely stupid question. But why don't
> > > you simply compare pfns? This would be just simpler, no?
> >
> > In original code, we already had pvmw->page around and I thought it would
> > be easier to get page for the pte intead of looking for pfn for both
> > sides.
> >
> > We these changes it's no longer the case.
> >
> > Do you care enough to send a patch? :)
>
> Well, memory sections are sparsemem concept IIRC. Unless I've missed
> something page_to_section is quarded by SECTION_IN_PAGE_FLAGS and that
> is conditional to CONFIG_SPARSEMEM. THP is a generic code so using it
> there is wrong unless I miss some subtle detail here.
>
> Comparing pfn should be generic enough.

Good point.

What about something like this?

From 861f68c555b87fd6c0ccc3428ace91b7e185b73a Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <[email protected]>
Date: Thu, 18 Jan 2018 18:24:07 +0300
Subject: [PATCH] mm, page_vma_mapped: Drop faulty pointer arithmetics in
check_pte()

Tetsuo reported random crashes under memory pressure on 32-bit x86
system and tracked down to change that introduced
page_vma_mapped_walk().

The root cause of the issue is the faulty pointer math in check_pte().
As ->pte may point to an arbitrary page we have to check that they are
belong to the section before doing math. Otherwise it may lead to weird
results.

It wasn't noticed until now as mem_map[] is virtually contiguous on flatmem or
vmemmap sparsemem. Pointer arithmetic just works against all 'struct page'
pointers. But with classic sparsemem, it doesn't.

Let's restructure code a bit and replace pointer arithmetic with
operations on pfns.

Signed-off-by: Kirill A. Shutemov <[email protected]>
Reported-by: Tetsuo Handa <[email protected]>
Fixes: ace71a19cec5 ("mm: introduce page_vma_mapped_walk()")
Cc: [email protected]
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
include/linux/swapops.h | 21 ++++++++++++++++++
mm/page_vma_mapped.c | 59 +++++++++++++++++++++++++++++++------------------
2 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/include/linux/swapops.h b/include/linux/swapops.h
index 9c5a2628d6ce..1d3877c39a00 100644
--- a/include/linux/swapops.h
+++ b/include/linux/swapops.h
@@ -124,6 +124,11 @@ static inline bool is_write_device_private_entry(swp_entry_t entry)
return unlikely(swp_type(entry) == SWP_DEVICE_WRITE);
}

+static inline unsigned long device_private_entry_to_pfn(swp_entry_t entry)
+{
+ return swp_offset(entry);
+}
+
static inline struct page *device_private_entry_to_page(swp_entry_t entry)
{
return pfn_to_page(swp_offset(entry));
@@ -154,6 +159,11 @@ static inline bool is_write_device_private_entry(swp_entry_t entry)
return false;
}

+static inline unsigned long device_private_entry_to_pfn(swp_entry_t entry)
+{
+ return 0;
+}
+
static inline struct page *device_private_entry_to_page(swp_entry_t entry)
{
return NULL;
@@ -189,6 +199,11 @@ static inline int is_write_migration_entry(swp_entry_t entry)
return unlikely(swp_type(entry) == SWP_MIGRATION_WRITE);
}

+static inline unsigned long migration_entry_to_pfn(swp_entry_t entry)
+{
+ return swp_offset(entry);
+}
+
static inline struct page *migration_entry_to_page(swp_entry_t entry)
{
struct page *p = pfn_to_page(swp_offset(entry));
@@ -218,6 +233,12 @@ static inline int is_migration_entry(swp_entry_t swp)
{
return 0;
}
+
+static inline unsigned long migration_entry_to_pfn(swp_entry_t entry)
+{
+ return 0;
+}
+
static inline struct page *migration_entry_to_page(swp_entry_t entry)
{
return NULL;
diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index d22b84310f6d..072ae9bc5fee 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -30,8 +30,28 @@ static bool map_pte(struct page_vma_mapped_walk *pvmw)
return true;
}

+/**
+ * check_pte - check if @pvmw->page is mapped at the @pvmw->pte
+ *
+ * page_vma_mapped_walk() found a place where @pvmw->page is *potentially*
+ * mapped. check_pte() has to validate this.
+ *
+ * @pvmw->pte may point to empty PTE, swap PTE or PTE pointing to arbitrary
+ * page.
+ *
+ * If PVMW_MIGRATION flag is set, returns true if @pvmw->pte contains migration
+ * entry that points to @pvmw->page or any subpage in case of THP.
+ *
+ * If PVMW_MIGRATION flag is not set, returns true if @pvmw->pte points to
+ * @pvmw->page or any subpage in case of THP.
+ *
+ * Otherwise, return false.
+ *
+ */
static bool check_pte(struct page_vma_mapped_walk *pvmw)
{
+ unsigned long pfn;
+
if (pvmw->flags & PVMW_MIGRATION) {
#ifdef CONFIG_MIGRATION
swp_entry_t entry;
@@ -41,37 +61,34 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)

if (!is_migration_entry(entry))
return false;
- if (migration_entry_to_page(entry) - pvmw->page >=
- hpage_nr_pages(pvmw->page)) {
- return false;
- }
- if (migration_entry_to_page(entry) < pvmw->page)
- return false;
+
+ pfn = migration_entry_to_pfn(entry);
#else
WARN_ON_ONCE(1);
#endif
- } else {
- if (is_swap_pte(*pvmw->pte)) {
- swp_entry_t entry;
+ } else if (is_swap_pte(*pvmw->pte)) {
+ swp_entry_t entry;

- entry = pte_to_swp_entry(*pvmw->pte);
- if (is_device_private_entry(entry) &&
- device_private_entry_to_page(entry) == pvmw->page)
- return true;
- }
+ /* Handle un-addressable ZONE_DEVICE memory */
+ entry = pte_to_swp_entry(*pvmw->pte);
+ if (!is_device_private_entry(entry))
+ return false;

+ pfn = device_private_entry_to_pfn(entry);
+ } else {
if (!pte_present(*pvmw->pte))
return false;

- /* THP can be referenced by any subpage */
- if (pte_page(*pvmw->pte) - pvmw->page >=
- hpage_nr_pages(pvmw->page)) {
- return false;
- }
- if (pte_page(*pvmw->pte) < pvmw->page)
- return false;
+ pfn = pte_pfn(*pvmw->pte);
}

+ if (pfn < page_to_pfn(pvmw->page))
+ return false;
+
+ /* THP can be referenced by any subpage */
+ if (pfn - page_to_pfn(pvmw->page) >= hpage_nr_pages(pvmw->page))
+ return false;
+
return true;
}

--
Kirill A. Shutemov

2018-01-19 12:09:24

by Michal Hocko

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Fri 19-01-18 14:49:17, Kirill A. Shutemov wrote:
> On Fri, Jan 19, 2018 at 11:33:42AM +0100, Michal Hocko wrote:
> > On Fri 19-01-18 13:02:59, Kirill A. Shutemov wrote:
> > > On Thu, Jan 18, 2018 at 06:22:13PM +0100, Michal Hocko wrote:
> > > > On Thu 18-01-18 18:40:26, Kirill A. Shutemov wrote:
> > > > [...]
> > > > > + /*
> > > > > + * Make sure that pages are in the same section before doing pointer
> > > > > + * arithmetics.
> > > > > + */
> > > > > + if (page_to_section(pvmw->page) != page_to_section(page))
> > > > > + return false;
> > > >
> > > > OK, THPs shouldn't cross memory sections AFAIK. My brain is meltdown
> > > > these days so this might be a completely stupid question. But why don't
> > > > you simply compare pfns? This would be just simpler, no?
> > >
> > > In original code, we already had pvmw->page around and I thought it would
> > > be easier to get page for the pte intead of looking for pfn for both
> > > sides.
> > >
> > > We these changes it's no longer the case.
> > >
> > > Do you care enough to send a patch? :)
> >
> > Well, memory sections are sparsemem concept IIRC. Unless I've missed
> > something page_to_section is quarded by SECTION_IN_PAGE_FLAGS and that
> > is conditional to CONFIG_SPARSEMEM. THP is a generic code so using it
> > there is wrong unless I miss some subtle detail here.
> >
> > Comparing pfn should be generic enough.
>
> Good point.
>
> What about something like this?
>
> >From 861f68c555b87fd6c0ccc3428ace91b7e185b73a Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <[email protected]>
> Date: Thu, 18 Jan 2018 18:24:07 +0300
> Subject: [PATCH] mm, page_vma_mapped: Drop faulty pointer arithmetics in
> check_pte()
>
> Tetsuo reported random crashes under memory pressure on 32-bit x86
> system and tracked down to change that introduced
> page_vma_mapped_walk().
>
> The root cause of the issue is the faulty pointer math in check_pte().
> As ->pte may point to an arbitrary page we have to check that they are
> belong to the section before doing math. Otherwise it may lead to weird
> results.
>
> It wasn't noticed until now as mem_map[] is virtually contiguous on flatmem or
> vmemmap sparsemem. Pointer arithmetic just works against all 'struct page'
> pointers. But with classic sparsemem, it doesn't.

it doesn't because each section memap is allocated separately and so
consecutive pfns crossing two sections might have struct pages at
completely unrelated addresses.

> Let's restructure code a bit and replace pointer arithmetic with
> operations on pfns.
>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
> Reported-by: Tetsuo Handa <[email protected]>
> Fixes: ace71a19cec5 ("mm: introduce page_vma_mapped_walk()")
> Cc: [email protected]
> Signed-off-by: Kirill A. Shutemov <[email protected]>

The patch makes sense but there is one more thing to fix here.

[...]
> static bool check_pte(struct page_vma_mapped_walk *pvmw)
> {
> + unsigned long pfn;
> +
> if (pvmw->flags & PVMW_MIGRATION) {
> #ifdef CONFIG_MIGRATION
> swp_entry_t entry;
> @@ -41,37 +61,34 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
>
> if (!is_migration_entry(entry))
> return false;
> - if (migration_entry_to_page(entry) - pvmw->page >=
> - hpage_nr_pages(pvmw->page)) {
> - return false;
> - }
> - if (migration_entry_to_page(entry) < pvmw->page)
> - return false;
> +
> + pfn = migration_entry_to_pfn(entry);
> #else
> WARN_ON_ONCE(1);
> #endif
> - } else {

now you allow to pass through with uninitialized pfn. We used to return
true in that case so we should probably keep it in this WARN_ON_ONCE
case. Please note that I haven't studied this particular case and the
ifdef is definitely not an act of art but that is a separate topic.

> - if (is_swap_pte(*pvmw->pte)) {
> - swp_entry_t entry;
> + } else if (is_swap_pte(*pvmw->pte)) {
> + swp_entry_t entry;
>
> - entry = pte_to_swp_entry(*pvmw->pte);
> - if (is_device_private_entry(entry) &&
> - device_private_entry_to_page(entry) == pvmw->page)
> - return true;
> - }
> + /* Handle un-addressable ZONE_DEVICE memory */
> + entry = pte_to_swp_entry(*pvmw->pte);
> + if (!is_device_private_entry(entry))
> + return false;
>
> + pfn = device_private_entry_to_pfn(entry);
> + } else {
> if (!pte_present(*pvmw->pte))
> return false;
>
> - /* THP can be referenced by any subpage */
> - if (pte_page(*pvmw->pte) - pvmw->page >=
> - hpage_nr_pages(pvmw->page)) {
> - return false;
> - }
> - if (pte_page(*pvmw->pte) < pvmw->page)
> - return false;
> + pfn = pte_pfn(*pvmw->pte);
> }
>
> + if (pfn < page_to_pfn(pvmw->page))
> + return false;
> +
> + /* THP can be referenced by any subpage */
> + if (pfn - page_to_pfn(pvmw->page) >= hpage_nr_pages(pvmw->page))
> + return false;
> +
> return true;
> }
>
> --
> Kirill A. Shutemov

--
Michal Hocko
SUSE Labs

2018-01-19 12:32:27

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Fri, Jan 19, 2018 at 12:07:47PM +0000, Michal Hocko wrote:
> > >From 861f68c555b87fd6c0ccc3428ace91b7e185b73a Mon Sep 17 00:00:00 2001
> > From: "Kirill A. Shutemov" <[email protected]>
> > Date: Thu, 18 Jan 2018 18:24:07 +0300
> > Subject: [PATCH] mm, page_vma_mapped: Drop faulty pointer arithmetics in
> > check_pte()
> >
> > Tetsuo reported random crashes under memory pressure on 32-bit x86
> > system and tracked down to change that introduced
> > page_vma_mapped_walk().
> >
> > The root cause of the issue is the faulty pointer math in check_pte().
> > As ->pte may point to an arbitrary page we have to check that they are
> > belong to the section before doing math. Otherwise it may lead to weird
> > results.
> >
> > It wasn't noticed until now as mem_map[] is virtually contiguous on flatmem or
> > vmemmap sparsemem. Pointer arithmetic just works against all 'struct page'
> > pointers. But with classic sparsemem, it doesn't.
>
> it doesn't because each section memap is allocated separately and so
> consecutive pfns crossing two sections might have struct pages at
> completely unrelated addresses.

Okay, I'll amend it.

> > Let's restructure code a bit and replace pointer arithmetic with
> > operations on pfns.
> >
> > Signed-off-by: Kirill A. Shutemov <[email protected]>
> > Reported-by: Tetsuo Handa <[email protected]>
> > Fixes: ace71a19cec5 ("mm: introduce page_vma_mapped_walk()")
> > Cc: [email protected]
> > Signed-off-by: Kirill A. Shutemov <[email protected]>
>
> The patch makes sense but there is one more thing to fix here.
>
> [...]
> > static bool check_pte(struct page_vma_mapped_walk *pvmw)
> > {
> > + unsigned long pfn;
> > +
> > if (pvmw->flags & PVMW_MIGRATION) {
> > #ifdef CONFIG_MIGRATION
> > swp_entry_t entry;
> > @@ -41,37 +61,34 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
> >
> > if (!is_migration_entry(entry))
> > return false;
> > - if (migration_entry_to_page(entry) - pvmw->page >=
> > - hpage_nr_pages(pvmw->page)) {
> > - return false;
> > - }
> > - if (migration_entry_to_page(entry) < pvmw->page)
> > - return false;
> > +
> > + pfn = migration_entry_to_pfn(entry);
> > #else
> > WARN_ON_ONCE(1);
> > #endif
> > - } else {
>
> now you allow to pass through with uninitialized pfn. We used to return
> true in that case so we should probably keep it in this WARN_ON_ONCE
> case. Please note that I haven't studied this particular case and the
> ifdef is definitely not an act of art but that is a separate topic.

Good catch. Thanks.

I think returning true here is wrong as we don't validate in any way what
is mapped there. I'll put "return false;".

And I take a look if we can drop the #ifdef.

--
Kirill A. Shutemov

2018-01-19 12:56:14

by Matthew Wilcox

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Fri, Jan 19, 2018 at 02:49:55AM +0300, Kirill A. Shutemov wrote:
> > So that's why you can't do pointer diffs between two arrays. Not
> > because you can't subtract the two pointers, but because the
> > *division* part of the C pointer diff rules leads to issues.
>
> Thanks a lot for the explanation!
>
> I wounder if this may be a problem in other places?
>
> For instance, perf uses address of a mutex to determinate the lock
> ordering. See mutex_lock_double(). The mutex is embedded into struct
> perf_event_context, which is allocated with kzalloc() so I don't see how
> we can presume that alignment is consistent between them.
>
> I don't think it's the only example in kernel. Are we just lucky?

If you're just *comparing* the addresses of two objects, GCC doesn't
care what the size of the object is. ie there's a difference between
'if (b < a)' and 'if ((a - b) < n)'.

But yes, if you go by the strict wording of the standard:

When two pointers are compared, the result depends on the relative
locations in the address space of the objects pointed to. [...] In
all other cases, the behavior is undefined

http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf

So really we should be casting 'b' and 'a' to uintptr_t to be fully
compliant with the spec.

2018-01-19 18:43:15

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Fri, Jan 19, 2018 at 4:55 AM, Matthew Wilcox <[email protected]> wrote:
>
> So really we should be casting 'b' and 'a' to uintptr_t to be fully
> compliant with the spec.

That's an unnecessary technicality.

Any compiler that doesn't get pointer inequality testing right is not
worth even worrying about. We wouldn't want to use such a compiler,
because it's intentionally generating garbage just to f*ck with us.
Why would you go along with that?

So the only real issue is that pointer subtraction case.

I actually asked (long long ago) for an optinal compiler warning for
"pointer subtraction with non-power-of-2 sizes". Not because of it
being undefined, but simply because it's expensive. The
divide->multiply thing doesn't always work, and a real divide is
really quite expensive on many architectures.

We *should* be careful about it. I guess sparse could be made to warn,
but I'm afraid that we have so many of these things that a warning
isn't reasonable.

And most of the time, a pointer difference really is inside the same
array. The operation doesn't tend to make sense otherwise.

Linus

2018-01-19 22:16:14

by Al Viro

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Fri, Jan 19, 2018 at 10:42:18AM -0800, Linus Torvalds wrote:
> On Fri, Jan 19, 2018 at 4:55 AM, Matthew Wilcox <[email protected]> wrote:
> >
> > So really we should be casting 'b' and 'a' to uintptr_t to be fully
> > compliant with the spec.
>
> That's an unnecessary technicality.
>
> Any compiler that doesn't get pointer inequality testing right is not
> worth even worrying about. We wouldn't want to use such a compiler,
> because it's intentionally generating garbage just to f*ck with us.
> Why would you go along with that?
>
> So the only real issue is that pointer subtraction case.
>
> I actually asked (long long ago) for an optinal compiler warning for
> "pointer subtraction with non-power-of-2 sizes". Not because of it
> being undefined, but simply because it's expensive. The
> divide->multiply thing doesn't always work, and a real divide is
> really quite expensive on many architectures.
>
> We *should* be careful about it. I guess sparse could be made to warn,
> but I'm afraid that we have so many of these things that a warning
> isn't reasonable.

You mean like -Wptr-subtraction-blows?

FWIW, allmodconfig on amd64 with C=2 CF=-Wptr-subtraction-blows is not too large
The tail (alphabetically sorted) is
lib/dynamic_debug.c:1013:9: warning: potentially expensive pointer subtraction
lib/extable.c:70:28: warning: potentially expensive pointer subtraction
mm/memory_hotplug.c:1530:13: warning: potentially expensive pointer subtraction
mm/memory_hotplug.c:734:13: warning: potentially expensive pointer subtraction
mm/memory_hotplug.c:831:41: warning: potentially expensive pointer subtraction
mm/page_owner.c:607:38: warning: potentially expensive pointer subtraction
mm/vmstat.c:1334:38: warning: potentially expensive pointer subtraction
net/core/net-sysfs.c:1040:19: warning: potentially expensive pointer subtraction
net/ipv4/ipmr.c:3026:32: warning: potentially expensive pointer subtraction
net/ipv6/ip6mr.c:458:32: warning: potentially expensive pointer subtraction
net/mac80211/tx.c:1307:41: warning: potentially expensive pointer subtraction
net/mac80211/tx.c:1351:44: warning: potentially expensive pointer subtraction
net/rds/ib_recv.c:345:49: warning: potentially expensive pointer subtraction
net/rds/ib_recv.c:861:38: warning: potentially expensive pointer subtraction
net/sched/cls_tcindex.c:622:43: warning: potentially expensive pointer subtraction
net/sched/sch_cbs.c:302:35: warning: potentially expensive pointer subtraction
net/sched/sch_hhf.c:367:23: warning: potentially expensive pointer subtraction
net/sched/sch_hhf.c:434:38: warning: potentially expensive pointer subtraction
net/sunrpc/svc_xprt.c:1377:43: warning: potentially expensive pointer subtraction
sound/pci/hda/hda_generic.c:301:20: warning: potentially expensive pointer subtraction

IOW it's not terribly noisy. Might be an interesting idea to teach sparse to
print the type in question... Aha - with

--- a/evaluate.c
+++ b/evaluate.c
@@ -848,7 +848,8 @@ static struct symbol *evaluate_ptr_sub(struct expression *expr)

if (value & (value-1)) {
if (Wptr_subtraction_blows)
- warning(expr->pos, "potentially expensive pointer subtraction");
+ warning(expr->pos, "[%s] potentially expensive pointer subtraction",
+ show_typename(lbase));
}

sub->op = '-';

we get things like
drivers/gpu/drm/i915/i915_gem_execbuffer.c:435:17: warning: [struct drm_i915_gem_exec_object2] potentially expensive pointer subtraction

OK, the top sources of that warning are:
91 struct cpufreq_frequency_table
36 struct Indirect (actually, that conflates ext2/ext4/minix/sysv)
21 struct ips_scb
18 struct runlist_element
13 struct zone
13 struct vring
11 struct usbhsh_device
10 struct xpc_partition
9 struct skge_element
9 struct lock_class
9 struct hstate
7 struct nvme_rdma_queue
7 struct iso_context
6 struct i915_power_well
6 struct hpet_dev
6 struct ext4_extent
6 struct esas2r_target
5 struct iio_chan_spec
5 struct hwspinlock
4 struct myri10ge_slice_state
4 struct ext4_extent_idx

everything beyond that is 3 instances or less...

2018-01-19 22:54:23

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Fri, Jan 19, 2018 at 2:12 PM, Al Viro <[email protected]> wrote:
> On Fri, Jan 19, 2018 at 10:42:18AM -0800, Linus Torvalds wrote:
>>
>> We *should* be careful about it. I guess sparse could be made to warn,
>> but I'm afraid that we have so many of these things that a warning
>> isn't reasonable.
>
> You mean like -Wptr-subtraction-blows?

Heh. Apparently I already did that trivial warning back in 2005. I'd
forgotten about it.

> FWIW, allmodconfig on amd64 with C=2 CF=-Wptr-subtraction-blows is not too large
>
> IOW it's not terribly noisy. Might be an interesting idea to teach sparse to
> print the type in question... Aha - with
>
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -848,7 +848,8 @@ static struct symbol *evaluate_ptr_sub(struct expression *expr)
>
> if (value & (value-1)) {
> if (Wptr_subtraction_blows)
> - warning(expr->pos, "potentially expensive pointer subtraction");
> + warning(expr->pos, "[%s] potentially expensive pointer subtraction",
> + show_typename(lbase));
> }
>
> sub->op = '-';
>
> we get things like
> drivers/gpu/drm/i915/i915_gem_execbuffer.c:435:17: warning: [struct drm_i915_gem_exec_object2] potentially expensive pointer subtraction

It would probably be good to add the size too, just to explain why
it's potentially expensive.

That said, apparently we do have hundreds of them, with just
cpufreq_frequency_table having a ton. Maybe some are hidden in macros
and removing one removes a lot.

The real problem is that sometimes the subtraction is simply the right
thing to do, and there's no sane way to say "yeah, this is one of
those cases you shouldn't warn about".

Linus

2018-01-20 02:07:06

by Al Viro

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Fri, Jan 19, 2018 at 02:53:25PM -0800, Linus Torvalds wrote:

> It would probably be good to add the size too, just to explain why
> it's potentially expensive.
>
> That said, apparently we do have hundreds of them, with just
> cpufreq_frequency_table having a ton. Maybe some are hidden in macros
> and removing one removes a lot.

cpufreq_table_find_index_...(), mostly.

> The real problem is that sometimes the subtraction is simply the right
> thing to do, and there's no sane way to say "yeah, this is one of
> those cases you shouldn't warn about".

FWIW, the sizes of the most common ones are
91 sizeof struct cpufreq_frequency_table = 12
Almost all of those come from
cpufreq_for_each_valid_entry(pos, table)
if (....)
return pos - table;
and I wonder if we would be better off with something like
#define cpufreq_for_each_valid_entry(pos, table, idx) \
for (pos = table, idx = 0; pos->frequency != CPUFREQ_TABLE_END; pos++, idx++) \
if (pos->frequency == CPUFREQ_ENTRY_INVALID) \
continue; \
else
so that those loops would become
cpufreq_for_each_valid_entry(pos, table, idx)
if (....)
return idx;
36 sizeof struct Indirect = 24
21 sizeof struct ips_scb = 216
18 sizeof struct runlist_element = 24
13 sizeof struct zone = 1728
Some are from
#define zone_idx(zone) ((zone) - (zone)->zone_pgdat->node_zones)
but there's
static inline int zone_id(const struct zone *zone)
{
struct pglist_data *pgdat = zone->zone_pgdat;

return zone - pgdat->node_zones;
}
and a couple of places where we have
for (zone = node_zones; zone - node_zones < MAX_NR_ZONES; ++zone) {
Those bloody well ought to be
for (zone = node_zones, end = node_zones + MAX_NR_ZONES; zone < end; zone++) {
13 sizeof struct vring = 40
11 sizeof struct usbhsh_device = 24
10 sizeof struct xpc_partition = 888
9 sizeof struct skge_element = 40
9 sizeof struct lock_class = 400
9 sizeof struct hstate = 29872
That little horror comes from get_hstate_idx() and hstate_index(). Code generated
for division is
movabsq $-5542915600080909725, %rax
sarq $4, %rdi
imulq %rdi, %rax
7 sizeof struct nvme_rdma_queue = 312
7 sizeof struct iso_context = 208
6 sizeof struct i915_power_well = 48
6 sizeof struct hpet_dev = 168
6 sizeof struct ext4_extent = 12
6 sizeof struct esas2r_target = 120
5 sizeof struct iio_chan_spec = 152
5 sizeof struct hwspinlock = 96
4 sizeof struct myri10ge_slice_state = 704
4 sizeof struct ext4_extent_idx = 12
Another interesting-looking one is struct vhost_net_virtqueue (18080 bytes)

Note that those sizes are rather sensitive to lockdep, spinlock debugging, etc.

2018-01-20 05:27:51

by Al Viro

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Sat, Jan 20, 2018 at 02:02:37AM +0000, Al Viro wrote:

> Note that those sizes are rather sensitive to lockdep, spinlock debugging, etc.

That they certainly are: on one of the testing .config I'm using it gave this:
1104 sizeof struct page = 56
81 sizeof struct cpufreq_frequency_table = 12
32 sizeof struct Indirect = 24
7 sizeof struct zone = 1400
7 sizeof struct hstate = 152
6 sizeof struct lock_class = 336
6 sizeof struct hpet_dev = 152
6 sizeof struct ext4_extent = 12
4 sizeof struct ext4_extent_idx = 12
3 sizeof struct mbox_chan = 456
2 sizeof struct strip_zone = 24
2 sizeof struct kobj_attribute = 48
2 sizeof struct kernel_param = 40
2 sizeof struct exception_table_entry = 12
1 sizeof struct vif_device = 104
1 sizeof struct unixware_slice = 12
1 sizeof struct svc_pool = 152
1 sizeof struct srcu_node = 152
1 sizeof struct r5worker_group = 56
1 sizeof struct pebs_record_core = 144
1 sizeof struct netdev_queue = 384
1 sizeof struct mirror = 40
1 sizeof struct mif_device = 56
1 sizeof struct e1000_tx_ring = 48
1 sizeof struct dx_frame = 24
1 sizeof struct bts_record = 24
1 sizeof struct ata_device = 2560
1 sizeof struct acpi_processor_cx = 52

2018-01-20 09:40:15

by Luc Van Oostenryck

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On Sat, Jan 20, 2018 at 05:24:32AM +0000, Al Viro wrote:
> On Sat, Jan 20, 2018 at 02:02:37AM +0000, Al Viro wrote:
>
> > Note that those sizes are rather sensitive to lockdep, spinlock debugging, etc.
>
> That they certainly are: on one of the testing .config I'm using it gave this:
> 1104 sizeof struct page = 56

Yes, I get this already with defconfig.
It's a problem with sparse which ignore the alignment attribute
(in fact all 'trailing' attributes in type declarations).

I'm looking to fix it.

-- Luc Van Oostenryck

2018-01-22 13:26:44

by Rasmus Villemoes

[permalink] [raw]

Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.

On 2018-01-19 19:42, Linus Torvalds wrote:
>
> I actually asked (long long ago) for an optinal compiler warning for
> "pointer subtraction with non-power-of-2 sizes". Not because of it
> being undefined, but simply because it's expensive. The
> divide->multiply thing doesn't always work,

Huh? If (compile-time constant, positive) d=m*2^k with m odd, and x is
known to be a multiple of d, x/d can always be computed as (x>>k)*m' ==
(x*m')>>k, with m' being the mod 2^N multiplicative inverse of m, right?
This works whether x has signed or unsigned type and whether it indeed
happens to be negative, AFAICT.

Sure, the multiplication by m' may not exactly be cheap, but one only
needs the low N bits of the result.

Rasmus

2018-01-22 20:00:23