Re: [PATCH] Fix for rtc.c non-atomic access to rtc_status

From: Manfred Spraul (manfreds@colorfullife.com)
Date: Sat Apr 22 2000 - 07:40:07 EST


Cesar Eduardo Barros wrote:
>
> (not on the list, please CC: replies to me)
>
> Using atomic_* seems to cause a false sense of security in some people.
> atomic_set(...,atomic_read(...)) is not atomic, and rtc.c did it everywhere.
> Also, testing for a bit and later enabling it (like in rtc_open) causes races
> (two proccesses in SMP could open the rtc at the same time if they got the
> right timing). I patched it to use a spinlock instead.
>

rtc_release, rtc_open and rtc_ioctl are protected by lock_kernel(), so
they are protected from SMP races.

>
> if (rtc_async_queue)
> + /* Is this OK on interrupt? */
> kill_fasync (rtc_async_queue, SIGIO, POLL_IN);
>

yes.

> @@ -699,8 +717,13 @@
> {
> /* interrupts and maybe timer disabled at this point by rtc_release */
>
> - if (atomic_read(&rtc_status) & RTC_TIMER_ON)
> + /* FIXME: how rtc_open here could be avoided? */

the magic of lock_kernel() helps:
rtc_exit() and rtc_open() both run with the big kernel lock.

I'm far more concerned about the xchg() in rtc_read:
that xchg() and rtc_interrupt() aren't synchronized, the rest could be
fixed with test_and_set(), atomic_mask(),...

--
	Manfred

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sun Apr 23 2000 - 21:00:20 EST