Linux 2.6.21-rc4

I pushed out the -git trees yesterday, but then got distracted, so the patches and tar-balls and the announcement got delayed until this morning. Oops. I'm a scatter-brain.

Anyway, the good news about -rc4 is that there's just lots of random fixes. I'm hoping that we've seriously cut down on the regression list, and I'd ask everybody who is on Adrian's list to please re-verify their regression, and in case it's one of the "patches available" ones but I haven't merged (maybe because it hasn't been sent to me!), make sure I do.

Shortlog appended. Nothing really stands out.

Linus
---
Adrian Bunk (2):
[DLM] fs/dlm/user.c should #include "user.h"
asus-laptop: make code static

Adrian Hunter (2):
[MTD] Correct partition failed erase address [MTD] [OneNAND] add Nokia Copyright and a credit

Aji Srinivas (1):
[BRIDGE]: adding new device to bridge should enable if up

Akiyama, Nobuyuki (1):
[IA64] add missing syscall trace clear

Al Viro (31):
fix deadlock in audit_log_task_context()
sanitize security_getprocattr() API
ibmtr probe is __devinit, not __init
const file_operations fallout
appldata build fix
(uml) sparse flags for userland glue are missing $(CF)
zatm __init abuse
stacktrace doesn't work on uml
fix ipath_dma_free_coherent() prototype
m32r dma-mapping.h should simply include generic/dma-mapping-broken.h include of asm/pgtable.h in nfsfh is bogus
BLK_DEV_IDE_CELLEB dependency fix
sparc: have dma-mapping.h include generic/dma-mapping-broken in non-PCI case rtc-cmos needs RTC_ALWAYS_BCD known
misc NULL noise
fastcall still doesn't make sense in paravirt
dmfe trivial endianness annotations
constant should be long
pasemi trivial iomem annotations
sparc: nr_free_pages() is unsigned long
trivial ATA iomem annotations
cciss endian annotations
qeth gfp_t annotations
C99 initializers, proper use of const in drivers/ps3
cifs endianness annotations
hid-core endianness annotations
ANSIfy powerpc floppy.h
atl1 trivial endianness misannotations
kill bogus casts in amd64 uaccess.h
paride endianness annotations
m68k dma-mapping: gfp_t annotations

Alan Cox (4):
[MTD] [MAPS] dilnetpc: Fix printk warning
libata-acpi: Try and stop all the non PCI devices crashing
ata_piix: Remove ugly layering violation
z85230: Fix FIFO handling

Alan Stern (4):
USB: set the correct interval for interrupt URBs
UHCI: fix port resume problem
sysfs and driver core: add callback helper, used by SCSI and S390
sysfs: reinstate exclusion between method calls and attribute unregistration

Alexey Starikovskiy (2):
ACPICA: Fix ACPI Global Lock re-entrancy
ACPI: ec: fix race in status register access

Andre Spahlinger (1):
USB: ipaq.c: Additional devices

Andrew Morton (1):
sony-laptop: fix uninitialised variable

Andrew Nayenko (1):
USB storage: Nokia 6288 unusual_devs entry

Anthony Godshall, Ampro Computers, Inc (1):
ACPI: make blacklist more verbose

Arnd Bergmann (2):
[POWERPC] spufs: fix possible memory corruption is spufs_mem_write [POWERPC] update cell_defconfig

Artem Bityutskiy (1):
[JFFS2] print a message when marking bad block

Benjamin Herrenschmidt (4):
[POWERPC] Fix warning in prom_parse.c of_irq_map_oldworld() [POWERPC] Fix warning in powermac feature.c [POWERPC] Fix warning in powermac pci.c
[POWERPC] Fix spu SLB invalidations

Bernhard Walle (1):
ACPI: Add kernel-parameters hint that acpi=off doesn't work on IA64.

Brice Goglin (4):
myri10ge: fix error checking and return value in myri10ge_allocate_rings
myri10ge: use pci_map_page to prepare the dmatest buffer
myri10ge: prevent 4k rdma on SGI TIOCE chipset
myri10ge: add a wc_enabled flag to myri10ge_priv

Chris Dearman (1):
[MIPS] Oprofile: Reset all performance registers for MIPS_MT_SMP configs

Chris Wright (1):
[IPV6] fix ipv6_getsockopt_sticky copy_to_user leak

Christoph Hellwig (1):
[POWERPC] avoid SPU_ACTIVATE_NOWAKE optimization

Cyrill Gorcunov (2):
[MTD] ESB2 check for closed ROM window
USB Elan FTDI: check for workqueue creation

Dale Farnsworth (1):
mv643xx: Clear pending interrupts before calling request_irq

Dan Williams (1):

      [ARM] 4248/1: lh7a40x: fix missing definitions for get_irqnr_preamble

Dave Jones (2):
update 'getting sparse' info.
build fix for i386 earlyquirk.c

David Brownell (1):
USB: at91_udc, fix more modpost bogosity (rename driver struct)

David Miller (1):
RDMA/cxgb3: Fix build on sparc64

David S. Miller (5):
[IPV6]: Handle np->opt being NULL in ipv6_getsockopt_sticky(). [SPARC64]: Fix PARPORT build (again).
[SPARC]: We do not need OLD_GETRLIMIT.
[SPARC]: Hook up missing syscalls.
[SPARC64]: Add missing HPAGE_MASK masks on address parameters.

David Woodhouse (2):
[JFFS2] Use yield() between GC passes in background thread. [JFFS2] Check for all-zero node headers

Davide Libenzi (1):
Add epoll compat_ code to fs/compat.c

Dirk Behme (1):
ARM: OMAP: Fix missing workqueue include in board-h2.c

Dmitriy Monakhov (1):

      kobject: new_device->kref wasn't putted after error in kobject_move()

Dmitry Torokhov (1):
Input: i8042 - another attempt to fix AUX delivery checks

Eric Paris (3):
[IPSEC]: xfrm_policy delete security check misplaced [IPSEC]: Add xfrm policy change auditing to pfkey_spdget [IPSEC]: xfrm audit hook misplaced in pfkey_delete and xfrm_del_sa

Eric W. Biederman (2):
msi: Safer state caching.
pci: Repair pci_save/restore_state so we can restore one save many times.

Evgeniy Polyakov (1):
[IPV4]: Fix rtm_to_ifaddr() error handling.

Fenghua Yu (1):

      [IA64] fsys_getcpu for IA64

Francois Romieu (2):
r8169: revert bogus BMCR reset
r8169: fix a race between PCI probe and dev_open

Gard Spreemann (1):
USB: Product ID for FT232RL in ftdi_sio

Gary Zambrano (1):
avr32: dma-mapping.h

Geert Uytterhoeven (2):
[POWERPC] ps3: always make sure were running on a PS3 [IPV4]: Fix warning in ip_mc_rejoin_group.

Gerrit Renker (2):
[DCCP]: Revert patch which disables bidirectional mode [DCCP]: Initialise write_xmit_timer also on passive sockets

Greg Kroah-Hartman (2):
Revert "driver core: refcounting fix" Driver core: add device symlink back to sysfs

Haavard Skinnemoen (5):
[AVR32] at32_spi_setup_slaves should be __init [AVR32] show_trace: Only walk valid stack addresses [AVR32] Fix typo in include/asm-avr32/Kbuild [AVR32] Fix bogus ti->flags manipulation in debug handler [AVR32] Don't use kmap() in flush_icache_page()

Henrique de Moraes Holschuh (3):
ACPI: ibm-acpi: fix initial status of backlight device
ACPI: ibm-acpi: make ibm-acpi bay support optional
ACPI: ibm-acpi: improve backlight power handling

Herbert Xu (2):
[UDP]: Reread uh pointer after pskb_trim [IPV6]: Do not set IF_READY if device is down

Hideo Saito (1):
sh: Fix kernel thread stack corruption with preempt.

Hoang-Nam Nguyen (1):
IB/ehca: Fix sync between completion handler and destroy cq

Horms (3):
[IA64] remove duplicate declaration of efi_initialize_iomem_resources [IA64] whitespace fixes for include/asm-ia64/sal.h
[IA64] put kdump_find_rsvd_region in __init

Ingo Molnar (1):
CPU hotplug: call check_tsc_sync_source() with irqs off

Ishizaki Kou (1):
[POWERPC] Celleb: bug fix caused by not casting pointer types

Jarek Poplawski (1):
[SCTP] ipv6: inconsistent lock state ipv6_add_addr/sctp_v6_copy_addrlist

Jeff Dike (1):
uml: arch_prctl should set thread fs

Jim Radford (2):
usb-serial: fix shutdown / device_unregister order
USB: ftdi_sio: use port_probe / port_remove thereby fixing access to the latency_timer

Jiri Kosina (3):
bluetooth: fix socket locking in hci_sock_dev_event()
HID: allocate hid_parser in a proper way
HID: zeroing of bytes in output fields is bogus

Joel Becker (4):
ocfs2: Proper cleanup in case of error in ocfs2_register_hb_callbacks()
ocfs2: Concurrent access of o2hb_region->hr_task was not locked
ocfs2: add some missing address space callbacks
configfs: add missing mutex_unlock()

Joerg Sommer (1):
bcm43xx: Fix bug in frequency to channel conversion

Johannes Berg (1):
driver core: export device_rename

John Keller (2):
ACPI: Altix: cannot register acpi bus driver before bus scan
ACPI: Altix: reinitialize acpi tables

Jon K Hellan (1):
USB: New device IDs for cp2101 driver

Josef Whiter (2):
[GFS2] fix locking mistake
[GFS2] fix hangup when multiple processes are trying to write to the same file

Joy Latten (1):
[XFRM]: Fix missing protocol comparison of larval SAs.

Julius Volz (1):
ACPI: video: Fix spelling and grammar mistakes

Jörn Engel (1):
Remove devfs from MAINTAINERS

KAMEZAWA Hiroyuki (1):
[IA64] fix NULL pointer in ia64/irq_chip-mask/unmask function

Keith Owens (1):
[IA64] Remove sparse warning from unwind code

Konstantin Karasyov (2):
ACPI: fix S3 fan resume issue
ACPI: ThinkPad Z60m: usb mouse stops working after suspend to RAM

Kristen Accardi (1):
libata-acpi: allow _GTF on SATA, but disable on PATA for now

Kumar Gala (1):

      [POWERPC] 85xx: Enable CONFIG_SERIAL_8250_SHARE_IRQ

Kyungmin Park (4):
[MTD] [OneNAND] Use oob buffer instead of main one in oob functions [MTD] [OneNAND] Fix typo & wrong comments
[MTD] [OneNAND] Exit the loop when transferring/filling of the oob is finished [MTD] [OneNAND] Classify the page data and oob buffer

Larry Finger (1):
bcm43xx: Fix errors in specs to code translation in B6PHY init

Len Brown (3):
ACPI: fix Thinkpad 600/600E/600X interrupts
ACPI: repair nvidia early quirk breakage on x86_64
ACPI: repair nvidia early quirk breakage on x86_64

Li Yang (1):
[POWERPC] 83xx: Minor fixes for 834x_mds USB setup code

Linsys Contractor Mithlesh Thukral (2):
NetXen: Bug fix for Jumbo frames on XG card
NetXen: Fix softlockup seen during hardware access

Linus Torvalds (3):
Revert "USB: pxa2xx_udc: fix hardcoded irq number" Disable NMI watchdog by default properly Linux 2.6.21-rc4

Martin Schiller (1):
USB: RTS/DTR signal patch for airprime driver

Mathieu Desnoyers (1):
[SPARC64]: Fix atomicity of TIF update in flush_thread()

Max Dmitrichenko (1):
USB: fix Unaligned access in EHCI driver

Michael Ellerman (1):
[POWERPC] Add missing newline in xmon help output

Michael Karcher (1):
ACPI: fix parallel port IRQ after resume from S3

Michael Olberg (1):
USB: add QL355P power supply ids to fdti_sio

Milan Svoboda (2):
USB: pxa2xx_udc: fix hardcoded irq number [ARM] 4263/1: fix IXP4XX_NPE[ABC]_BASE_VIRT address

Olaf Kirch (1):
[IPV6]: Fix for ipv6_setsockopt NULL dereference

Oliver Neukum (5):
USB: ratelimit debounce error messages
USB: kill dead code from hub.c
USB: fix usb-serial device naming bug
USB: further fix for usb-serial
USB: fix spinlock recursion in cdc-acm.c

Olof Johansson (1):
[POWERPC] No DEEPNAP on 970MP 1.0

Paolo 'Blaisorblade' Giarrusso (9):
uml: hostfs: fix double free
uml: hostfs: make hostfs= option work as a jail, as intended.
um: fix a memory leak in the multicast driver
um: remove dead code about os_usr1_signal() and os_usr1_process()
um: mark both consoles as CON_ANYTIME
um: fix confusion irq early reenabling
uml: activate_fd: return ENOMEM only when appropriate
um: fix errno usage
x86_64: fix 2.6.18 regression - PTRACE_OLDSETOPTIONS should be accepted

Pat Gefre (1):
2.6 Altix: console fix for CONFIG_DEBUG_SHIRQ usage

Patrick McHardy (3):
[NETFILTER]: nf_conntrack_ipv6: fix incorrect classification of IPv6 fragments as ESTABLISHED [NETFILTER]: nfnetlink_log: zero-terminate prefix
[NETFILTER]: nfnetlink_log: fix crash on bridged packet

Paul Moore (1):
[NetLabel]: parse the CIPSO ranged tag on incoming packets

Paul Mundt (6):
doc: Add SH to vdso and earlyprintk in kernel-parameters.txt
sysctl: Support vdso_enabled sysctl on SH.
sh: Use L1_CACHE_BYTES for .data.cacheline_aligned.
sh: Enable SM501 support for RTS7751R2D.
sh: Revert lazy dcache writeback changes.
sh: Kill off I/O cruft for R7780RP.

Pavel Pisa (3):
[ARM] 4254/1: i.MX/MX1 CPU Frequency scaling honor boot loader set BCLK_DIV. [ARM] 4255/1: i.MX/MX1 Correct MPU PLL reference clock value. [ARM] 4256/1: i.MX/MX1 SDHC fix/workaround of SD card recognition problems

Pavel Roskin (1):
bcm43xx: Fix assertion failures in interrupt handler

