Re: [REGRESSION] rtc/interface.c: kills suspend-to-ram

[Mark Lord <kernel@teksavvy.com>]

On 12-04-17 04:11 PM, Mark Lord wrote:
> On 12-04-17 01:13 AM, John Stultz wrote:
> ..
>> -    rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
>> +    //rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
>> +    dump_stack();
> ..
> 
> Okay, the call into here is coming from a "hwclock -w -u" line
> in the system suspend script.

Forgot the stack dump:

Pid: 4353, comm: hwclock Tainted: P           O 3.3.2 #5
Call Trace:
 [<ffffffff8123cfcd>] ? rtc_timer_remove+0x66/0xb2
 [<ffffffff8103d2ff>] ? should_resched+0x5/0x23
 [<ffffffff8123d21b>] ? rtc_update_irq_enable+0xd0/0x108
 [<ffffffff812dd582>] ? __mutex_lock_common.isra.5+0x3b/0x166
 [<ffffffff8123e058>] ? rtc_dev_ioctl+0x36d/0x468
 [<ffffffff8101b78a>] ? do_page_fault+0x264/0x2ce
 [<ffffffff81027650>] ? timespec_add_safe+0x33/0x63
 [<ffffffff810077a8>] ? read_tsc+0x5/0x14
 [<ffffffff810483fa>] ? timekeeping_get_ns+0xd/0x2a
 [<ffffffff810ab492>] ? do_vfs_ioctl+0x45a/0x49c
 [<ffffffff810abb8e>] ? poll_select_copy_remaining+0xdb/0xfb
 [<ffffffff810ab511>] ? sys_ioctl+0x3d/0x60
 [<ffffffff812df222>] ? system_call_fastpath+0x16/0x1b

> Since that command isn't touching the hardware Alarm,
> then neither should the Linux kernel.  Yet it is touching it.
> 
>>      CMOS_WRITE(rtc_control, RTC_CONTROL);
>> -    hpet_mask_rtc_irq_bit(mask);
>> +    //hpet_mask_rtc_irq_bit(mask);
>>
>> -    cmos_checkintr(cmos, rtc_control);
>> +    //cmos_checkintr(cmos, rtc_control); 
> ...
> 
> The problem still occurs (lockup on suspend)
> with both lines above commented out.
> 
> Note that it's not 100% in any case, more like 8/10,
> indicating a possible strong race condition somewhere.
> 
> I think all that should be done here, is to change the kernel
> to NOT enable/disable the Alarm unless told to do so by
> an explicit userspace action.  Eg. writing to /sys/../wakealarm
> and/or /proc/acpi/alarm.
> 
> If userspace leaves the alarm alone, then so should the kernel when possible.
> That's the old behaviour before the new alarm_irq_enable() stuff.
> 
> Cheers