From: Ning Yu <ning.yu@canonical.com>
To: Ian Kent <raven@themaw.net>
Cc: autofs <autofs@vger.kernel.org>, Yu Ning <ning.yu@ubuntu.com>
Subject: Re: [PATCH v3 2/3] autofs-5.1.1 - use monotonic clock for pthread cond timed wait.
Date: Fri, 18 Sep 2015 15:20:06 +0800 [thread overview]
Message-ID: <CAA1QoHSCj0h4bmx7d5wS+WQqC73p572iVv_p23TNihG5kVnQ2w@mail.gmail.com> (raw)
In-Reply-To: <1442540212.2947.43.camel@themaw.net>
On Fri, Sep 18, 2015 at 9:36 AM, Ian Kent <raven@themaw.net> wrote:
> On Thu, 2015-09-17 at 17:12 +0800, Ning Yu wrote:
>> On Thu, Sep 17, 2015 at 4:39 PM, Ian Kent <raven@themaw.net> 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@themaw.net> 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@ubuntu.com>
>> >> >> ---
>> >> >> 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?
>
> Sure, send an updated patch, I'm not able to start work on it quite yet.
>
v5 patches are sent to you, please check for v5-0002 for the updates.
>>
>> >>
>> >> >> +
>> >> >> if (pattrs)
>> >> >> pthread_attr_destroy(pattrs);
>> >> >>
>> >> >
>> >> >
>> >
>> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
next prev parent reply other threads:[~2015-09-18 7:20 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-16 8:29 Mount point not auto unmounted after system date/time change Ning Yu
2015-09-16 8:45 ` Ning Yu
2015-09-16 9:16 ` Ian Kent
2015-09-16 10:28 ` Ning Yu
2015-09-16 10:43 ` Ian Kent
2015-09-16 11:02 ` Ning Yu
2015-09-16 11:21 ` Ian Kent
2015-09-17 3:48 ` [PATCH v3 1/3] autofs-5.1.1 - use clock_gettime() instead of gettimeofday() Yu Ning
2015-09-17 3:48 ` [PATCH v3 2/3] autofs-5.1.1 - use monotonic clock for pthread cond timed wait Yu Ning
2015-09-17 6:49 ` Ian Kent
2015-09-17 7:19 ` Ning Yu
2015-09-17 8:39 ` Ian Kent
2015-09-17 9:12 ` Ning Yu
2015-09-18 1:36 ` Ian Kent
2015-09-18 7:16 ` [PATCH v5 1/3] autofs-5.1.1 - use clock_gettime() instead of gettimeofday() Yu Ning
2015-09-18 7:16 ` [PATCH v5 2/3] autofs-5.1.1 - use monotonic clock for pthread cond timed wait Yu Ning
2015-09-18 7:16 ` [PATCH v5 3/3] autofs-5.1.1 - use monotonic clock instead of time() Yu Ning
2015-09-18 7:20 ` Ning Yu [this message]
2015-09-17 3:48 ` [PATCH v3 " Yu Ning
2015-09-17 7:04 ` Ian Kent
2015-09-17 7:16 ` Ian Kent
2015-09-17 7:34 ` Ning Yu
2015-09-17 4:02 ` Mount point not auto unmounted after system date/time change Ning Yu
2015-09-17 4:09 ` [PATCH v4 1/3] autofs-5.1.1 - use clock_gettime() instead of gettimeofday() Yu Ning
2015-09-17 4:09 ` [PATCH v4 2/3] autofs-5.1.1 - use monotonic clock for pthread cond timed wait Yu Ning
2015-09-17 4:09 ` [PATCH v4 3/3] autofs-5.1.1 - use monotonic clock instead of time() Yu Ning
2015-09-17 4:17 ` Mount point not auto unmounted after system date/time change Ning Yu
2015-09-17 6:07 ` Ian Kent
2015-09-17 6:05 ` Ian Kent
2015-09-16 10:30 ` Ning Yu
2015-09-16 10:30 ` Ning Yu
2015-09-16 10:30 ` Ning Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAA1QoHSCj0h4bmx7d5wS+WQqC73p572iVv_p23TNihG5kVnQ2w@mail.gmail.com \
--to=ning.yu@canonical.com \
--cc=autofs@vger.kernel.org \
--cc=ning.yu@ubuntu.com \
--cc=raven@themaw.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).