Peter Zijlstra (1):
ecryptfs: nested locking annotation

Petr Vandrovec (1):
Fix simplex adapters with libata

Phil Dibowitz (1):
USB storage: Removed duplicate supertop unusual_dev entry

Philipp Reisner (1):
[CONNECTOR]: Bugfix for cn_call_callback()

Ralf Baechle (14):
[CHAR] ds1286: Fix handling of seconds in RTC_ALM_SET ioctl. [MIPS] ISA: Fix typo
[MIPS] ARC: Fix several compiler warnings.
[MIPS] DEC: Remove call to register_prom_console.
netxen: fix warnings
3c59x: Fix several modpost warnings
ibmtr: Drain rich supply of modpost warnings.
[MIPS] Sibyte: Do not allow enabling LDT support if PCI is disabled. [MIPS] Sibyte: Fix ZBbus profiler
USB: goku_udc: Remove crude cache coherency code
[ROSE]: Remove ourselves from waitqueue when receiving a signal [ROSE]: Socket locking is a great invention.
[MIPS] Viper2: Remove defective support.
[MIPS] kspd: ioctl needs a translation entry.

Richard Fearn (1):
[GFS2] add newline to printk message

Richard Purdie (3):
[ARM] 4249/1: Fix tosa compile failure
[ARM] 4250/1: Fix locomo backlight conversion error/compile failure [ARM] 4251/1: Fix sharpsl_pm dependency

Richard Woodruff (1):
ARM: OMAP: Fix OMAP2 dss2 so clk_set_parent works

Robert Hancock (1):
sata_nv: revert use of notifiers for now

Robert P. J. Day (3):
[MTD] [NAND] Correct misspelled preprocessor variable.
ACPI: Kconfig: hide ACPI menu when CONFIG_PM=n
[WANROUTER]: Delete superfluous source file "net/wanrouter/af_wanpipe.c".

Robert Reif (2):
[CG14]: Fix section mismatch warnings.
[BW2]: Fix section mismatch warnings.

Roger Luethi (1):
via-rhine: set avoid_D3 for broken BIOSes

Roland Dreier (2):
IPoIB: Only handle async events for one port IB/mthca: Fix error path in mthca_alloc_memfree()

Russ Anderson (2):
[IA64] Proper handling of TLB errors from duplicate itr.d dropins [IA64] Cache error recovery

Ryusuke Sakato (1):
sh: Fix sigmask trampling in signal delivery.

Sam Ravnborg (3):
pcie: fix section mismatch warning
PCI: aer: fix section mismatch warning
pci: fix section mismatch warning

Sean Hefty (2):
RDMA/cma: Initialize rdma_bind_list in cma_alloc_any_port() RDMA/ucma: Avoid sending reject if backlog is full

Segher Boessenkool (1):
[POWERPC] Select u-image as default image for Linkstation

Shaohua Li (1):
ACPI: fix boot hang w/o "noapic" on MSI MS-6390-L

Shirley Ma (1):
IPoIB: Turn on interface's carrier after broadcast group is joined

Simon Horman (2):
[IA64] kexec: declare ia64_mca_pal_base in mca.h rather than kexec.h [IA64] Cleanup in crash.c

Stephen Hemminger (2):
sky2: turn off Rx checksum on bad hardware
skge: set mac address bonding fix

Stephen Rothwell (3):
[POWERPC] Allocate syscall number for sys_getcpu [POWERPC] Wire up sys_epoll_pwait
[POWERPC] sys_move_pages should be callable from an SPU

Steve Wise (8):
RDMA/cxgb3: Start ep timer on a MPA reject RDMA/cxgb3: Don't use mm after it's freed in iwch_mmap() RDMA/cxgb3: Fixes for "normal close" failures RDMA/cxgb3: Move QP to error on destroy if the state is IDLE RDMA/cxgb3: Stop EP timer when MPA exchange is aborted by peer RDMA/cxgb3: Squelch logging AE errors
RDMA/cxgb3: Don't reuse skbs that are non-linear or cloned RDMA/cxgb3: Fix MR permission problems

Steven Whitehouse (5):
[GFS2] Fix bz 230143, incorrect flushing of rgrps
[GFS2] Fix bz 229831, lookup returns wrong inode
[GFS2] Remove unused variable
[GFS2] go_drop_bh is never used, so remove it
[GFS2] Fix bz 229873, alternate test: assertion "!ip->i_inode.i_mapping->nrpages" failed

Stuart Menefy (1):
sh: Clear UBC when not in use.

Sunil Mushran (2):
ocfs2_dlm: Missing get/put lockres in dlm_run_purge_lockres
ocfs2_dlm: Add missing locks in dlm_empty_lockres

Tejun Heo (3):
libata: fix ata_host_release() free order
devres: release resources on device_del()
PCI: allow multiple calls to pcim_pin_device()

Thomas Schleusener (1):
USB: add Additional PIDs in ftdi_sio

Tony Lindgren (1):
ARM: OMAP: Include missing header

Tony Luck (2):
[IA64] Pick highest possible saved_max_pfn for crash_dump [IA64] refresh config files

Uwe Kleine-König (1):

      [ARM] 4247/1: Fix long name for cc9p9360dev

Vijay Sampath (1):
[MTD] [NOR] Fix oops in cfi_amdstd_sync

Vitaly Wool (2):
[MTD] [NAND] make oobavail public
[JFFS2] Fix writebuffer recovery in the first page of a block

Wendy Cheng (2):
[GFS2] NFS filehandle check
[GFS2] pass formal ino in do_filldir_main

William Lee Irwin III (1):
[SPARC]: Fix TIF_USEDFPU flag atomicity

Wim Van Sebroeck (1):
[WATCHDOG] i8xx TCO driver - mark for removal

YOSHIFUJI Hideaki (1):
USBNET: DM9501: Add Corega FEther USB-TXC support.

Zachary Amsden (2):
Fix VMI and COMPAT_VDSO for 2.6.21
Fix vmi time header bug

Zhang, Yanmin (1):
[IA64] pci_get_legacy_ide_irq should return irq (not GSI)

[email protected] (1):
[GFS2] build fix

[email protected] (3):
natsemi: Consistently use interrupt enable/disable functions
natsemi: Fix NAPI for interrupt sharing
natsemi: Avoid IntrStatus lossage if RX state machine resets.

suzuki (1):
check_partition(): fix error check

At Fri, 16 Mar 2007 09:33:54 -0700 (PDT),

alsa-git merge seems missing in rc4:

	http://lkml.org/lkml/2007/3/14/43

Linus, could you pull from below (linus brunch)?

master.kernel.org:/pub/scm/linux/kernel/git/perex/alsa.git linus

It includes regression fixes.

Thanks,

Takashi

Hi,

I've got *bad* news. Bug described here
http://www.ussg.iu.edu/hypermail/linux/kernel/0703.0/index.html#0889
http://www.ussg.iu.edu/hypermail/linux/kernel/0703.0/index.html#1165
probably leaked into mainline.

Fsck!

http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.21-rc4/git-config
http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.21-rc4/git-console.log

Takashi, unfortunately this bug isn't fixed
http://www.ussg.iu.edu/hypermail/linux/kernel/0703.0/index.html#1684

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

fsck indeed. I don't even understand what's happening with that one - it seems like the kernel schedules a user process, but never deschedules it again.

Tomorrow, I'll try to find out how to reproduce this bug.

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

"I have noticed some strange system behavior. When i try to build a kernel (medium load) - X, keyboard, mouse and sound hangs."

Note that ping is handled in interrupt or softirq context. So something has locked up. Try without X? Or perhaps attack a serial console/netconsole, and when it hangs, use Sysrq to dump the process' states.

Jan

I already did this
http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.21-rc4/git-console.log

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

Hello,

Me too.

http://www.ussg.iu.edu/hypermail/linux/kernel/0703.0/1243.html

Although I can't reproduce it with 2.6.21-rc3-mm1.

Regards,

Mariusz Kozlowski

At Fri, 16 Mar 2007 18:44:26 +0100,

The patch wasn't merged to rc4.
Or, do you mean the bug is still present even with the patch?

Takashi

I know. I use patched 2.6.21-rc3-git4 for crashdump.

This patch doesn't fix the problem.
http://www.ussg.iu.edu/hypermail/linux/kernel/0703.0/3080.html

Tomorrow, I'll try to provide more info about this problem.

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

You said at [1] that the patch from [2] (not yet merged into Linus' tree) fixed this problem for you.

Or do I confuse anything?

cu
Adrian

[1] http://lkml.org/lkml/2007/3/13/276
[2] http://lkml.org/lkml/2007/3/7/540

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Adrian Bunk napisał(a):

Sorry, it's my fault.

This patch doesn't fix the problem.

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

I'm afraid that if CONFIG_TICK_ONESHOT or CONFIG_NO_HZ is set, we still have a problem with RCU synchronization while nonboot CPUs are being enabled during a resume (http://lkml.org/lkml/2007/3/11/144, http://lkml.org/lkml/2007/3/4/88).

Can someone who had this problem with -rc3 check if it's present in -rc4?

Thanks,
Rafael

I finally found a box today, which shows this problem. I'm working on a fix.

tglx

I finally found a dual core box, which survives suspend/resume without crashing in the middle of nowhere. Sigh, I never figured out from the code and the bug reports what's going on.

The observed hangs are caused by a stale state transition of the clock event devices, which keeps the RCU synchronization away from completion, when the non boot CPU is brought back up.

The suspend/resume in oneshot mode needs the similar care as the periodic mode during suspend to RAM. My assumption that the state transitions during the different shutdown/bringups of s2disk would go through the periodic boot phase and then switch over to highres resp. nohz mode were simply wrong.

Add the appropriate suspend / resume handling for the non periodic modes.

index 5567745..eadfce2 100644
@@ -307,12 +307,19 @@ int tick_resume_broadcast(void)
 	spin_lock_irqsave(&tick_broadcast_lock, flags);
 
 	bc = tick_broadcast_device.evtdev;
-	if (bc) {
-		if (tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC &&
-		    !cpus_empty(tick_broadcast_mask))
-			tick_broadcast_start_periodic(bc);
 
-		broadcast = cpu_isset(smp_processor_id(), tick_broadcast_mask);
+	if (bc) {
+		switch (tick_broadcast_device.mode) {
+		case TICKDEV_MODE_PERIODIC:
+			if(!cpus_empty(tick_broadcast_mask))
+				tick_broadcast_start_periodic(bc);
+			broadcast = cpu_isset(smp_processor_id(),
+					      tick_broadcast_mask);
+			break;
+		case TICKDEV_MODE_ONESHOT:
+			broadcast = tick_resume_broadcast_oneshot(bc);
+			break;
+		}
 	}
 	spin_unlock_irqrestore(&tick_broadcast_lock, flags);
 

@@ -347,6 +354,16 @@ static int tick_broadcast_set_event(ktime_t expires, int force)
 	}
 }
 
