Messages in this thread Patch in this message | | | From | "Liu, Chuansheng" <> | Subject | RE: [PATCH 2/3] alarmtimer: Using the alarmtimer_get_rtcdev for all posix clock interface | Date | Fri, 2 Nov 2012 06:10:45 +0000 |
| |
> rtcdev = NULL > > CPU0 CPU 1 > > alarmtimer_rtc_add_device(A) > if (rtcdev) alarmtimer_rtc_add_device(B) > return -EBUSY; if (rtcdev) > mutex_lock(); return -EBUSY; > rtcdev = A; mutex_lock(); > mutex_unlock; > > bla = alarmtimer_rtc_get_device() > > So bla = A > mutex_lock() returns > rtcdev = B; > mutex_unlock(); > > The next call to alarmtimer_rtc_get_device() will return B. Not what > you want. Maybe you want that, but then your patch is missing an > explanation why you want that and why this would be a desired > behaviour. >
Based on current implementation alarmtimer_rtc_add_device() will just be called once thru class interface, also &parent->p->mutex is there to protect the class_intf->add_dev() calling, so we can remove rtcdev_lock. Is it making sense?
Subject: [PATCH] alarmtimer: Removing the usage of lock rtcdev_lock
alarmtimer_rtc_add_device() will be called once at the time alarmtimer_init() is called.
alarmtimer_init --> alarmtimer_rtc_interface_setup --> class_interface_register --> mutex_lock(&parent->p->mutex) ... class_intf->add_dev()/alarmtimer_rtc_add_device ... mutex_unlock(&parent->p->mutex);
But also the mutex parent->p->mutex has protected the add_dev() callback.
So here removing the lock of rtcdev_lock.
Signed-off-by: liu chuansheng <chuansheng.liu@intel.com> --- kernel/time/alarmtimer.c | 13 ++++--------- 1 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c index eed3b26..660091c 100644 --- a/kernel/time/alarmtimer.c +++ b/kernel/time/alarmtimer.c @@ -51,7 +51,6 @@ static struct wakeup_source *ws; /* rtc timer and device for setting alarm wakeups at suspend */ static struct rtc_timer rtctimer; static struct rtc_device *rtcdev; -static DEFINE_SPINLOCK(rtcdev_lock); /** * alarmtimer_get_rtcdev - Return selected rtcdevice @@ -69,7 +68,6 @@ struct rtc_device *alarmtimer_get_rtcdev(void) static int alarmtimer_rtc_add_device(struct device *dev, struct class_interface *class_intf) { - unsigned long flags; struct rtc_device *rtc = to_rtc_device(dev); if (rtcdev) @@ -80,13 +78,10 @@ static int alarmtimer_rtc_add_device(struct device *dev, if (!device_may_wakeup(rtc->dev.parent)) return -1; - spin_lock_irqsave(&rtcdev_lock, flags); - if (!rtcdev) { - rtcdev = rtc; - /* hold a reference so it doesn't go away */ - get_device(dev); - } - spin_unlock_irqrestore(&rtcdev_lock, flags); + rtcdev = rtc; + /* hold a reference so it doesn't go away */ + get_device(dev); + return 0; } -- 1.7.0.4
| |