lkml.org 
[lkml]   [2012]   [Jul]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] alarmtimer: add .remove_dev hook to put device
On 07/18/2012 04:16 PM, John Stultz wrote:
> On 07/04/2012 09:13 AM, Shawn Guo wrote:
>> The following is a test sequence that installs a rtc module, remove it
>> and installs it again.
>>
>> $ insmod rtc-snvs.ko
>> snvs_rtc 20cc034.snvs-rtc-lp: rtc core: registered
>> 20cc034.snvs-rtc-lp as rtc0
>> $ hwclock
>> Thu Jul 5 08:53:35 2012 0.000000 seconds
>> $ rmmod rtc-snvs.ko
>> $ insmod rtc-snvs.ko
>> snvs_rtc 20cc034.snvs-rtc-lp: rtc core: registered
>> 20cc034.snvs-rtc-lp as rtc1
>> $ hwclock
>> hwclock: can't open '/dev/misc/rtc': No such file or directory
>> $
>>
>> The device is registered as rtc0 for the first time insmod, while it
>> becomes rtc1 with the later insmod.
>>
>> It's root caused by alarmtimer which never puts the device even when
>> the rtc is removed. The patch adds a .remove_dev hook to have device
>> properly put, so that above insmod/rmmod sequence can the rtc device
>> registered in a consistent behavior.
>>
>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>> ---
>> kernel/time/alarmtimer.c | 12 ++++++++++++
>> 1 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
>> index aa27d39..39e8773 100644
>> --- a/kernel/time/alarmtimer.c
>> +++ b/kernel/time/alarmtimer.c
>> @@ -96,6 +96,17 @@ static int alarmtimer_rtc_add_device(struct device
>> *dev,
>> return 0;
>> }
>>
>> +static void alarmtimer_rtc_remove_device(struct device *dev,
>> + struct class_interface *class_intf)
>> +{
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&rtcdev_lock, flags);
>> + rtcdev = NULL;
>> + put_device(dev);
>> + spin_unlock_irqrestore(&rtcdev_lock, flags);
>> +}
>> +
>
> Looking at this a little closer, you need to validate that the device
> being removed is the actual rtcdev before setting it to null.
>
> Otherwise if there's two rtc drivers, and the 1st is the current
> rtcdev, you'll clear rtcdev if the second rtc driver is removed.
>
> I'll go ahead and fix this up for you.

Actually, this change opens up a bunch of other races, as any caller of
alarmtimer_get_rtcdev() could have the rtcdevice removed under it.

We'll need to have proper reference counting w/ get/put calls, probably
also adding a alarmtimer_put_rtcdev() interface.

So for now I'm dropping this from my tree. Do you think you might be
able to take another stab at this?

Sorry again for the delayed review here.

thanks
-john



\
 
 \ /
  Last update: 2012-07-19 02:01    [W:0.083 / U:0.988 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site