From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ig0-f177.google.com (mail-ig0-f177.google.com [209.85.213.177]) by dpdk.org (Postfix) with ESMTP id 2DA1B5A56 for ; Thu, 4 Jun 2015 04:29:49 +0200 (CEST) Received: by igbpi8 with SMTP id pi8so127358102igb.1 for ; Wed, 03 Jun 2015 19:29:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=Ksbhi5v3O75KJaOwsKuwd/ahfyfx6s2JsCCEGS1Jup4=; b=yETFLk7K72hP49gH31sB1L5kFE2wAZHEmpw59XVN0XzowzHiN4hs3DWuDFjthHwJfJ Arsj8VwTIEgkiES039P6W0A24VvD85l3rCDtwqxmonUsR23h92mhaogA2W74j5nMe7Gh 93eVjAFva9tEBXEoB7RuK6HgmgfP0e//C+G/xsheGkiZUHfwaeLNtjO9N/EQL80SGkzB 5iB8tFTXNKiDL5YnRhh4rVzBsO8Z0B0r3GaURABrVIC1ohM3yeQiRLOo/ctj5m5qTjU5 Arzm1ZfzA8oLX25Q1bNn199hWK4jMZHtYOI1hFixDeb6Kji9MZ2r8JhxyaWciaG+0WaD Mo4Q== MIME-Version: 1.0 X-Received: by 10.107.10.151 with SMTP id 23mr10466558iok.89.1433384988527; Wed, 03 Jun 2015 19:29:48 -0700 (PDT) Received: by 10.79.112.140 with HTTP; Wed, 3 Jun 2015 19:29:48 -0700 (PDT) In-Reply-To: References: <20150603125410.GA5880@bricha3-MOBL3> Date: Thu, 4 Jun 2015 10:29:48 +0800 Message-ID: From: Selmon Yang To: Jay Rolette Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: DPDK Subject: Re: [dpdk-dev] rte_eal_alarm_set() is affected by discontinuous jumps in the system time X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Jun 2015 02:29:49 -0000 Hi, Bruce, Jay, I am sorry that the patch in last mail is the wrong one, please replace it with the attached one in this mail. 2015-06-04 10:09 GMT+08:00 Selmon Yang : > Hi, Bruce, > > Thank you very much for your positive response. > The attachment is the patch. > Please have a look. > > Hi, Jay, > > Thank you so much for your kindly reminding. > However, due to I really like the alarm functionalities not to be affected. > I guess I need to replace CLOCK_MONOTONIC_RAW with CLOCK_MONOTONIC > and do more tests to see if it also works for me. > > > 2015-06-03 21:07 GMT+08:00 Jay Rolette : >> >> On Wed, Jun 3, 2015 at 7:54 AM, Bruce Richardson >> wrote: >>> >>> On Wed, Jun 03, 2015 at 02:31:47PM +0800, Selmon Yang wrote: >>> > Hi, >>> > >>> > I found that, in dpdk 2.0, rte_eal_alarm_set() is affected by >>> > discontinuous jumps in the system time because eal_alarm_callback() >>> > and rte_eal_alarm_set() use gettimeofday() to get the current time. >>> > >>> > Here is what I encountered. >>> > I set up a rte eal alarm as below, and I like it to be triggered every >>> > second. >>> > #define USE_PER_S 1000 * 1000 >>> > void my_alarm_cb(void *arg) >>> > { >>> > /* send heartbeat signal out, etc. */ >>> > >>> > rte_eal_alarm_set(1 * US_PER_S, my_alarm_cb, NULL); >>> > return; >>> > } >>> > >>> > int main(void) >>> > { >>> > /* ..., do something */ >>> > rte_eal_alarm_set(1 * US_PER_S, my_alarm_cb, NULL); >>> > /* ... do something else */ >>> > } >>> > >>> > It works fine in most of time. >>> > However, if I change system time manually, it is possible that rte alarm >>> > function works out of my expectation. >>> > Suppose that current time is 11:00:00 AM, and eal_alarm_callback() >>> > is triggered because I executed >>> > rte_eal_alarm_set(1 * US_PER_S, my_alarm_cb, NULL) at 10:59:59 AM. >>> > eal_alarm_callback() gets the current time (11:00:00 AM) >>> > and calls my_alarm_cb() as below. >>> > while ((ap = LIST_FIRST(&alarm_list)) !=NULL && >>> > gettimeofday(&now, NULL) == 0 && >>> > (ap->time.tv_sec < now.tv_sec || >>> > (ap->time.tv_sec == now.tv_sec && >>> > ap->time.tv_usec <= >>> > now.tv_usec))){ >>> > ap->executing = 1; >>> > ap->executing_id = pthread_self(); >>> > rte_spinlock_unlock(&alarm_list_lk); >>> > >>> > ap->cb_fn(ap->cb_arg); >>> > >>> > rte_spinlock_lock(&alarm_list_lk); >>> > >>> > LIST_REMOVE(ap, next); >>> > rte_free(ap); >>> > } >>> > >>> > In my_alarm_cb(), rte_eal_alarm_set() is called again. >>> > rte_eall_alarm_set() gets the current time (11:00:00 AM), plus 1 second, >>> > and adds the new alarm entry to alarm_list. >>> > /* use current time to calculate absolute time of alarm */ >>> > gettimeofday(&now, NULL); >>> > >>> > new_alarm->cb_fn = cb_fn; >>> > new_alarm->cb_arg = cb_arg; >>> > new_alarm->time.tv_usec = (now.tv_usec + us) % US_PER_S; >>> > new_alarm->time.tv_sec = now.tv_sec + ((now.tv_usec + us) / >>> > US_PER_S); >>> > >>> > rte_spinlock_lock(&alarm_list_lk); >>> > if (!handler_registered) { >>> > ret |= rte_intr_callback_register(&intr_handle, >>> > eal_alarm_callback, NULL); >>> > handler_registered = (ret == 0) ? 1 : 0; >>> > } >>> > >>> > if (LIST_EMPTY(&alarm_list)) >>> > LIST_INSERT_HEAD(&alarm_list, new_alarm, next); >>> > else { >>> > LIST_FOREACH(ap, &alarm_list, next) { >>> > if (ap->time.tv_sec > new_alarm->time.tv_sec || >>> > (ap->time.tv_sec == >>> > new_alarm->time.tv_sec && >>> > >>> > ap->time.tv_usec > new_alarm->time.tv_usec)){ >>> > LIST_INSERT_BEFORE(ap, new_alarm, next); >>> > break; >>> > } >>> > if (LIST_NEXT(ap, next) == NULL) { >>> > LIST_INSERT_AFTER(ap, new_alarm, next); >>> > break; >>> > } >>> > } >>> > } >>> > >>> > After the new alarm entry is added to alarm_list, if current time is >>> > set to 8:00:00 AM manually, the current time in eal_alarm_callback() >>> > will be updated to 8:00:00 AM, too. >>> > Then the new alarm entry will be triggered after 3 hours and 1 second. >>> > >>> > I think rte alarm should not be affected by discontinuous jumps in >>> > the system time. >>> > I tried to replace gettimeofday() with >>> > clock_gettime(CLOCK_MONOTONIC_RAW, &now), >>> > and it looks work fine. >>> > What do you think about this modification? >>> > Will you consider to modify rte_alarm functions to be not affected >>> > by discontinuous jumps in the system time? >>> >>> I agree with you that the alarm functionality should not be affected by >>> jumps >>> in system time. If you have a patch that fixes this bug, it would be great >>> if >>> you could upstream it here. >>> >>> Thanks, >>> /Bruce >> >> >> I haven't looked through the RTE alarm code, but one thing to consider is >> whether you want to use CLOCK_MONOTONIC_RAW or just CLOCK_MONOTONIC. The RAW >> version is ~10x slower than the CLOCK_MONOTONIC. >> >> CLOCK_MONOTONIC isn't completely protected from NTP frequency adjustments, >> but it won't have discontinuities. >> >> We've found the rte_eal_alarm calls to be surprisingly (ie., we hadn't >> bothered to look at the implementation yet) variable and intermittently >> slow, so the 10x difference on the call to clock_gettime() may be relevant. >> >> Jay