+int tick_resume_broadcast_oneshot(struct clock_event_device *bc)
+{
+	clockevents_set_mode(bc, CLOCK_EVT_MODE_ONESHOT);
+
+	if(!cpus_empty(tick_broadcast_oneshot_mask))
+		tick_broadcast_set_event(ktime_get(), 1);
+
+	return cpu_isset(smp_processor_id(), tick_broadcast_oneshot_mask);
+}
+
 /*
  * Reprogram the broadcast device:
  *

index 43ba1bd..bfda3f7 100644
@@ -298,18 +298,17 @@ static void tick_shutdown(unsigned int *cpup)
 	spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
-static void tick_suspend_periodic(void)
+static void tick_suspend(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
 	unsigned long flags;
 
 	spin_lock_irqsave(&tick_device_lock, flags);
-	if (td->mode == TICKDEV_MODE_PERIODIC)
-		clockevents_set_mode(td->evtdev, CLOCK_EVT_MODE_SHUTDOWN);
+	clockevents_set_mode(td->evtdev, CLOCK_EVT_MODE_SHUTDOWN);
 	spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
-static void tick_resume_periodic(void)
+static void tick_resume(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
 	unsigned long flags;

@@ -317,6 +316,8 @@ static void tick_resume_periodic(void)
 	spin_lock_irqsave(&tick_device_lock, flags);
 	if (td->mode == TICKDEV_MODE_PERIODIC)
 		tick_setup_periodic(td->evtdev, 0);
+	else
+		tick_resume_oneshot();
 	spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 

@@ -348,13 +349,13 @@ static int tick_notify(struct notifier_block *nb, unsigned long reason,
 		break;
 
 	case CLOCK_EVT_NOTIFY_SUSPEND:
-		tick_suspend_periodic();
+		tick_suspend();
 		tick_suspend_broadcast();
 		break;
 
 	case CLOCK_EVT_NOTIFY_RESUME:
 		if (!tick_resume_broadcast())
-			tick_resume_periodic();
+			tick_resume();
 		break;
 
 	default:

index 75890ef..c9d203b 100644
@@ -19,12 +19,13 @@ extern void tick_setup_oneshot(struct clock_event_device *newdev,
 extern int tick_program_event(ktime_t expires, int force);
 extern void tick_oneshot_notify(void);
 extern int tick_switch_to_oneshot(void (*handler)(struct clock_event_device *));
-
+extern void tick_resume_oneshot(void);
 # ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 extern void tick_broadcast_setup_oneshot(struct clock_event_device *bc);
 extern void tick_broadcast_oneshot_control(unsigned long reason);
 extern void tick_broadcast_switch_to_oneshot(void);
 extern void tick_shutdown_broadcast_oneshot(unsigned int *cpup);
+extern int tick_resume_broadcast_oneshot(struct clock_event_device *bc);
 # else /* BROADCAST */
 static inline void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
 {

@@ -43,6 +44,10 @@ void tick_setup_oneshot(struct clock_event_device *newdev,
 {
 	BUG();
 }
+static inline void tick_resume_oneshot(void)
+{
+	BUG();
+}
 static inline int tick_program_event(ktime_t expires, int force)
 {
 	return 0;

@@ -54,6 +59,10 @@ static inline void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
 }
 static inline void tick_broadcast_oneshot_control(unsigned long reason) { }
 static inline void tick_shutdown_broadcast_oneshot(unsigned int *cpup) { }
+static inline int tick_resume_broadcast_oneshot(struct clock_event_device *bc)
+{
+	return 0;
+}
 #endif /* !TICK_ONESHOT */
 
 /*

index 2e8b7ff..f6997ab 100644
@@ -41,6 +41,18 @@ int tick_program_event(ktime_t expires, int force)
 }
 
 /**
+ * tick_resume_onshot - resume oneshot mode
+ */
+void tick_resume_oneshot(void)
+{
+	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
+	struct clock_event_device *dev = td->evtdev;
+
+	clockevents_set_mode(dev, CLOCK_EVT_MODE_ONESHOT);
+	tick_program_event(ktime_get(), 1);
+}
+
+/**
  * tick_setup_oneshot - setup the event device for oneshot mode (hres or nohz)
  */
 void tick_setup_oneshot(struct clock_event_device *newdev,

Hi,
I can confirm that this patch fixed the problem on Thinkpad X60s.

Thanks !

Milan
--
[email protected]

Excellent work. Now suspend to disk is working again. But:

1.) The quirk added in commit a417a21e10831bca695b4ba9c74f4ddf5a95ac06 for the appletouch driver doesn't seem to work after resume.

2.) The first suspend to disk works with no problems, but the second
suspend to disk in a row results in an oops:
->resume_device ->
pci_device_resume->ata_host_resume->ahci_pci_device_resume->ata_pci_device_do_resume->pci_restore_state

Can you please see if this problem is already in the Adrian's list of known regressions?

Thanks,
Rafael

AFAIK not, I've given it an own entry.

cu
Adrian

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Not a regression, but still a bug.
Appropriate people added to the Cc.

cu
Adrian

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

(trimmed CC a little bit)

Thomas, could you please provide more information? Did it ever work for you after suspend/resume cycle and it just broke at some point in the past, or you are not sure whether it ever worked?

Also, what exactly does it mean that it doesn't work? Are both the interfaces after resume bound to usbhid driver (you can check this in /sys)? When the quirk works correctly, only the keyboard interface should be bound to usbhid driver. Please check the binding both before and after suspend/resume cycle, and let me know.

Thanks,

Jiri Kosina

I'm not sure whether this ever worked.

Appletouch is bound to the device:

T:  Bus=01 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#=  7 Spd=12  MxCh= 0
D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
P:  Vendor=05ac ProdID=0218 Rev= 0.60
S:  Manufacturer=Apple Computer
S:  Product=Apple Internal Keyboard / Trackpad

C:* #Ifs= 3 Cfg#= 1 Atr=a0 MxPwr= 40mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=01 Prot=01 Driver=usbhid
E: Ad=83(I) Atr=03(Int.) MxPS= 8 Ivl=8ms
I:* If#= 1 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=01 Prot=02 Driver=appletouch
E: Ad=81(I) Atr=03(Int.) MxPS= 64 Ivl=8ms
I:* If#= 2 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=00 Prot=00 Driver=usbhid
E: Ad=84(I) Atr=03(Int.) MxPS= 1 Ivl=8ms

But the X server touchpad driver doesn't work anymore, that means i can't emulte a right click by tapping with 3 fingers on the mouse pad. after restarting x the mouse driver works again. So i think this is maybe a problem in X?

OK, so the quirk actually works fine ...

... but there is something apparently wrong either with the appletouch driver or X. Could you test via evtest whether the events are properly generated by the kernel? If they do, I'd say it is almost certainly X bug.

Thanks,

Jiri Kosina

Yes, it works fine, but...

It seems, that after the resume all usb devices gets removed and plug in again (virtually!). This results in a new input device name:

before suspend and resume:

appletouch Geyser 3 inited.
input: appletouch as /class/input/input2

after resume:
Restarting tasks ... <6>usb 1-2: USB disconnect, address 3
PM: Removing info for No Bus:usbdev1.3_ep83
PM: Removing info for usb:1-2:1.0
PM: Removing info for No Bus:usbdev1.3_ep81
input: appletouch disconnected
[cut]
PM: Adding info for No Bus:usbdev1.4_ep83
PM: Adding info for usb:1-2:1.1
appletouch Geyser 3 inited.
input: appletouch as /class/input/input15

This change confuses the X synaptics driver:

Touchpad no synaptics event device found (checked 11 nodes) Touchpad The /dev/input/event* device nodes seem to be missing (EE) xf86OpenSerial: Cannot open device /dev/input/event2
No such file or directory.
(WW) Touchpad: cannot open input device

And so X falls back to my second pointer device which is a UsbMouse under /dev/input/mice

One could say that the synaptics driver rightly complains about the missing event2 device!
So is this a bug in the X synaptics driver?

Comments are welcome.

Yes, this is what actually happens. JFYI see current thread on lkml which is a bit realted - http://lkml.org/lkml/2007/3/27/149 if interested.

You can of course work this around by adding an udev rule such as

SUBSYSTEM=="input",KERNEL=="event*",SYSFS{name}=="appletouch",SYMLINK+="input/appletouchpad"

and the let Xorg use /dev/input/appletouchpad, which will always be a symlink to the correct device.

Jiri Kosina

I am not sure if this would help... According to the excerpt from X log synaptics driver attempted to scan evdev devices and locate the touchpad. However if this scan happen before udev had a chance to process the event and create new /dev/input/eventX device node it will fail.

I wonder if we should adjust the X driver to spin for a couple of seconds in EventAutoDevProbe if the touchpad was already seen once... Peter?

Dmitry

This was my first idea, too. But then i found this entry in the Changelog of the synaptics driver:

- In the DeviceOn() function, if opening the device node fails, try to auto-detect the correct event device again. This fixes some problems which occur after a suspend/resume cycle or after rmmod/insmod-ing the psmouse kernel driver.

Okay. This strengthens above statement. And udev is too slow to create the devices, while the driver already scanned the directory.

Yes, it looks like this is the root cause.

However I must admit that I don't like this behavior too much. We shouldn't rely on drivers individual userland to wait for a reasonable time before the udev settles down. This is not a nice API to provide. Will try to think of some solution which would have reasonable nastiness/functionality ratio.

Jiri Kosina

The proper fix would be teach X to recognize hotplug events. I've heard they are working on it.

Dmitry

This didn't fix the suspend problems on my Thinkpad R60. (Sorry for nagging - please let me know if I can assist in debugging this...)

Marcus

I did not expect that it fixes your problem. clockevents are only used in arch/i386 right now. You are running a 64 bit kernel, so a change of your problem would have been very surprising.

You said, that the breakage came between 2.6.20 and rc2. Can you bisect it ?

tglx

The XFS workqueue patch [1] fixes my problem [2].

Marcus

[1] http://permalink.gmane.org/gmane.linux.kernel/507616
[2] http://permalink.gmane.org/gmane.linux.kernel/505570

Unfortunately it only fixed suspend to RAM. Suspend to disk still hangs at "snapshotting system". Will try to bisect it...

Marcus

I don't know if this is related, but using 2.6.21-rc4 and suspend2 2.2.9.7, I also get a hang at suspend. I could try if this also happens with 2.6.20 and the same suspend2 version.

Regards,
Tino

Do you use the microcode driver?

Rafael

No.

I'm in the middle of bisecting. Strange enough the symptoms change as I go. Some commits manage to suspend to RAM, but hang at resume.

Marcus

allmodconfig on i386:

WARNING: "default_idle" [arch/i386/kernel/apm.ko] undefined!
WARNING: "machine_real_restart" [arch/i386/kernel/apm.ko] undefined! make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Please ignore.

I think that this was the result of doing 'make allyesconfig && make all' followed by 'make allmodconfig && make all' without doing a 'make clean' between them.

---
~Randy

This would seem to be a bug in the build system then. Or are you supposed to "make clean" after every config change?

Chris

No. When .config is changed, include/linux/config/ is updated, which causes things that depends on it one or the other way to rebuild. At least that is what I observed since ages.

Jan

But then we have a dependency error somewhere we need to track down. I will try to test here.

Sam

So far no luck in reproducing this.
I will await additional reports before looking more into this one.

Sam

Hi Sam,
It's reproducible for me.

(I'm on x86_64:)

make clean
make ARCH=i386 allyesconfig
make ARCH=i386 all
make ARCH=i386 allmodconfig
make ARCH=i386 all

What kind of debug info do you want/need on this?

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

This email lists some known regressions in Linus' tree compared to 2.6.20.

If you find your name in the Cc header, you are either submitter of one of the bugs, maintainer of an affectected subsystem or driver, a patch of you caused a breakage or I'm considering you in any other way possibly involved with one or more of these issues.

Due to the huge amount of recipients, please trim the Cc when answering.

Subject : weird system hangs
References : http://lkml.org/lkml/2007/3/16/288
Submitter : Michal Piotrowski <[email protected]>
Mariusz Kozlowski <[email protected]>
Status : unknown

Subject : crashes in KDE
References : http://bugzilla.kernel.org/show_bug.cgi?id=8157
Submitter : Oliver Pinter <[email protected]>
Status : unknown

Subject : kwin dies silently
References : http://lkml.org/lkml/2007/2/28/112
Submitter : Sid Boyce <[email protected]>
Status : unknown

Since I didn't see any mention of this:

I'm seeing an Oops when removing the ohci1394 module:

[   16.047275] ieee1394: Node removed: ID:BUS[158717321-38:0860]  GUID[c033ced600000000]
[   16.047287] BUG: unable to handle kernel NULL pointer dereference at virtual address 00000094
[   16.047451]  printing eip:
[   16.047524] c02daf3d
[   16.047527] *pde = 00000000
[   16.047603] Oops: 0000 [#1]
[   16.047676] PREEMPT 
[   16.047788] Modules linked in: backlight ohci1394 parport_pc parport
[   16.048069] CPU:    0
[   16.048071] EIP:    0060:[<c02daf3d>]    Not tainted VLI
[   16.048074] EFLAGS: 00010246   (2.6.21-rc4 #35)
[   16.048298] EIP is at class_device_remove_attrs+0xa/0x30
[   16.048377] eax: dfd04338   ebx: 00000000   ecx: df655988   edx: 00000000
[   16.048456] esi: 00000000   edi: dfd04338   ebp: 00000000   esp: df506e38
[   16.048535] ds: 007b   es: 007b   fs: 00d8  gs: 0033  ss: 0068
[   16.048614] Process rmmod (pid: 1455, ti=df506000 task=df6cc0b0 task.ti=df506000)
[   16.048693] Stack: dfd04338 dfd04340 00000000 c02db02f 00000000 dfd04338 dfd041e4 c0331871 
[   16.049159]        00000000 c02db065 dfd041b0 c0331858 c055006d 0975d589 00000026 0000035c 
[   16.049626]        00000000 c033ced6 00000000 df24c000 c0331879 c02d859f df24c0bc df24c0bc 
[   16.050091] Call Trace:
[   16.050233]  [<c02db02f>] class_device_del+0xcc/0xfa
[   16.050352]  [<c0331871>] __nodemgr_remove_host_dev+0x0/0xb
[   16.050475]  [<c02db065>] class_device_unregister+0x8/0x10
[   16.050595]  [<c0331858>] nodemgr_remove_ne+0x61/0x7a
[   16.050714]  [<c033ced6>] ether1394_header_cache+0x0/0x43
[   16.050835]  [<c0331879>] __nodemgr_remove_host_dev+0x8/0xb
[   16.050954]  [<c02d859f>] device_for_each_child+0x1a/0x3c
[   16.051073]  [<c0331b98>] nodemgr_remove_host+0x30/0x90
[   16.051192]  [<c032f12c>] __unregister_host+0x1a/0xad
[   16.051311]  [<c032ee17>] hl_get_hostinfo+0x5b/0x76
[   16.051430]  [<c032f34a>] highlevel_remove_host+0x21/0x42
[   16.051549]  [<c032ed9d>] hpsb_remove_host+0x37/0x56
[   16.051668]  [<e0869263>] ohci1394_pci_remove+0x44/0x1c7 [ohci1394]
[   16.051794]  [<c027e5b0>] pci_device_remove+0x16/0x35
[   16.053376]  [<c02da6d7>] __device_release_driver+0x6e/0x8b
[   16.053496]  [<c02dab77>] driver_detach+0xa1/0xde
[   16.053613]  [<c02da33f>] bus_remove_driver+0x57/0x75
[   16.053733]  [<c02dabd4>] driver_unregister+0x8/0x13
[   16.053850]  [<c027e732>] pci_unregister_driver+0xc/0x6e
[   16.053969]  [<c0134d56>] sys_delete_module+0x174/0x19a
[   16.054091]  [<c0113cea>] do_page_fault+0x277/0x525
[   16.054211]  [<c0148b0d>] do_munmap+0x193/0x1ac
[   16.054331]  [<c0103d0c>] syscall_call+0x7/0xb
[   16.054450]  =======================
[   16.054523] Code: ff c3 85 c0 74 08 83 c0 08 e9 9b f8 ea ff b8 ea ff ff ff c3 85 c0 74 08 83 c0 08 e9 b9 db ea ff c3 57 89 c7 56 53 31 db 8b 70 44 <83> be 94 00 00 00 00 75 09 eb 17 89 f8 e8 d7 ff ff ff 89 da 83 
[   16.057248] EIP: [<c02daf3d>] class_device_remove_attrs+0xa/0x30 SS:ESP 0068:df506e38

Tobias						PGP: http://9ac7e0bc.uguu.de

You missed the following entry in my list [1]:

Subject : Oops in __nodemgr_remove_host_dev
References : http://lkml.org/lkml/2007/3/14/4

             http://lkml.org/lkml/2007/3/18/87

Submitter : Ismail Dönmez <[email protected]>
Stefan Richter <[email protected]> Thomas Meyer <[email protected]>

             commit 43cb76d91ee85f579a69d42bc8efc08bac560278
             commit 40cf67c5fcc513406558c01b91129280208e57bf

Status : problem is being debugged

cu
Adrian

[1] not meant as an offence - there are so many items in the list that it's easy to miss one

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

According to the console log, it seems to be hung because a lot of processes are stuck in D state in various variations of this:

	Call Trace:
	 [<c01ba134>] start_this_handle+0x2d7/0x355
	 [<c01ba265>] journal_start+0xb3/0xe1
	 [<c01b2837>] ext3_journal_start_sb+0x48/0x4a
	 [<c01b0924>] ext3_create+0x47/0xe2
	 [<c017820c>] vfs_create+0xcd/0x13e
	 [<c017ab6e>] open_namei+0x176/0x5b5
	 [<c0170026>] do_filp_open+0x26/0x3b
	 [<c017007e>] do_sys_open+0x43/0xc2
	 [<c0170135>] sys_open+0x1c/0x1e
	 [<c0104064>] syscall_call+0x7/0xb

and then you have "kget" (whatever that is) which is doing

	Call Trace:
	 [<c0318981>] schedule_timeout+0x70/0x8e
	 [<c03189b4>] schedule_timeout_uninterruptible+0x15/0x17
	 [<c01b964a>] journal_stop+0xe2/0x1e6
	 [<c01ba2b0>] journal_force_commit+0x1d/0x1f
	 [<c01b29fb>] ext3_force_commit+0x22/0x24
	 [<c01ad607>] ext3_write_inode+0x34/0x3a
	 [<c0189f74>] __writeback_single_inode+0x1c5/0x2cb
	 [<c018a096>] sync_inode+0x1c/0x2e
	 [<c01a9ff7>] ext3_sync_file+0xab/0xc0
	 [<c018c8c5>] do_fsync+0x4b/0x98
	 [<c018c932>] __do_fsync+0x20/0x2f
	 [<c018c951>] sys_fdatasync+0x10/0x12
	 [<c0104064>] syscall_call+0x7/0xb

with kjournald in D sleep at

	 [<c01bb7b2>] journal_commit_transaction+0x15d/0x11d3
	 [<c01bfcbe>] kjournald+0xab/0x1e8
	 [<c01333dd>] kthread+0xb5/0xe0
	 [<c0104cd3>] kernel_thread_helper+0x7/0x10

which certainly looks like something is waiting for an IO to finish.

In contrast, the hang reported by Mariusz Kozlowski has a slightly different feel to it, but there's a tantalizing pattern in there too:

http://www.ussg.iu.edu/hypermail/linux/kernel/0703.0/1243.html

	Call Trace:
	[<c03ec87e>] io_schedule+0x42/0x59
	[<c0184915>] sleep_on_buffer+0x8/0xc
	[<c03ed217>] __wait_on_bit+0x47/0x6c
	[<c03ed297>] out_of_line_wait_on_bit+0x5b/0x64
	[<c01848a8>] __wait_on_buffer+0x27/0x2d
	[<c01b4228>] journal_commit_transaction+0x707/0x127f
	[<c01b868b>] kjournald+0xac/0x1ed
	[<c0126af5>] kthread+0xa2/0xc9
	[<c010422b>] kernel_thread_helper+0x7/0x1c

which certainly also looks like an IO never completed (or completed but never woke anything up).

It also seems to be related to *buffers*. Maybe the whole bh layer thing is a fluke, but it's not waiting for normal data, it's very much waiting for those journal things that all use buffer heads.Which just makes me worry about those patches by Nick (which did come in through Andrew). I don't think it's the memorder one (it looks safe and shouldn't matter on x86 anyway!), but what about the

fs: fix __block_write_full_page error case buffer submission

locking change for example? Or that "fs: fix nobh data leak" thing with its fix? It uses "SetPageUptodate(page);" without waking up anybody who might wait for it (but the waiters here seem to wait on buffers, so that's probably not it)..

Alternatively, maybe it really is an _io_ problem (and the buffer-head thing is just a red herring, and it could happen to other IO, it's just that metadata IO uses buffer heads), and it's the scheduler changes since 2.6.20..

Jens, Nick.. Could you take a look?

Linus

Nothing sleeps on PageUptodate, so I don't think that could explain it.

The fs: fix __block_write_full_page error case buffer submission patch does change the locking, but I'd be really suprised if that was the problem, because it changes locking to match the regular non-error path submission.

It could be possible that ext3 is doing something weird and expecting the old behaviour if it failed get_block, but that seems pretty weird to do, and would need fixing.

fs: nobh data leak... again hard to see how it could cause an unlock/wakeup to get lost. Is Mariusz using the nobh mount option?

It wouldn't hurt to test with these patches backed out...

I see what you mean. Could it be an ext3 or jbd change I wonder?

SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Good point. I forget that we just test "uptodate", but then always sleep on "locked".

I'd agree, except something clearly has changed ;^)

jbd hasn't changed since 2.6.20, and the ext3 changes are mostly things like const'ness fixes. And others were things like changing "journal_current_handle()" into "ext3_journal_current_handle()", which looked exciting considering that the hung processes were waiting for the journal, but the fact is, that's just an inline function that just calls the old function, so..

But interestingly, there *is* a "EA block reference count racing fix" that does move a lock_buffer()/unlock_buffer() to cover a bigger area. It looks "obviously correct", but maybe there's a deadlock possibility there with ext3_forget() or something?

Linus

I might missed something, so far I can't see a deadlock yet. If there is a deadlock, I think we should see ext3_xattr_release_block() and ext3_forget() on the stack. Is this the case?

Regards,
Mingming

[ Ok, I think it's those timers again...

Ingo: let me just state how *happy* I am that I told you off when you wanted to merge the hires timers and NO_HZ before 2.6.20 because they were "stable". You were wrong, and 2.6.20 is at least in reasonable shape. Now we just need to make sure that 2.6.21 will be too.. ]

No. What's strange is that two (maybe more, I didn't check) processes seem to be stuck in

	 [<c0318981>] schedule_timeout+0x70/0x8e
	 [<c03189b4>] schedule_timeout_uninterruptible+0x15/0x17
	 [<c01b964a>] journal_stop+0xe2/0x1e6
	 [<c01ba2b0>] journal_force_commit+0x1d/0x1f
	 [<c01b29fb>] ext3_force_commit+0x22/0x24
	 [<c01ad607>] ext3_write_inode+0x34/0x3a
	 [<c0189f74>] __writeback_single_inode+0x1c5/0x2cb
	 [<c018a096>] sync_inode+0x1c/0x2e
	 [<c01a9ff7>] ext3_sync_file+0xab/0xc0
	 [<c018c8c5>] do_fsync+0x4b/0x98
	 [<c018c932>] __do_fsync+0x20/0x2f
	 [<c018c960>] sys_fsync+0xd/0xf
	 [<c0104064>] syscall_call+0x7/0xb

but that that thing is literally:

		...
                do {
                        old_handle_count = transaction->t_handle_count;
                        schedule_timeout_uninterruptible(1);
                } while (old_handle_count != transaction->t_handle_count);
		...

and especially if nothing is happening, I'd not expect "transaction->t_handle_count" to keep changing, so it should stop very quickly.

Maybe it's CONFIG_NO_HZ again, and the problem is that timeout, and simply no timer tick happening?

Bingo. I think that's it.

	active timers:
	 #0: hardirq_stack, tick_sched_timer, S:01
	 # expires at 9530893000000 nsecs [in -2567889 nsecs]
	 #1: hardirq_stack, hrtimer_wakeup, S:01
	 # expires at 10858649798503 nsecs [in 1327754230614 nsecs]
	  .expires_next   : 9530893000000 nsecs

See

	http://lkml.org/lkml/2007/3/16/288

and that in turn points to the kernel log:

	http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.21-rc4/git-console.log

Seems convincing. Michal, can you post your .config, and if you had dynticks and hrtimers enabled, try reproducing without them?

http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.21-rc4/git-config

I don't know how to reproduce this bug on 2.6.21-rc4.On 2.6.21-rc2-mm1 it was very simple, just run youtube, bash_shared_mapping etc. In fact I didn't see this bug for a week.

Unfortunately, I wasn't able to take a crash dump because of sound card driver bug (I've got crash dump from 2.6.21-rc2-mm1).

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

OK... for some reason this is listed as a regression against 2.6.21-rc4.

You do have CONFIG_NO_HZ=y, and it is likely to be the cause of your 2.6.21-rc2-mm1 problems, but maybe there have been fixes since then? Ingo?

Due to
http://lkml.org/lkml/2007/3/16/288

cu
Adrian

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

hm. The sysrq-q info you provided shows a weird (=='impossible') high-res timers state, in that CPU#1's hrtimer_cpu_base->expires_next is at KTIME_MAX, and state has been there for around 8-9 minutes. The timers there just stay pending and never expire. Thomas and me have just spent 4 hours reviewing the affected code inside out and upside down but we can find no credible way for this condition to trigger. But it obviously happened on your box, and persisted for many minutes.

So to move this issue forward, i've written a hrtimers debug patch (attached) - which checks a couple of key assumptions in the hrtimers.c code, and which also extends the SysRq-Q debug info with this:

.expires_next : 88378000000 nsecs
.exp_prev : 88377000000 nsecs
last expires_next stacktrace:
update_cpu_base_expires_next+5f/63
hrtimer_interrupt+177/1b3
smp_apic_timer_interrupt+6e/80
apic_timer_interrupt+33/38
<ffffffff>

knowing which codepath updated expires_next to KTIME_MAX would be very helpful to us.

So could you please pick up latest git (12998096c or later), undo commit 25496caec (which broke my laptop - it might break your system too), and apply the attached patch, and keep lockdep enabled (so that you get CONFIG_STACKTRACE=y, essential for the stacktrace output above)? It might take some time for you to trigger this bug again, but that's the best idea we have so far. If we are lucky then one of the WARN_ON()s triggers much sooner.

if the hang occurs again then please do a SysRq-Q again and send us the output.

Ingo

---------------------------->
Subject: [patch] hrtimers debug patch
From: Ingo Molnar <[email protected]>

debugging helper for hrtimers. Keep a lookout for WARN_ON messages. Saves a stacktrace on every expires_next update, and makes that stack-trace available in SysRq-Q (or /proc/timer_list) output.

( make sure to run this on a lockdep-enabled kernel, so that
CONFIG_STACKTRACE=y. )

---
 arch/i386/kernel/apic.c    |    2 +
 include/linux/hrtimer.h    |    7 +++++
 kernel/hrtimer.c           |   54 +++++++++++++++++++++++++++++++++++----------
 kernel/time/tick-oneshot.c |   13 ++++++++++
 kernel/time/timer_list.c   |   21 ++++++++++++++++-
 5 files changed, 83 insertions(+), 14 deletions(-)

Index: linux/arch/i386/kernel/apic.c
===================================================================
@@ -523,6 +523,8 @@ void __init setup_boot_APIC_clock(void)
 		 */
 		if (nmi_watchdog != NMI_IO_APIC)
 			lapic_clockevent.features &= ~CLOCK_EVT_FEAT_DUMMY;
+		else
+			printk(KERN_WARNING "APIC timer registered as dummy, due to nmi_watchdog=1!\n");
 	}
 
 	/* Setup the lapic or request the broadcast */

Index: linux/include/linux/hrtimer.h
===================================================================
@@ -15,6 +15,7 @@
 #ifndef _LINUX_HRTIMER_H
 #define _LINUX_HRTIMER_H
 
+#include <linux/stacktrace.h>
 #include <linux/rbtree.h>
 #include <linux/ktime.h>
 #include <linux/init.h>

@@ -196,6 +197,12 @@ struct hrtimer_cpu_base {
 	struct hrtimer_clock_base	clock_base[HRTIMER_MAX_CLOCK_BASES];
 #ifdef CONFIG_HIGH_RES_TIMERS
 	ktime_t				expires_next;
+# define HRTIMERS_STACK_TRACE_DEPTH	32
+# ifdef CONFIG_STACKTRACE
+	struct stack_trace		exp_trace;
+	unsigned long			exp_entries[HRTIMERS_STACK_TRACE_DEPTH];
+# endif
+	ktime_t				exp_prev;
 	int				hres_active;
 	struct list_head		cb_pending;
 	unsigned long			nr_events;

Index: linux/kernel/time/tick-oneshot.c
===================================================================
@@ -73,8 +73,19 @@ int tick_switch_to_oneshot(void (*handle
 	struct clock_event_device *dev = td->evtdev;
 
 	if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT) ||
-	    !tick_device_is_functional(dev))
+		    !tick_device_is_functional(dev)) {
+
+		printk("could not switch to one-shot clockevents mode.\n");
+		if (!dev) {
+			printk("because no tick device\n");
+		} else {
+			if (!(dev->features & CLOCK_EVT_FEAT_ONESHOT))
+				printk("because %s does not support one-shot mode.\n", dev->name);
+			if (!tick_device_is_functional(dev))
+				printk("because %s is not functional.\n", dev->name);
+		}
 		return -EINVAL;
+	}
 
 	td->mode = TICKDEV_MODE_ONESHOT;
 	dev->event_handler = handler;

Index: linux/kernel/time/timer_list.c
===================================================================
@@ -46,7 +46,7 @@ static void print_name_offset(struct seq
 
 	sym_name = kallsyms_lookup(addr, &size, &offset, &modname, namebuf);
 	if (sym_name)
-		SEQ_printf(m, "%s", sym_name);
+		SEQ_printf(m, "%s+%lx/%lx", sym_name, offset, size);
 	else
 		SEQ_printf(m, "<%p>", sym);
 }

@@ -129,6 +129,23 @@ print_base(struct seq_file *m, struct hr
 	print_active_timers(m, base, now);
 }
 
+static void print_cpu_base_stack_trace(struct seq_file *m,
+				       struct hrtimer_cpu_base *cpu_base)
+{
+#ifdef CONFIG_STACKTRACE
+	struct stack_trace *trace = &cpu_base->exp_trace;
+	int i;
+
+	SEQ_printf(m, " last expires_next stacktrace:\n");
+	for (i = 0; i < trace->nr_entries; i++) {
+		SEQ_printf(m, "  ");
+		print_name_offset(m, (void *)trace->entries[i]);
+		SEQ_printf(m, "\n");
+	}
+	SEQ_printf(m, "\n");
+#endif
+}
+
 static void print_cpu(struct seq_file *m, int cpu, u64 now)
 {
 	struct hrtimer_cpu_base *cpu_base = &per_cpu(hrtimer_bases, cpu);

@@ -147,6 +164,8 @@ static void print_cpu(struct seq_file *m
 
 #ifdef CONFIG_HIGH_RES_TIMERS
 	P_ns(expires_next);
+	P_ns(exp_prev);
+	print_cpu_base_stack_trace(m, cpu_base);
 	P(hres_active);
 	P(nr_events);
 #endif

Can you please upload a disassembly of hrtimer_interrupt() ?

I stared at hrtimer_interrupt() for quite a long time and I don't see how it can leave without reprogramming the event, when there are timers pending. And there are enough timers pending.

tglx

http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.21-rc5/dis_hrtimer_interrupt

http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.21-rc5/gdb.debug

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

I just noticed, that 'dis hrtimer_interrupt > dis_hrtimer_interrupt' doesn't save the whole output.

http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.21-rc5/hrtimer_interrupt

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

thanks! I have stared at hrtimer.c a few more hours and the good news is that i found a narrow SMP race. The bad news is that i dont think it could explain your bug symptoms: the worst-case effect of the race should be an incorrect timeout on the current CPU - not a KTIME_MAX thing like your logs show.

But maybe i didnt think through the effects of the bug well enough, and your box has a HT CPU, with HT CPUs being pretty good at triggering narrow SMP races - so maybe we are lucky? Fix attached below. Patch is build and boot-tested.

Ingo

------------------>
Subject: [patch] hrtimers: fix reprogramming SMP race
From: Ingo Molnar <[email protected]>

hrtimer_start() incorrectly set the 'reprogram' flag to enqueue_hrtimer(), which should only be 1 if the hrtimer is queued to the current CPU.

doing otherwise could result in a reprogramming of the current CPU's clockevents device, with a timer that is not queued to it - resulting in a bogus next expiry value.

---
 kernel/hrtimer.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux/kernel/hrtimer.c
===================================================================
@@ -844,7 +844,12 @@ hrtimer_start(struct hrtimer *timer, kti
 
 	timer_stats_hrtimer_set_start_info(timer);
 
-	enqueue_hrtimer(timer, new_base, base == new_base);
+	/*
+	 * Only allow reprogramming if the new base is on this CPU.
+	 * (it might still be on another CPU if the timer was pending)
+	 */
+	enqueue_hrtimer(timer, new_base,
+			new_base->cpu_base == &__get_cpu_var(hrtimer_bases));
 
 	unlock_hrtimer_base(timer, &flags);
 

Thanks. Verification may take some time.

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

Hi all,

I didn't see this weird hang for 12 days. I think that this patch solves this issue.

Huge thans!

Regards,
Michal

Michal K. K. Piotrowski
LTG - Linux Testers Group (PL)
(http://www.stardust.webpages.pl/ltg/)
LTG - Linux Testers Group (EN)
(http://www.stardust.webpages.pl/linux_testers_group_en/)

cool :)

huge thanks to you for testing it!

Ingo

there's a new post-rc4 regression: my T60 hangs during early bootup. I bisected the hang down to this recent commit:

| commit 25496caec111481161e7f06bbfa12a533c43cc6f
| Author: Thomas Renninger <[email protected]>
| Date:   Tue Feb 27 12:13:00 2007 -0500
|
|    ACPI: Only use IPI on known broken machines (AMD, Dothan/BaniasPentium M)

undoing this change fixes my T60 so it correctly boots again.

the commit has this confidence-raising comment:

|   However, I am not sure about the naming of the parameter and how it 
|   could/should get integrated into the dyntick part 
|   (CONFIG_GENERIC_CLOCKEVENTS). There, a more fine grained check (TSC 
|   still running?, ..) is needed?

could we please revert this commit until it's done correctly?

and did this end up being a 'fix'? The change weakens the scope of a hardware workaround, which IMO has no place so late in the cycle. At a minimum the clockevents maintainer (Thomas) should have been Cc:-ed on it.

Ingo

Ingo,

I had seen it before, and I had no objections under the premise, that it does not break things and especially survives on Andrews VAIO. I expected that to come in via -mm so it gets enough testing.

We should revert that patch and add a "trust_lapic_timer_in_c2" commandline option instead. So we are on the safe side.

tglx

Here is a patch which applies after reverting
25496caec111481161e7f06bbfa12a533c43cc6f

It turned out that it is almost impossible to trust ACPI, BIOS & Co. regarding the C states. This was the reason to switch the local apic timer off in C2 state already. OTOH there are sane and well behaving systems, which get punished by that decision.

Allow the user to confirm that the local apic timer is trustworthy in C2 state. This keeps the default behaviour on the safe side.

index e39ab0c..09640a8 100644
@@ -780,6 +780,9 @@ and is between 256 and 4096 characters. It is defined in the file
 	lapic		[IA-32,APIC] Enable the local APIC even if BIOS
 			disabled it.
 
+	lapic_timer_c2_ok	[IA-32,APIC] trust the local apic timer in
+			C2 power state.
+
 	lasi=		[HW,SCSI] PARISC LASI driver for the 53c700 chip
 			Format: addr:<io>,irq:<irq>
 

index 244c3fe..e884152 100644
@@ -64,6 +64,9 @@ static int enable_local_apic __initdata = 0;
 static int local_apic_timer_verify_ok;
 /* Disable local APIC timer from the kernel commandline or via dmi quirk */
 static int local_apic_timer_disabled;
+/* Local APIC timer works in C2 */
+int local_apic_timer_c2_ok;
+EXPORT_SYMBOL_GPL(local_apic_timer_c2_ok);
 
 /*
  * Debug level, exported for io_apic.c

@@ -1232,6 +1235,13 @@ static int __init parse_disable_lapic_timer(char *arg)
 }
 early_param("nolapic_timer", parse_disable_lapic_timer);
 
+static int __init parse_lapic_timer_c2_ok(char *arg)
+{
+	local_apic_timer_c2_ok = 1;
+	return 0;
+}
+early_param("lapic_timer_c2_ok", parse_lapic_timer_c2_ok);
+
 static int __init apic_set_verbosity(char *str)
 {
 	if (strcmp("debug", str) == 0)

index 6077300..cdf7894 100644
@@ -268,6 +268,7 @@ static void acpi_timer_check_state(int state, struct acpi_processor *pr,
 				   struct acpi_processor_cx *cx)
 {
 	struct acpi_processor_power *pwr = &pr->power;
+	u8 type = local_apic_timer_c2_ok ? ACPI_STATE_C3 : ACPI_STATE_C2;
 
 	/*
 	 * Check, if one of the previous states already marked the lapic

@@ -276,7 +277,7 @@ static void acpi_timer_check_state(int state, struct acpi_processor *pr,
 	if (pwr->timer_broadcast_on_state < state)
 		return;
 
-	if (cx->type >= ACPI_STATE_C2)
+	if (cx->type >= type)
 		pr->power.timer_broadcast_on_state = state;
 }
 

index cc6b165..a19810a 100644
@@ -117,6 +117,7 @@ extern void enable_NMI_through_LVT0 (void * dummy);
 #define ARCH_APICTIMER_STOPS_ON_C3	1
 
 extern int timer_over_8254;
+extern int local_apic_timer_c2_ok;
 
 #else /* !CONFIG_X86_LOCAL_APIC */
 static inline void lapic_shutdown(void) { }

Hi!

...

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Yep, post .21.

I still twist my brain how to autodetect that in a safe way, which would make it really useful for the firmware tester.

tglx

I think the only fool-proof way to do this automatically is to
1. do a boot-time calibration vs HPET, RTC, or 8254 to make sure it starts sane. ideally, this boot time test would enter the deepest available C-state -- as that would catch 99% of the failures.
2. do a _continuous_ sanity check against the same time to make sure it never gets off track. It has to be continuous because we apparently have no control over
when the BIOS breaks it on some systems. However, continuous really
means "long term" here, because over the uptime of the system we'd
probably notice the drift between different timers etc, so we'd
have to reset the sanity check periodically to not get fooled by that.

If this worked, then we could delete the new DMI entry for the nx6325, as that would get detected and disable the lapic timer use automatically. We could also delete the check for C2 and the check for C3 to disable the lapic timer.

-Len

Why not just take the known-good CPUID signature?

Screw firmware or ACPI tables. They're going to be occasionally wrong.

If we know that "Core 2, version X" has a good local APIC timer, we use it. Otherwise we don't.

That's generally how we handle other APIC bugs too (the read-after-write thing, for example, or the differences between integrated and off-chip APIC's). Sometimes we check the APIC version itself, sometimes we check the CPUID information, and sometimes we check both ("modern_apic()").

Linus

Yep, this is what we tried to do last week.
It failed, and the patch was reverted.

I agree, the BIOS vendor can lie with ACPI tables. In particular, they can map any hardware C-state to any ACPI C-state. Our expectation that they would not map hardware C3 to ACPI C2 appears at this point to have been invalid.

So, speaking for Intel parts, every single one that supports HW C3 from the beginning of history through today has a broken LAPIC timer. (and a few listed in that patch are known to
be broken in HW C2) If we can't guarantee that the BIOS vendor
will not map that broken HW C3 to ACPI C2 (or even C1 via SMM) then we have to not use the LAPIC timer except for systems with a "known-good" signature = "part supports only C1".

If we really care about using the LAPIC timer on systems with deeper than C1 support, the only alternative seems to be to test if it actually works or not at boot and run-time. Otherwise, we wait for future hardware with guaranteed not to break under any (BIOS) conditions ships, and check for that.

Based on what I read of the HP nx6325 where the LAPIC timer is breaking C1, AMD is in the same boat.

-Len

On an Intel processor, it seems that the safe and simple route is if the system exports C2 or deeper, don't use the LAPIC timer. (which is what 2.6.21-rc5 is doing as of this moment) We may be able to white-list some systems over time.

The nx6325 (Turion 64 X2) exports only C1.
I'm not sure how the conclusion was drawn that it has a broken lapic timer as reflected in the "nolapic_timer" patch:

+ /*

+                * BIOS exports only C1 state, but uses deeper power
+                * modes behind the kernels back.
+                */
+                 .callback = lapic_check_broken_bios,
+                 .ident = "HP nx6325",
+                 .matches = {

+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq nx6325"),
+ },
+ },

But if this is true, then I don't know how to determine on an AMD system if the LAPIC timer is guaranteed to work -- even for systems with just C1.

Jordan, William,
can you clarify?

thanks,
-Len

On AMD it is the same, except that there seems to be at least one system that does C2 like things while only exporting C1. That is why i proposed to check for a battery too -- if there is one always disable it too.

-Andi

If both cores goes into C1 at the same time, the chipset can move the processor into a C3 like state called C1e.

For K7 and K8 through and including revision E, the LAPIC timer is guaranteed to work in C1.

For K8 revisions F and G, and for upcoming family 0x10 and 0x11 parts, if either bit in MSRC001_0055[28:27] is set, C1e is enabled and the LAPIC timer cannot be trusted in C1.

AMD can craft a patch to sort this out as soon as we have an idea what the framework is going to look like.

-Mark Langsdorf
Operating Systems Research Center
AMD, Inc.

... and that seems to break the local APIC timer.

Just a snippet to detect it would be great. Then the dmi scan could be removed and replaced with that. This would be a 2.6.21 candidate imho over the DMI hack.

-Andi

Yes. The APIC timer still runs, but no longer has an HT link to send the signal on.

Reviewed but not tested. Needs to be wrapped in an AMD specific call.

#define ENABLE_C1E_MASK 0x18000000
#define CPUID_PROCESSOR_SIGNATURE 1
#define CPUID_XFAM 0x0ff00000
#define CPUID_XFAM_K8 0x00000000
#define CPUID_XFAM_10H 0x00100000
#define CPUID_XFAM_11H 0x00200000
#define CPUID_XMOD 0x000f0000
#define CPUID_XMOD_REV_F 0x00040000

	int safe_c1 = 1;
	u32 eax, lo, hi;
	eax = cpuid_eax(CPUID_PROCESSOR_SIGNATURE)
	switch (eax & CPUID_XFAM) {
	case CPUID_XFAM_K8:
		if ((eax & CPUID_XMOD) < CPUID_XMOD_REV_F)
			break;
	case CPUID_XFAM_10H:
	case CPUID_XFAM_11H:
		rdmsr(MSR_ENABLE_C1E, lo, hi);
		if (lo & ENABLE_C1E_MASK)
			safe_c1 = 0;
		break;
	default:
		/* err on the side of caution */
		safe_c1 = 0;
	}

-Mark Langsdorf
Operating Systems Research Center
AMD, Inc.

Here's a patch. I don't have a system with C1E, so i only tested that the apic timer still works on a older AMD box.

Would be good if someone with a Turion laptop, especially the HP nx6325 could test it with CONFIG_NO_HZ enabled.

-Andi

Disable local APIC timer use on AMD systems with C1E

AMD dual core laptops with C1E do not run the APIC timer correctly when they go idle. Previously the code assumed this only happened on C2 or deeper. But not all of these systems report support C2.

Use a AMD supplied snippet to detect C1E being enabled and then disable local apic timer use.

This supercedes an earlier workaround using DMI detection of specific systems.

Index: linux/arch/i386/kernel/apic.c
===================================================================
@@ -272,32 +272,6 @@ static void __devinit setup_APIC_timer(v
 }
 
 /*
- * Detect systems with known broken BIOS implementations
- */
-static int __init lapic_check_broken_bios(struct dmi_system_id *d)
-{
-	printk(KERN_NOTICE "%s detected: disabling lapic timer.\n",
-		       d->ident);
-	local_apic_timer_disabled = 1;
-	return 0;
-}
-
-static struct dmi_system_id __initdata broken_bios_dmi_table[] = {
-	{
-		/*
-		 * BIOS exports only C1 state, but uses deeper power
-		 * modes behind the kernels back.
-		 */
-		  .callback = lapic_check_broken_bios,
-		  .ident = "HP nx6325",
-		  .matches = {
-			DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq nx6325"),
-		  },
-	 },
-	 {}
-};
-
-/*
  * In this functions we calibrate APIC bus clocks to the external timer.
  *
  * We want to do the calibration only once since we want to have local timer

@@ -357,6 +331,44 @@ static void __init lapic_cal_handler(str
 	}
 }
 
+#define ENABLE_C1E_MASK         0x18000000
+#define CPUID_PROCESSOR_SIGNATURE       1
+#define CPUID_XFAM              0x0ff00000
+#define CPUID_XFAM_K8           0x00000000
+#define CPUID_XFAM_10H          0x00100000
+#define CPUID_XFAM_11H          0x00200000
+#define CPUID_XMOD              0x000f0000
+#define CPUID_XMOD_REV_F        0x00040000
+
+/* AMD systems with C1E don't have a working lAPIC timer. Check for that. */
+static __init int amd_apic_timer_broken(void)
+{
+	u32 lo, hi;
+	u32 eax = cpuid_eax(CPUID_PROCESSOR_SIGNATURE);
+	switch (eax & CPUID_XFAM) {
+	case CPUID_XFAM_K8:
+		if ((eax & CPUID_XMOD) < CPUID_XMOD_REV_F)
+			break;
+	case CPUID_XFAM_10H:
+	case CPUID_XFAM_11H:
+		rdmsr(MSR_K8_ENABLE_C1E, lo, hi);
+		if (lo & ENABLE_C1E_MASK)
+			return 1;
+                break;
+        default:
+                /* err on the side of caution */
+		return 1;
+        }
+	return 0;
+}
+
+static __init int apic_timer_broken(void)
+{
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+		return amd_apic_timer_broken();
+	return 0;
+}
+
 /*
  * Setup the boot APIC
  *

@@ -372,12 +384,12 @@ void __init setup_boot_APIC_clock(void)
 	long delta, deltapm;
 	int pm_referenced = 0;
 
-	/* Detect know broken systems */
-	dmi_check_system(broken_bios_dmi_table);
+	if (apic_timer_broken())
+		local_apic_timer_disabled = 1;
 
 	/*
 	 * The local apic timer can be disabled via the kernel
-	 * commandline or from the dmi quirk above. Register the lapic
+	 * commandline or from the test above. Register the lapic
 	 * timer as a dummy clock event source on SMP systems, so the
 	 * broadcast mechanism is used. On UP systems simply ignore it.
 	 */

Index: linux/include/asm-i386/msr.h
===================================================================
@@ -275,6 +275,8 @@ static inline void wrmsr_on_cpu(unsigned
 #define MSR_K7_FID_VID_CTL		0xC0010041
 #define MSR_K7_FID_VID_STATUS		0xC0010042
 
+#define MSR_K8_ENABLE_C1E		0xC0010055
+
 /* extended feature register */
 #define MSR_EFER 			0xc0000080
 

I think this looks better than what we have now, but it would look even better if the core CPUID stuff was in arch/i386/kernel/cpu/amd.c, and we simply had X86_FEATURE_BROKEN_C1_LAPIC etc..

And then the apic.c code would just check

	if (boot_cpu_has(X86_FEATURE_BROKEN_C1_LAPIC))
		return -1;

or similar.

Doing the same for C2 and C3 gives us a clean way to have all these per-vendor things in their relevant places, rather than having various vendor-specific checks sprinkled in random places..

That's *especially* true for something like this that can hit both on x86-64 and i386, where the cpuid logic is shared, but the APIC logic is *not* shared. If I read your patch correctly, this only fixes it on 32-bit platforms, and I don't think the problem is in any way 32-bit specific, is it?

Linus

Hi,

Please don't, this would break the interface logically. X86_FEATURE_xxx usually denotes a *feature*, not a "feature" (Micro$oft speak for "bug" ;).
IOW most flags express a positive attribute, not a negative one. An exception to this probably is X86_FEATURE_FXSAVE_LEAK and X86_FEATURE_CMP_LEGACY, but all others seem to be positive, so we might want to enforce this rule.

Thus, how about e.g. X86_FEATURE_LAPIC_C1_OK?
Or is this less precise from a "C1 working ok" detection point of view? (i.e. we'd just assume by default that most machines are ok except the ones where we have issue detection code for, which might be too fuzzy)

Andreas Mohr

Sure, we could make it positive instead, and I agree it would make code nicer to read.

I have no trouble at all with something like that instead. Anybody willing to cook up a patch?

Linus

We already have several of those. And negative is much easier.

-Andi

Ok fair point. Here's an updated patch.

It's not shared.

It is because 64bit doesn't have dyntick yet and doesn't try to use the lapic timer only by default. It has a "apicmaintimer" option, but that is never set automatically. When dyntick is ported over it will be needed there too.

-Andi

Disable local APIC timer use on AMD systems with C1E

AMD dual core laptops with C1E do not run the APIC timer correctly when they go idle. Previously the code assumed this only happened on C2 or deeper. But not all of these systems report support C2.

Use a AMD supplied snippet to detect C1E being enabled and then disable local apic timer use.

This supercedes an earlier workaround using DMI detection of specific systems.

Index: linux/arch/i386/kernel/apic.c
===================================================================
@@ -272,32 +272,6 @@ static void __devinit setup_APIC_timer(v
 }
 
 /*
- * Detect systems with known broken BIOS implementations
- */
-static int __init lapic_check_broken_bios(struct dmi_system_id *d)
-{
-	printk(KERN_NOTICE "%s detected: disabling lapic timer.\n",
-		       d->ident);
-	local_apic_timer_disabled = 1;
-	return 0;
-}
-
-static struct dmi_system_id __initdata broken_bios_dmi_table[] = {
-	{
-		/*
-		 * BIOS exports only C1 state, but uses deeper power
-		 * modes behind the kernels back.
-		 */
-		  .callback = lapic_check_broken_bios,
-		  .ident = "HP nx6325",
-		  .matches = {
-			DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq nx6325"),
-		  },
-	 },
-	 {}
-};
-
-/*
  * In this functions we calibrate APIC bus clocks to the external timer.
  *
  * We want to do the calibration only once since we want to have local timer

@@ -372,12 +346,12 @@ void __init setup_boot_APIC_clock(void)
 	long delta, deltapm;
 	int pm_referenced = 0;
 
-	/* Detect know broken systems */
-	dmi_check_system(broken_bios_dmi_table);
+	if (boot_cpu_has(X86_FEATURE_LAPIC_TIMER_BROKEN))
+		local_apic_timer_disabled = 1;
 
 	/*
 	 * The local apic timer can be disabled via the kernel
-	 * commandline or from the dmi quirk above. Register the lapic
+	 * commandline or from the test above. Register the lapic
 	 * timer as a dummy clock event source on SMP systems, so the
 	 * broadcast mechanism is used. On UP systems simply ignore it.
 	 */

Index: linux/include/asm-i386/msr.h
===================================================================
@@ -275,6 +275,8 @@ static inline void wrmsr_on_cpu(unsigned
 #define MSR_K7_FID_VID_CTL		0xC0010041
 #define MSR_K7_FID_VID_STATUS		0xC0010042
 
+#define MSR_K8_ENABLE_C1E		0xC0010055
+
 /* extended feature register */
 #define MSR_EFER 			0xc0000080
 

Index: linux/arch/i386/kernel/cpu/amd.c
===================================================================
@@ -22,6 +22,37 @@
 extern void vide(void);
 __asm__(".align 4\nvide: ret");
 
+#define ENABLE_C1E_MASK         0x18000000
+#define CPUID_PROCESSOR_SIGNATURE       1
+#define CPUID_XFAM              0x0ff00000
+#define CPUID_XFAM_K8           0x00000000
+#define CPUID_XFAM_10H          0x00100000
+#define CPUID_XFAM_11H          0x00200000
+#define CPUID_XMOD              0x000f0000
+#define CPUID_XMOD_REV_F        0x00040000
+
+/* AMD systems with C1E don't have a working lAPIC timer. Check for that. */
+static __cpuinit int amd_apic_timer_broken(void)
+{
+	u32 lo, hi;
+	u32 eax = cpuid_eax(CPUID_PROCESSOR_SIGNATURE);
+	switch (eax & CPUID_XFAM) {
+	case CPUID_XFAM_K8:
+		if ((eax & CPUID_XMOD) < CPUID_XMOD_REV_F)
+			break;
+	case CPUID_XFAM_10H:
+	case CPUID_XFAM_11H:
+		rdmsr(MSR_K8_ENABLE_C1E, lo, hi);
+		if (lo & ENABLE_C1E_MASK)
+			return 1;
+                break;
+        default:
+                /* err on the side of caution */
+		return 1;
+        }
+	return 0;
+}
+
 static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 {
 	u32 l, h;

@@ -241,6 +272,9 @@ static void __cpuinit init_amd(struct cp
 
 	if (cpuid_eax(0x80000000) >= 0x80000006)
 		num_cache_leaves = 3;
+
+	if (amd_apic_timer_broken())
+		set_bit(X86_FEATURE_LAPIC_TIMER_BROKEN, c->x86_capability);
 }
 
 static unsigned int __cpuinit amd_size_cache(struct cpuinfo_x86 * c, unsigned int size)

Index: linux/include/asm-i386/cpufeature.h
===================================================================
@@ -75,6 +75,7 @@
 #define X86_FEATURE_ARCH_PERFMON (3*32+11) /* Intel Architectural PerfMon */
 #define X86_FEATURE_PEBS	(3*32+12)  /* Precise-Event Based Sampling */
 #define X86_FEATURE_BTS		(3*32+13)  /* Branch Trace Store */
+#define X86_FEATURE_LAPIC_TIMER_BROKEN (3*32+ 14) /* lapic timer broken in C1 */
 
 /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
 #define X86_FEATURE_XMM3	(4*32+ 0) /* Streaming SIMD Extensions-3 */

On Fri, 30 Mar 2007 00:05:39 +0200, Andi Kleen wrote

This patch also works. If You want bootlog or output of /proc/interrupts I can post it.

--
Greetings - CeHo - Grzegorz Chwesewicz

On Fri, 30 Mar 2007 00:05:39 +0200, Andi Kleen wrote

I've tested this patch little bit more on my nx6325 and I've found scenario in which my box works slow. When I boot HP with connected AC (it boots fast), and then after boot I unplug AC and try to power HP off it's working very slow (powering off process take few minutes). On battery it's always booting and powering off fast. Can enybody with nx6325 confirm this ?

--
Greetings - CeHo - Grzegorz Chwesewicz

On Thu, 29 Mar 2007 22:49:37 +0200, Andi Kleen wrote

I have nx6325 with Turion.

ensima-hp linux-2.6 # cat .config|grep NO_HZ
CONFIG_NO_HZ=y

After patching system works ok on battery and on AC.

On battery:

cat /proc/interrupts

CPU0 CPU1

  0:     115771          0  local-APIC-edge-fasteoi   timer
  1:        508          1   IO-APIC-edge      i8042
  8:          1          1   IO-APIC-edge      rtc
 12:        147          2   IO-APIC-edge      i8042
 14:         36          1   IO-APIC-edge      ide0
 16:          0          1   IO-APIC-fasteoi   yenta, sdhci:slot0
 17:       4538        116   IO-APIC-fasteoi   eth0
 18:       1697          6   IO-APIC-fasteoi   libata, HDA Intel
 19:         50          1   IO-APIC-fasteoi   ohci_hcd:usb1, ohci_hcd:usb2,
ehci_hcd:usb3
 21:         30          9   IO-APIC-fasteoi   acpi
NMI:          0          0
LOC:          0     115601
ERR:          1
MIS:          0

sleep 10

CPU0 CPU1

  0:     125777          0  local-APIC-edge-fasteoi   timer
  1:        509          1   IO-APIC-edge      i8042
  8:          1          1   IO-APIC-edge      rtc
 12:        147          2   IO-APIC-edge      i8042
 14:         36          1   IO-APIC-edge      ide0
 16:          0          1   IO-APIC-fasteoi   yenta, sdhci:slot0
 17:       4749        116   IO-APIC-fasteoi   eth0
 18:       1704          6   IO-APIC-fasteoi   libata, HDA Intel
 19:         50          1   IO-APIC-fasteoi   ohci_hcd:usb1, ohci_hcd:usb2,
ehci_hcd:usb3
 21:         30          9   IO-APIC-fasteoi   acpi
NMI:          0          0
LOC:          0     125607
ERR:          1
MIS:          0

#######################################

On AC:

cat /proc/interrupts

CPU0 CPU1

  0:        261          0  local-APIC-edge-fasteoi   timer
  1:        346          1   IO-APIC-edge      i8042
  8:          1          1   IO-APIC-edge      rtc
 12:        147          2   IO-APIC-edge      i8042
 14:         36          1   IO-APIC-edge      ide0
 16:          0          1   IO-APIC-fasteoi   yenta, sdhci:slot0
 17:       1135        115   IO-APIC-fasteoi   eth0
 18:       1620          6   IO-APIC-fasteoi   libata, HDA Intel
 19:         50          1   IO-APIC-fasteoi   ohci_hcd:usb1, ehci_hcd:usb2,
ohci_hcd:usb3
 21:         24          9   IO-APIC-fasteoi   acpi
NMI:          0          0
LOC:       7562       9007
ERR:          0
MIS:          0

sleep 10

CPU0 CPU1

  0:        261          0  local-APIC-edge-fasteoi   timer
  1:        347          1   IO-APIC-edge      i8042
  8:          1          1   IO-APIC-edge      rtc
 12:        147          2   IO-APIC-edge      i8042
 14:         36          1   IO-APIC-edge      ide0
 16:          0          1   IO-APIC-fasteoi   yenta, sdhci:slot0
 17:       1411        115   IO-APIC-fasteoi   eth0
 18:       1627          6   IO-APIC-fasteoi   libata, HDA Intel
 19:         50          1   IO-APIC-fasteoi   ohci_hcd:usb1, ehci_hcd:usb2,
ohci_hcd:usb3
 21:         24          9   IO-APIC-fasteoi   acpi
NMI:          0          0
LOC:       7592       9184
ERR:          0
MIS:          0

--
Greetings - CeHo - Grzegorz Chwesewicz

On Thu, 29 Mar 2007 23:43:28 +0200, Grzegorz Chwesewicz wrote

<cut>

Bootlogs on AC and battery can be found here:
http://bugzilla.kernel.org/show_bug.cgi?id=8235

--
Greetings - CeHo - Grzegorz Chwesewicz

I didn't think we could reliably distingush mobile cpus with C2+ versus desktop CPUs without it. Or rather you would need a table for each new CPU revision Intel/AMD put out. That would be probably quite nightmarish to maintain.

-Andi

Damn. I applied your patch, but it breaks on x86-64:

drivers/acpi/processor_idle.c:271: error: 'local_apic_timer_c2_ok'
undeclared (f irst use in this function)

I really wish we had an x86-64 maintainer that understood that it's confusing that files in arch/i386/ are also used for arch/x86-64.

Linus

Sorry, that was unfair. The patch was simply buggy. It added the test to drivers/acpi/ *without* adding it to the architectures that used it, it wasn't an i386/x86-64 thing.

Thomas, please fix.

Linus

Here's a possible fix. It compiles. And I still wish we had common files.

ia64 shouldn't be affected, because ia64 doesn't #define the ARCH_APICTIMER_STOPS_ON_C3 flag (and then we don't use the "c2_ok" thing either. But this is still pretty damn ugly.

Maybe a field in "struct acpi_processor" for C2/C3 problems?

Linus

---

index 723417d..46acf4f 100644
@@ -47,6 +47,10 @@ int apic_calibrate_pmtmr __initdata;
 
 int disable_apic_timer __initdata;
 
+/* Local APIC timer works in C2? */
+int local_apic_timer_c2_ok;
+EXPORT_SYMBOL_GPL(local_apic_timer_c2_ok);
+
 static struct resource *ioapic_resources;
 static struct resource lapic_resource = {
 	.name = "Local APIC",

@@ -1192,6 +1196,13 @@ static __init int setup_nolapic(char *str)
 } 
 early_param("nolapic", setup_nolapic);
 
+static int __init parse_lapic_timer_c2_ok(char *arg)
+{
+	local_apic_timer_c2_ok = 1;
+	return 0;
+}
+early_param("lapic_timer_c2_ok", parse_lapic_timer_c2_ok);
+
 static __init int setup_noapictimer(char *str) 
 { 
 	if (str[0] != ' ' && str[0] != 0)

index e81d0f2..7cfb39c 100644
@@ -102,5 +102,6 @@ void switch_ipi_to_APIC_timer(void *cpumask);
 #define ARCH_APICTIMER_STOPS_ON_C3	1
 
 extern unsigned boot_cpu_id;
+extern int local_apic_timer_c2_ok;
 
 #endif /* __ASM_APIC_H */

You beat me by 30 seconds.

Right, ia64 does not see it.

Yes it is.

Hmm, the acpi processor stuff is modular.

tglx

agreed - this seems to be a genuine CONFIG_HIGH_RES_TIMERS=y bug. (which has probably not been fixed since -rc4 either, we have no bugfix in this area that could explain the expires_next==KTIME_MAX timer state visible in SysRq-Q.)

there seems to be a trend in the reports: HT P4 CPUs.

yes - i was quite wrong pushing it so hard. (and doubly so given your stated focus of making v2.6.20 a quiet release) Sorry :-/

yeah - we are working hard on it.

Ingo

Hello,

Just to make things clear. I didn't say I could reproduce it on 2.6.21-rc4. In fact I'm running 2.6.21-rc4-mm1 with no problems so far. I just replied to show my sysrq dumps of processes states with 2.6.21-rc2-mm1.

I could reproduce similar (but still each time slightly different) hangs on -mm series from 2.6.20-mm1 to 2.6.21-rc2-mm1. 2.6.21-rc3-mm1 worked well for me so not sure If my report is still valid here.

Sorry if I didn't make it clear enough.

As I previously noticed each time the system hang I/O activity to disk looked dead (couldn't even sysrq-s).

True. I'm using ext3.

No. He is not.

Regards,

Mariusz Kozlowski

This email lists some known regressions in Linus' tree compared to 2.6.20.

If you find your name in the Cc header, you are either submitter of one of the bugs, maintainer of an affectected subsystem or driver, a patch of you caused a breakage or I'm considering you in any other way possibly involved with one or more of these issues.

Due to the huge amount of recipients, please trim the Cc when answering.

Subject : x86_64: boot hangs unless CONFIG_PCIEPORTBUS=n and acpi=off
References : http://bugzilla.kernel.org/show_bug.cgi?id=8162
Submitter : Randy Dunlap <[email protected]>
Status : unknown

Subject : AMD Elan: Crash after "Allocating PCI resources"
References : http://bugzilla.kernel.org/show_bug.cgi?id=8161
Submitter : Vladimir Brik <[email protected]>

Status : patch available

Subject : ACPI regression with noapic
References : http://lkml.org/lkml/2007/3/8/468
Submitter : Ray Lee <[email protected]>
Status : unknown

Subject : acpi_serialize locks system during boot
References : http://bugzilla.kernel.org/show_bug.cgi?id=8171
Submitter : Colchao <[email protected]>

Patch : http://bugzilla.kernel.org/show_bug.cgi?id=8171
Status : patch available

Subject : NCQ problem with ahci and Hitachi drive (ACPI related)
References : http://lkml.org/lkml/2007/3/4/178

             http://lkml.org/lkml/2007/3/9/475

Submitter : Mathieu Bérard <[email protected]>

Status : problem is being debugged

Subject : kernels fail to boot with drives on ATIIXP controller
(ACPI/IRQ related)
References : https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229621

             http://lkml.org/lkml/2007/3/4/257

Submitter : Michal Jaegermann <[email protected]>
Status : unknown

Subject : libata: PATA UDMA/100 configured as UDMA/33
References : http://lkml.org/lkml/2007/2/20/294

             http://www.mail-archive.com/[email protected]/msg04115.html
             http://bugzilla.kernel.org/show_bug.cgi?id=8133
             http://bugzilla.kernel.org/show_bug.cgi?id=8164

Submitter : Fabio Comolli <[email protected]>
Plamen Petrov <[email protected]> Laurent Riffard <[email protected]>

Alan Cox <[email protected]>
Status : Alan: Some cases should be fixed now but probably not all
(eg the Nvidia one)

-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Patch is in Linus' tree now.

-Andi

Bug is rejected due to user error.
This seems to be a netconsole hang, not ACPI.

---
~Randy

Thanks for this information.

Is the netconsole hang a regression?

cu
Adrian

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

I have to do more debugging to find out what is going on. It may be PCIEPORTBUS-related, but 2.6.20 or later now boot for me without netconsole and hang when using netconsole. The ending lines in the console log are:

[   17.946918] PCI: Setting latency timer of device 0000:00:02.0 to 64
[   17.953211] assign_interrupt_mode Found MSI capability
[   17.958388] Allocate Port Service[0000:00:02.0:pcie00]
[   17.963565] Allocate Port Service[0000:00:02.0:pcie01]
[   17.968736] Allocate Port Service[0000:00:02.0:pcie02]
[   17.973903] Allocate Port Service[0000:00:02.0:pcie03]
[   17.979134] PCI: Setting latency timer of device 0000:00:03.0 to 64
[   17.985430] assign_interrupt_mode Found MSI capability
[   17.990565] Losing some ticks... checking if CPU frequency changed.
[   17.996857] Allocate Port Service[0000:00:03.0:pcie00]
[   18.002013] Allocate Port Service[0000:00:03.0:pcie01]
[   18.007175] Allocate Port Service[0000:00:03.0:pcie02]
[   18.012336] Allocate Port Service[0000:00:03.0:pcie03]
[   18.017552] PCI: Setting latency timer of device 0000:00:03.1 to 64
[   18.023848] assign_interrupt_mode Found MSI capability
[   18.029012] Allocate Port Service[0000:00:03.1:pcie00]

~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

[...]

I finally have time to start bisecting this. I'll have something in the next day.

Ray

This email lists some known regressions in Linus' tree compared to 2.6.20.

If you find your name in the Cc header, you are either submitter of one of the bugs, maintainer of an affectected subsystem or driver, a patch of you caused a breakage or I'm considering you in any other way possibly involved with one or more of these issues.

Due to the huge amount of recipients, please trim the Cc when answering.

Subject : ThinkPad X60: resume no longer works (PCI related?)
References : http://lkml.org/lkml/2007/3/13/3
Submitter : Dave Jones <[email protected]>
Jeremy Fitzhardinge <[email protected]>
Caused-By : PCI merge

             commit 78149df6d565c36675463352d0bfe0000b02b7a7

Rafael J. Wysocki <[email protected]>
Status : problem is being debugged

Subject : ThinkPad doesn't resume from suspend to RAM
References : http://lkml.org/lkml/2007/2/27/80

             http://lkml.org/lkml/2007/2/28/348

Submitter : Jens Axboe <[email protected]>
Jeff Chua <[email protected]>
Status : unknown

Subject : suspend to disk hangs
References : http://lkml.org/lkml/2007/3/6/142
Submitter : Jeff Chua <[email protected]>
Status : unknown

Subject : suspend to disk hangs
References : http://lkml.org/lkml/2007/3/16/126
Submitter : Maxim Levitsky <[email protected]>
Status : unknown

Subject : suspend to disk hangs
References : http://bugzilla.kernel.org/show_bug.cgi?id=8224
Submitter : Mike Harris <[email protected]>
Status : unknown

Subject : ThinkPad R60: suspend broken
References : http://lkml.org/lkml/2007/3/16/57
Submitter : Marcus Better <[email protected]>
Status : unknown

Subject : laptop immediately resumes after suspend
References : http://lkml.org/lkml/2007/3/8/469
Submitter : Ray Lee <[email protected]>

             commit ed41dab90eb40ac4911e60406bc653661f0e4ce1

Patch : http://lkml.org/lkml/2007/3/12/228
Status : patch available

Subject : second suspend to disk in a row results in an oops (libata?)
References : http://lkml.org/lkml/2007/3/17/43
Submitter : Thomas Meyer <[email protected]>
Status : unknown

Subject : SATA breakage on resume
References : http://lkml.org/lkml/2007/3/7/233
Submitter : Thomas Gleixner <[email protected]>
Soeren Sonnenburg <[email protected]>
Status : unknown

FWIW, it's still broken as of commit cd05a1f818073a623455a58e756c5b419fc98db9.

The in-kernel suspend to disk or RAM both hang after "Suspending consoles", with the fan stopped.

The good news is on 2.6.21-rc5, suspend to disk, and resume from disk works. But, it only works with CONFIG_NO_HZ unset.

Setting CONFIG_NO_HZ will cause suspend to disk to hang just before saving memory to disk.

Resume from RAM (s2ram) still broke (tried with or without CONFIG_NO_HZ). Suspend to RAM seems ok, but upon resume, the screen will only display "inu" and only after pressing the power button will the system return to console. But "date" still doesn't advance.

Thanks,
Jeff.

Thanks for this information.

Thanks, noted.

This might be related to the following regression:

Subject : first disk access after resume takes several minutes
('date' does not advance after resume from RAM, CONFIG_NO_HZ=n)
References : http://lkml.org/lkml/2007/3/8/117

             http://lkml.org/lkml/2007/3/25/20

Submitter : Michael S. Tsirkin <[email protected]>

Ingo Molnar <[email protected]>
Status : problem is being debugged

cu
Adrian

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Adrian,

It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can suspend and resume from RAM (s2ram). Even better, it works with/without CONFIG_NO_HZ.

But, suspend to disk still broke with CONFIG_NO_HZ set.

Thanks,
Jeff.

Does the patch below fix the HPET_TIMER=y case ?

tglx

index f3ab61e..76afea6 100644
@@ -197,7 +197,7 @@ static int hpet_next_event(unsigned long delta,
 	cnt += delta;
 	hpet_writel(cnt, HPET_T0_CMP);
 
-	return ((long)(hpet_readl(HPET_COUNTER) - cnt ) > 0);
+	return ((long)(hpet_readl(HPET_COUNTER) - cnt ) > 0) ? -ETIME : 0;
 }
 
 /*

Thomas, I tried, but it didn't help. Upon resume from ram, "date" still didn't advance.

Thanks,
Jeff.

Can you please issue a SysRq-Q in this situation and provide the dmesg output ?

Thanks,

tglx

Hi,
I almost sure Iknow why this happens,
The problem is that both hpet clock source and hpet clockevents doesn't have a suspend/resume function On resume we should enable the main counter _and_ enable legacy replacement mode,
On my system main counter in enabled, by I think by bios, but legacy replacement mode is not, so if a system doesn't use lapic as a tick source, but use hpet+broadcast, it will hang for sure on resume, and i tested it

The patch below is a temporally fix, until clock-events and clocksources will get proper suspend/resume hooks:

Regards,
Maxim Levitsky

---

Add suspend/resume for HPET

---

index 0fd9fba..a1ec79e 100644
@@ -152,6 +152,16 @@ static void hpet_set_mode(enum clock_event_mode mode,
 	unsigned long cfg, cmp, now;
 	uint64_t delta;
 
+
+	if ( mode != CLOCK_EVT_MODE_UNUSED && mode != CLOCK_EVT_MODE_SHUTDOWN)
+	{
+		unsigned long cfg = hpet_readl(HPET_CFG);
+		cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY;
+		hpet_writel(cfg, HPET_CFG);
+		
+	}
+		
+
 	switch(mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
 		delta = ((uint64_t)(NSEC_PER_SEC/HZ)) * hpet_clockevent.mult;

cool! Your patch is a definite improvement on my t60 (where suspend/resume never worked with hpet enabled). But it does not fix everything - for example the timings are way off after resume. Thomas?

Ingo

The problem is that HPET consists of two parts:
one is a main counter and second is a a timers.

To enable main counter one must set HPET_CFG_ENABLE. It is set only on boot, and not on resume now.

But on my system I think bios re-enables it.

Secondary to enable HPET to act as a timer on IRQ0 and to make it replace RTC IRQ8 one must set HPET_CFG_LEGACY

This bit is definitely not set on resume, so on my system I get a hang if I use HPET as clockevents device, and on other system where bios doesn't reenable HPET, then even if HPET is used as timing device 'date won't advance'

But I set those bits only in initialization of HPET clockevents device, and I set always, when it is turned on, It is not that good,but works.

Now I don't have a clue how to set those bits if only HPET is used as clock source because now clocksources don't have _any_ resume hook.

The timing problem you mention I think isd connected to the fact that HPET is not enabled instantly after resume, but after some time when clockevents core calls HPET to enable event timer.

Best regards,
Maxim Levitsky

One thing that drives me wild about that "clocksource resume" thing is that it seems to think that clocksources are somehow different from any other system devices..

Why isn't the HPET considered a "device", and has it's own *device* "suspend" and "resume"? Why do we seem to think that only "set_mode()" etc should wake up clock sources?

It's a *device*, dammit. It should save and resume like one (probably as a system device). The "set_mode()" etc stuff is at a completely different (higher) conceptual level.

Thomas? It does seem like Maxim has hit the nail on the head (at least partly) on the HPET timer resume problems..

Linus

Agreed, except about "probably as a system device".

Last I checked, there was no good reason to use sysdev suspend()/resume() rather than platform_device suspend_late()/early_resume(). Which more or less means no good reason to use sysdev in new code...

Also, making HPET use the legacy mode seems like a step backwards.

- Dave

Hi,
It is not 'legacy' mode,
It is a legacy replacement mode.
It this mode HPET takes over IRQ0 and IRQ 8 and provides this way replacement for PIT and RTC periodic function

Best regards,
Maxim Levitsky

Typo, sorry.

It's that RTC periodic thing that bothers me, I don't mind about the PIT. Remember that IRQ8 is also used for other RTC functions.

Now, if there were a way to tell rtc-cmos that HPET is active, and arrange some kind of handshake ... that would be different.

- Dave

Yes,
When HPET is active it eats RTC IRQ,
So the only way out is to emulate RTC using HPET,
It is done this way in old rtc driver, rtc-cmos should do the same.

Of course suspend resume is not supported at all by old rtc driver

I already wrote complete support for suspend/resume for old rtc driver (I wrote it long time ago)

Now I fixed it to support HPET , and this way I discovered that HPET doesn't have suspend resume functions

I will do last checks now and send this patch very soon

I am also planning to add support of HPET and suspend/resume for rtc-cmos, but I didn't start this yet.

Best regards,
Maxim Levitsky

Only when HPET timers 0 and 1 are set up for "Legacy Replacement Mode". In the more sensible "Standard Mode", they have their own IRQs.

No. Patches like

http://marc.info/?l=linux-kernel&m=117219531503973&w=2

should be merged (I hope they're in the 2.6.22 queue!), making HPET run in "Standard Mode" so that HPET can stop sticking its fingers in code where they don't belong.

It's already got suspend/resume support, and in the 2.6.22 queue are RTC framework updates which will let the RTC framework replace a lot more platform-specific RTC support. (Platform changes can come later, where they're needed. ARM for example doesn't need any.)

Once HPET stops using "Legacy Replacement Mode" you won't need to touch anything in the RTC stack (except maybe the legacy char/rtc.c driver, removing HPET stuff).

The open issue with suspend/resume support in rtc-cmos relates to how ACPI wakeup alarms should trigger. I've not made time to test those patches.

- Dave

Hi,
It is not that simple,

Only in legacy replacement mode HPET can be put on IRQ0 (and sadly IRQ8) At least this is true on some systems, on mine for example

On my system first 2 hpet timers can only be assigned to IRQ21-23 and third to ether IRQ11, IRQ21-IRQ23

Or in legacy replacement mode first is assigned IRQ0 and second IRQ8

this will make it difficult to use it as a clockevents source

Not to mention the fact that current code assumes that BIOS assigned IRQs to all timers which is not true on my system.

I have brand new intel DG965 motherboard.

What is wrong with relying on HPET to provide RTC IRQ ?

Best regards,
Maxim Levitsky

also: http://marc.info/?l=linux-kernel&m=117219531526317&w=2

Right, that's the entire point of legacy replacement mode.

But so what? In standard mode, HPET just uses other IRQs. Nothing would care about irq0, and irq8 would be used by the RTC as necessary.

Why? It's not like the clockevent logic cares what IRQ a given programmable timer uses. So long as the HPET driver can receive its IRQ, it'll make a fine clockevent. There's no reason to have HPET prevent other drivers from working, by insisting it use that nasty "prevent other hardware from issuing IRQs" mode.

The patches from Venkatesh sure seem to have behaved for him. And while he might have complained about difficulty, I think that'd be more likely due to the SMP issues he also addressed than because of some (historical?) attachment to irq0.

Getting IRQ routing sorted out is a problem that's been solved numerous times before. And again, the patches referenced above seem to have resolved any such issues.

For starters, it's not an RTC. Why in the world would you want to make the OS think it's an RTC ... unless you're Microsoft, and are desperate to get another periodic timer (and don't much care about the other RTC functionality?

... that's all off-topic for 2.6.21 regressions though; it's too late to merge x86_64 clockevent support, or fix HPET issues like not using "standard mode".

- Dave

No they don't,

First patch does that:

                        hd.hd_irq[i] = (timer->hpet_config &
                                        Tn_INT_ROUTE_CNF_MASK) >>                                
					Tn_INT_ROUTE_CNF_SHIFT;

This just reads values assigned by BIOS.

Hi,
I feel that you are right,

You meant that one of HPET timers will be used as clock source and will be assigned some IRQ (not IRQ0)

Seems fine,

Only one thing: the kernel must assign IRQs to HPETS , relying on bios is not good, Also the IRQ for clocksource should be not shared, but maybe I am wrong here (I am afraid that latencies might be a problem here)

By the way I never thought about the fact that legacy replacement mode is a 'virtual legacy'

I mean that it is intended to simulate RTCs and PITs on systems that don't have them, am I right here ? HPET spec says that RTC is still requred to provide all its usial functions except periodic freq.

Best regards,
Maxim Levitsky

also: http://marc.info/?l=linux-kernel&m=117219531526317&w=2

Right, that's the entire point of legacy replacement mode.

But so what? In standard mode, HPET just uses other IRQs. Nothing would care about irq0, and irq8 would be used by the RTC as necessary.

Why? It's not like the clockevent logic cares what IRQ a given programmable timer uses. So long as the HPET driver can receive its IRQ, it'll make a fine clockevent. There's no reason to have HPET prevent other drivers from working, by insisting it use that nasty "prevent other hardware from issuing IRQs" mode.

The patches from Venkatesh sure seem to have behaved for him. And while he might have complained about difficulty, I think that'd be more likely due to the SMP issues he also addressed than because of some (historical?) attachment to irq0.

Getting IRQ routing sorted out is a problem that's been solved numerous times before. And again, the patches referenced above seem to have resolved any such issues.

For starters, it's not an RTC. Why in the world would you want to make the OS think it's an RTC ... unless you're Microsoft, and are desperate to get another periodic timer (and don't much care about the other RTC functionality?

... that's all off-topic for 2.6.21 regressions though; it's too late to merge x86_64 clockevent support, or fix HPET issues like not using "standard mode".

- Dave

Only when HPET timers 0 and 1 are set up for "Legacy Replacement Mode". In the more sensible "Standard Mode", they have their own IRQs.

No. Patches like

http://marc.info/?l=linux-kernel&m=117219531503973&w=2

should be merged (I hope they're in the 2.6.22 queue!), making HPET run in "Standard Mode" so that HPET can stop sticking its fingers in code where they don't belong.

It's already got suspend/resume support, and in the 2.6.22 queue are RTC framework updates which will let the RTC framework replace a lot more platform-specific RTC support. (Platform changes can come later, where they're needed. ARM for example doesn't need any.)

Once HPET stops using "Legacy Replacement Mode" you won't need to touch anything in the RTC stack (except maybe the legacy char/rtc.c driver, removing HPET stuff).

The open issue with suspend/resume support in rtc-cmos relates to how ACPI wakeup alarms should trigger. I've not made time to test those patches.

- Dave

I won't disagree - it might well be much nicer to just show it in the "real" device tree. I'm not 100% sure where in the tree it would go, though. It should probably be "inside" the root entry, before any of the PCI buses. It's generally what we've used those "system device" things for, but I agree that it would be better to just make system devices show up early on the regular device list than it is to have them be special cases.

Bit I think that's a separate (and fairly small) issue compared to the "don't use the clocksource infrastructure as a make-believe suspend/resume mechanism" problem that Maxim's patch had.

(Maxim, don't take that the wrong way - I think your analysis and patch were great, I just think another organization would be better)

I don't think that's actually "legacy" in any sense but the interrupt delivery, where the "legacy mode" bit is not so much that the HPET itself is "legacy" but that it *replaces* legacy devices.

But I may have misunderstood the thing. I'm an old fart, so I know the old timers much better than I know the new ones ;). Somebody feel free to hit me with the clue-2x4.

Linus

Mixing "inside" and "before" is a small linguistic clue about one of the issues with driver model PM. Off topic here; and in terms of suspend/resume callback sequencing that answer shouldn't matter much for HPET (as I understand things).

Yes -- where "platform_device" is a regular Joe-Sixpack kind of device, but "sysdev" is a special case.

Agreed -- although isn't it the "clockevent" change which is at issue?

A "clockevent" thingie wraps various kinds of timer IRQs; the clocksource is conceptually just a free run counter. Clocksources have been around for a while, with no particular problems.

It's clockevent sources have been the problem with dynamic tick solutions all along, since they mask such chaos inside x86 hardware and interact with so many different parts of the kernel. ;)

- Dave

Exactly, I agree completely
I said that my patch was a temporary fix, and I agree that the best way is to create a new system device and use its suspend/resume hooks to bring HPET back to life on resume.

Best regards,
Maxim Levitsky

Hi,
I am sending here a patch that as was discussed here adds hpet to list of system devices and adds suspend/resume hooks this way.
I tested it and it works fine.

---
Add suspend/resume support for HPET

---
 arch/i386/kernel/hpet.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)

index 0fd9fba..ac41476 100644
@@ -3,6 +3,8 @@
 #include <linux/errno.h>
 #include <linux/hpet.h>
 #include <linux/init.h>
+#include <linux/sysdev.h>
+#include <linux/pm.h>
 
 #include <asm/hpet.h>
 #include <asm/io.h>

@@ -524,3 +526,65 @@ irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 #endif
+
+
+/*
+ * Suspend/resume part
+ */
+
+#ifdef CONFIG_PM
+
+static int hpet_suspend(struct sys_device *sys_device, pm_message_t state)
+{
+	unsigned long cfg = hpet_readl(HPET_CFG);
+
+	cfg &= ~(HPET_CFG_ENABLE|HPET_CFG_LEGACY);
+	hpet_writel(cfg, HPET_CFG);
+
+	return 0;
+}
+
+static int hpet_resume(struct sys_device *sys_device)
+{
+	unsigned int id;
+
+	hpet_start_counter();
+
+	id = hpet_readl(HPET_ID);
+
+	if (id & HPET_ID_LEGSUP)
+		hpet_enable_int();
+
+	return 0;
+}
+
+static struct sysdev_class hpet_class = {
+	set_kset_name("hpet"),
+	.suspend	= hpet_suspend,
+	.resume		= hpet_resume,
+};
+
+static struct sys_device hpet_device = {
+	.id		= 0,
+	.cls		= &hpet_class,
+};
+
+
+static __init int hpet_register_sysfs(void)
+{
+	int err;
+
+	err = sysdev_class_register(&hpet_class);
+
+	if (!err) {
+		sysdev_register(&hpet_device);
+		if (err)
+			sysdev_class_unregister(&hpet_class);
+	}
+
+	return err;
+}
+
+device_initcall(hpet_register_sysfs);
+
+#endif

1.4.4.2

Ok, it certainly looks better, but it *also* looks like it just assumes the HPET is there. Which would work in testing _with_ a HPET, but would likely break on hardware without one, no?

Shouldn't there be at least something like a

	if (!is_hpet_capable())
		return 0;

at the top of that init routine? I'd also expect that you'd need to check that "hpet_virt_address" is valid or something?

(Or, better yet, shouldn't we set "boot_hpet_disable" when we decide not to use the HPET, and set hpet_virt_address to NULL?)

Linus

This is done here

out_nohpet:

	iounmap(hpet_virt_address);
	hpet_virt_address = NULL;

Hi,
Of course, I forgot.

I was planning to put sysdev code in hpet_enable() but it is not possible because this function is called too early.

Thus I put sysdev initialization in separate function but forgot to test for HPET

Thanks a lot.

Best regards
Maxim Levitsky

---
This adds support of suspend/resume on i386 for HPET

---
 arch/i386/kernel/hpet.c |   68 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)

index 0fd9fba..7c67780 100644
@@ -3,6 +3,8 @@
 #include <linux/errno.h>
 #include <linux/hpet.h>
 #include <linux/init.h>
+#include <linux/sysdev.h>
+#include <linux/pm.h>
 
 #include <asm/hpet.h>
 #include <asm/io.h>

@@ -310,6 +312,7 @@ int __init hpet_enable(void)
 out_nohpet:
 	iounmap(hpet_virt_address);
 	hpet_virt_address = NULL;
+	boot_hpet_disable = 1;
 	return 0;
 }
 

@@ -524,3 +527,68 @@ irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 #endif
+
+
+/*
+ * Suspend/resume part
+ */
+
+#ifdef CONFIG_PM
+
+static int hpet_suspend(struct sys_device *sys_device, pm_message_t state)
+{
+	unsigned long cfg = hpet_readl(HPET_CFG);
+
+	cfg &= ~(HPET_CFG_ENABLE|HPET_CFG_LEGACY);
+	hpet_writel(cfg, HPET_CFG);
+
+	return 0;
+}
+
+static int hpet_resume(struct sys_device *sys_device)
+{
+	unsigned int id;
+
+	hpet_start_counter();
+
+	id = hpet_readl(HPET_ID);
+
+	if (id & HPET_ID_LEGSUP)
+		hpet_enable_int();
+
+	return 0;
+}
+
+static struct sysdev_class hpet_class = {
+	set_kset_name("hpet"),
+	.suspend	= hpet_suspend,
+	.resume		= hpet_resume,
+};
+
+static struct sys_device hpet_device = {
+	.id		= 0,
+	.cls		= &hpet_class,
+};
+
+
+static __init int hpet_register_sysfs(void)
+{
+	int err;
+
+	if (!is_hpet_capable())
+		return 0;
+
+	err = sysdev_class_register(&hpet_class);
+
+	if (!err) {
+		sysdev_register(&hpet_device);
+		if (err)
+			sysdev_class_unregister(&hpet_class);
+	}
+
+	return err;
+}
+
+device_initcall(hpet_register_sysfs);
+
+#endif