On 16/03/2021 19:04:14+0100, Lukasz Stelmach wrote: > OK, you are right. The problem seems to be elsewhere. > > How about this scnario? We call rtc_update_irq_enable(). We read rtc > with __rtc_read_time() and calculate the alarm time. We get through > rtc_timer_enqueue() and down to __rtc_set_alarm(). We loose the race > condition (I can do it, I've got really slow connection to DS3231) and > we return -ETIME from __rtc_set_alarm() > > if (scheduled <= now) > return -ETIME; > > and 0 from rtc_timer_enqueue() and the very same zero from > rtc_update_irq_enable(). The caller of ioctl() thinks they can expect > interrupts when, in fact, they won't receive any. > > The really weird stuff happens in rtc_timer_do_work(). For the timer to > be dequeued __rtc_set_alarm() needs to return EINVAL three times in a > row. In my setup this doesn't happen and the code keeps running loops > around "reporogram" and "again" labels. > > With my patch we never risk the above race condition between > __rtc_read_time() in rtc_update_irq_enable() and the one in > __rtc_set_alarm(), because we know rtc doesn't support alarms before we > start the race. In fact there is another race between __rtc_read_time() > and actually setting the alarm in the chip. > > IMHO the solution is to introduce RTC_HAS_ALARM flag for struct > rtc_device and check it at the very beginning of __rtc_set_alarm() the > same way it is being done in ds1337_set_alarm(). What are your thoughts? > I did introduce RTC_FEATURE_ALARM for that in v5.12. I'm sending patches that are not well tested but should solve your issue. -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
