On Thu, Sep 17, 2015 at 4:39 PM, Ian Kent <raven@xxxxxxxxxx> wrote:
> On Thu, 2015-09-17 at 15:19 +0800, Ning Yu wrote:
>> On Thu, Sep 17, 2015 at 2:49 PM, Ian Kent <raven@xxxxxxxxxx> wrote:
>> > On Thu, 2015-09-17 at 11:48 +0800, Yu Ning wrote:
>> >> The default PTHREAD_COND_INITIALIZER initializer uses realtime clock,
>> >> we need to switch to use the monotic clock.
>> >>
>> >> Signed-off-by: Yu Ning <ning.yu@xxxxxxxxxx>
>> >> ---
>> >> lib/alarm.c | 13 ++++++++++++-
>> >> 1 file changed, 12 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/lib/alarm.c b/lib/alarm.c
>> >> index 65a80ae..5b98b2d 100755
>> >> --- a/lib/alarm.c
>> >> +++ b/lib/alarm.c
>> >> @@ -23,7 +23,7 @@ struct alarm {
>> >> };
>> >>
>> >> static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
>> >> -static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
>> >> +static pthread_cond_t cond;
>> >> static LIST_HEAD(alarms);
>> >>
>> >> #define alarm_lock() \
>> >> @@ -212,6 +212,7 @@ int alarm_start_handler(void)
>> >> pthread_t thid;
>> >> pthread_attr_t attrs;
>> >> pthread_attr_t *pattrs = &attrs;
>> >> + pthread_condattr_t condattrs;
>> >> int status;
>> >>
>> >> status = pthread_attr_init(pattrs);
>> >> @@ -224,8 +225,18 @@ int alarm_start_handler(void)
>> >> #endif
>> >> }
>> >>
>> >> + status = pthread_condattr_init(&condattrs);
>> >> + if (status)
>> >> + fatal(status);
>> >> +
>> >> + pthread_condattr_setclock(&condattrs, CLOCK_MONOTONIC);
>> >> + pthread_cond_init(&cond, &condattrs);
>> >> +
>> >> status = pthread_create(&thid, pattrs, alarm_handler, NULL);
>> >>
>> >> + pthread_condattr_destroy(&condattrs);
>> >> + pthread_condattr_destroy(&cond);
>> >
>> > I don't think we can do this.
>> >
>> > The condition is used by the alarm_handler() for the duration of it's
>> > life so I don't think we can't destroy its attributes after thread
>> > creation.
>> >
>> > In fact I'm not sure we even need the pthread_condattr_destroy(&cond)
>> > but I'm not sure.
>> >
>> > Destroying the condition attributes (condattrs) OTOH must not affect any
>> > condition which it has been used to initialize so destroying that is
>> > fine.
>> >
>> > Let me have a closer look at this, maybe I've misunderstood what's going
>> > on, and report back later.
>> >
>>
>> I think you are right, we shouldn't destroy the cond attrs, nor the cond
>> (and I used wrong function to destroy cond...) if the thread is successfully
>> created.
>>
>> In fact I think we should do the cleanup at the end of the thread, however
>> since the thread is expected to live __forever__, maybe we can simply ignore
>> the cleanup.
>
> That's what I'm thinking too.
>
> I don't think there is any problem assuming the monotonic clock is
> available (I think it's been around a very long time) so there's no
> point in complicating things to cater for that case.
>
> Although I am thinking of checking the return and calling the fatal()
> function on failure as I do for other pthreads functions that should
> never fail, so if it does fail we'll get a simple message and a core so
> we know where to look.
>
Yes, those messages will definitely be very useful for debugging, do
you mind that I update my patch to add those checking and messages? Or
will it be better to submit a new patch for that job?
>>
>> >> +
>> >> if (pattrs)
>> >> pthread_attr_destroy(pattrs);
>> >>
>> >
>> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
