From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f50.google.com (mail-pa0-f50.google.com [209.85.220.50]) by dpdk.org (Postfix) with ESMTP id 44BDF688E for ; Wed, 11 Nov 2015 17:27:52 +0100 (CET) Received: by padhx2 with SMTP id hx2so35107535pad.1 for ; Wed, 11 Nov 2015 08:27:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber_org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=z5TPeZBNdkBZlfBXUMXEqLLjSWnn5toAgFwH42I+DeU=; b=GZoEhFCRFAhWBEm4nvPKcxUhPIwENCfPboMCA87uIoA1lOfqK52za9+A9jlzzKVea3 +uJY3cR0Me209zOPyINmTbinjYWRCuDAxZH0vwNdkE30cfZfHAj8psdYWvLWXi7xoyBn 8iFsmekvb/i2YzIbmzZR+mNRl3y5bXVr6ilrfomVqgXLjTB50FkWXDoIdF+WAYc9DeHw Csn9ZdpcKUcZYWjy2c/BCAkGUCys9vAe9KnwaaF0lUI658tc5XyXdwOEr4IBYsfR9hPA 3NS6uhw2AmS2BP8mRgKDsrgiOsPjVETEA8SNNwenwDWnKlgAQjZrshCvw7vIcd79tKaK E9HA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=z5TPeZBNdkBZlfBXUMXEqLLjSWnn5toAgFwH42I+DeU=; b=jWtWOQ1GDwjnX8JAK66MT0aJxZYz1iSvj1FhcqBQxw/jQhPoYyx6wqvqCTHmgJgfoz dQDpUw0B4cd9NOOMJ4Wjb/LX4D87YKbwNURWQ5grNMszywFptKtKut1b/SqWMomEQjtq RR0Jot6KD85r8m0gZ5cP1TzmKKKWpfWsOLl53lAIJIT/WMqiGHRNPJrrfky6YA7l1mJo yK2uB0X0Y1nmqSgiW8SbT+U6nbFCPLeI3+o8fLooLl+SdBnsXUyN/28ntbF1sOV5cUPD 4dMMcQT73ANdJxppVXCVkj0pDAZDLPAnsyHz99pzzVBb0EfgYzuzC47LLIzmyESQFYWK /BCg== X-Gm-Message-State: ALoCoQl+B3ZmGDQ40Ru08VYtpA6h9uZbnUGU2jHJs3kJmrsnVBBbKx3jfWPdICXaGEIT+BaMhWPq X-Received: by 10.66.161.35 with SMTP id xp3mr15851435pab.13.1447259271499; Wed, 11 Nov 2015 08:27:51 -0800 (PST) Received: from xeon-e3 (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by smtp.gmail.com with ESMTPSA id rs5sm9617520pbb.61.2015.11.11.08.27.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 11 Nov 2015 08:27:51 -0800 (PST) Date: Wed, 11 Nov 2015 08:28:01 -0800 From: Stephen Hemminger To: Remy Horton Message-ID: <20151111082801.3eedaf21@xeon-e3> In-Reply-To: <1446723178-14876-2-git-send-email-remy.horton@intel.com> References: <1446723178-14876-1-git-send-email-remy.horton@intel.com> <1446723178-14876-2-git-send-email-remy.horton@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v4 1/3] rte: add keep alive functionality 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: Wed, 11 Nov 2015 16:27:52 -0000 In general, try not to introduce new thinks and avoid extra code. Also Linux has more robust mechanism in the watchdog timers, why not use that? Could you use RTE_PER_LCORE some how? > +#ifdef KEEPALIVE_DEBUG_MSGS Any #ifdef must have a config option to enable it. > +static void > +print_trace(const char *msg, struct rte_keepalive *keepcfg, int idx_core) > +{ > + printf("%sLast seen %" PRId64 "ms ago.\n", > + msg, > + ((rte_rdtsc() - keepcfg->last_alive[idx_core])*1000) > + / rte_get_tsc_hz() > + ); > +} > +#else > +static void > +print_trace(__attribute__((unused)) const char *msg, > + __attribute__((unused)) struct rte_keepalive *keepcfg, > + __attribute__((unused)) int idx_core) > +{ > +} > +#endif Agree with others don't introduce not logging macros. > +void > +rte_keepalive_dispatch_pings(__attribute__((unused)) void *ptr_timer, Use __rte_unused > + void *ptr_data) > +{ > + struct rte_keepalive *keepcfg = (struct rte_keepalive *)ptr_data; Cast of void * is unnecessary in C. > + int idx_core; > + > + for (idx_core = 0; idx_core < RTE_KEEPALIVE_MAXCORES; idx_core++) { > + if (keepcfg->active_cores[idx_core] == 0) > + continue; > + switch (keepcfg->state_flags[idx_core]) { My personal preference, prefer blank line after continue. > + case ALIVE: /* Alive */ > + keepcfg->state_flags[idx_core] = 0; > + keepcfg->last_alive[idx_core] = rte_rdtsc(); > + break; > + case MISSING: /* MIA */ > + print_trace("Core MIA. ", keepcfg, idx_core); > + keepcfg->state_flags[idx_core] = 2; > + break; > + case DEAD: /* Dead */ > + keepcfg->state_flags[idx_core] = 3; > + print_trace("Core died. ", keepcfg, idx_core); > + if (keepcfg->callback) > + keepcfg->callback( > + keepcfg->callback_data, > + idx_core > + ); > + break; > + case GONE: /* Buried */ > + break; > + } > + } > +} > + > + > +struct rte_keepalive * > +rte_keepalive_create(rte_keepalive_failure_callback_t callback, > + void *data) > +{ > + int idx_core; > + struct rte_keepalive *keepcfg; > + > + keepcfg = malloc(sizeof(struct rte_keepalive)); Why not use rte_zmalloc()? > + if (keepcfg != NULL) { > + for (idx_core = 0; > + idx_core < RTE_KEEPALIVE_MAXCORES; > + idx_core++) { > + keepcfg->state_flags[idx_core] = 0; > + keepcfg->active_cores[idx_core] = 0; > + } > + keepcfg->callback = callback; > + keepcfg->callback_data = data; > + keepcfg->tsc_initial = rte_rdtsc(); > + keepcfg->tsc_mhz = rte_get_tsc_hz() / 1000; > + } > + return keepcfg; > +} > + > + > +void > +rte_keepalive_register_core(struct rte_keepalive *keepcfg, const int id_core) > +{ > + if (id_core < RTE_KEEPALIVE_MAXCORES) > + keepcfg->active_cores[id_core] = 1; > +}