From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47]) by dpdk.org (Postfix) with ESMTP id 98191DE3 for ; Thu, 9 Jun 2016 16:46:04 +0200 (CEST) Received: by mail-wm0-f47.google.com with SMTP id v199so110899168wmv.0 for ; Thu, 09 Jun 2016 07:46:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=lZY9SDHM/jAxyMjwTbu6Joryv7JeJMoRkJcBMj9+fNs=; b=zXacrmH4nmM0InMqcusVX8NAhfdHueVY7qsgx1nXvFUyb7J7YNidpL9LtRui8qGi6/ MwdyPDpAVNc/GZHzkeiwQOkUZkhBk1K/EoAQzNRe9abBeXkRdcbNjf+y2cU9oz6eSWqa U/nx2kaiR1Gf4IZi4HoKuXmW3E0/8b8El1+YPBqcVio1PmAXmmN7gRlSmvfDiXKmc1ow 8elylZCAEnvXu0CTrb/CNzDv+rztiTyHB27kGU3SmpkZ8YAcYJ/NewwdPVri4vOs6bWW jNEqZMctK/UH0fbLmVoBYu9TSO+QjbUImvuAj19N150/N6g0JGelXdyKBLNQFdOni54S CPLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=lZY9SDHM/jAxyMjwTbu6Joryv7JeJMoRkJcBMj9+fNs=; b=NY8Mpm2LF0GlnwWhRf1foLfpxRii8hornqJQjeqxbYN/DZanoHu4CMFOrYGG/LSHW0 ijHkwX22gJUxSM3AAUHqR0wk/Sikj3Yq2HW5jXp46KR8B/3V+ZxK3gsQ2eSgE8E0dAJ7 d1u70OADsDlfshotNrrjQnf5XdJFqlZxQOJ7mcvt5+BZ6VBduXIVU31KRzcghhoSz7I2 FUxURMW/LXKz1Z9LtCO9/E7ku3kesKNG1Z9doMeYe9NO9yGkChxBiOcK7JGBRtIZtAFI niKPHmfHF+T4gltDqunZKOcBLyx6Q2lD9WmCkgocG8LwAcWFCK8b0cDKjwOpwgEBYMof kMbw== X-Gm-Message-State: ALyK8tKArzidgkR7grWMqUuEMP56wsFxo996/r8U5MgsmKCJrghpI0o5hHMyhJVsN+y3LsqziI/cZ564Bowsw21N X-Received: by 10.194.228.102 with SMTP id sh6mr10977090wjc.173.1465483564190; Thu, 09 Jun 2016 07:46:04 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.16.13 with HTTP; Thu, 9 Jun 2016 07:45:44 -0700 (PDT) In-Reply-To: <1465481396-23968-1-git-send-email-thomas.monjalon@6wind.com> References: <1465481396-23968-1-git-send-email-thomas.monjalon@6wind.com> From: David Marchand Date: Thu, 9 Jun 2016 16:45:44 +0200 Message-ID: To: Thomas Monjalon Cc: "dev@dpdk.org" Content-Type: text/plain; charset=UTF-8 Subject: Re: [dpdk-dev] [PATCH] log: deprecate history dump 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, 09 Jun 2016 14:46:04 -0000 Thomas, On Thu, Jun 9, 2016 at 4:09 PM, Thomas Monjalon wrote: > The log history uses rte_mempool. In order to remove the mempool > dependency in EAL (and improve the build), this feature is deprecated. > The ABI is kept but the behaviour is now voided because it seems this > function was not used. The history can be read from syslog. It does look like it is not really used. I am for this change unless someone complains. Comments below. > Signed-off-by: Thomas Monjalon > --- > app/test-pmd/cmdline.c | 3 - > app/test/autotest_test_funcs.py | 5 -- > app/test/commands.c | 4 +- > app/test/test_logs.c | 3 - > doc/guides/rel_notes/deprecation.rst | 3 + > lib/librte_eal/bsdapp/eal/eal_debug.c | 6 -- > lib/librte_eal/common/eal_common_log.c | 128 +-------------------------- > lib/librte_eal/common/include/rte_log.h | 8 ++ > lib/librte_eal/linuxapp/eal/eal_debug.c | 6 -- > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 1 - > lib/librte_eal/linuxapp/eal/eal_ivshmem.c | 1 - > lib/librte_eal/linuxapp/eal/eal_log.c | 3 - > lib/librte_mempool/rte_mempool.c | 4 - > 13 files changed, 16 insertions(+), 159 deletions(-) - You missed autotest_data.py. $ git grep dump_log_history app/test/autotest_data.py: "Command" : "dump_log_history", - eal Makefile still refers to librte_mempool. - Since you are looking at this, what keeps us from removing the dependency on librte_ring ? I would say it was mainly because of mempool. Maybe ivshmem ? [snip] > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst > index ad05eba..f11df93 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -8,6 +8,9 @@ API and ABI deprecation notices are to be posted here. > Deprecation Notices > ------------------- > > +* The log history is deprecated in release 16.07. > + It is voided and will be completely removed in release 16.11. > + It is voided in 16.07 and will be completely removed in release 16.11. > * The ethdev hotplug API is going to be moved to EAL with a notification > mechanism added to crypto and ethdev libraries so that hotplug is now > available to both of them. This API will be stripped of the device arguments > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c > index b5e37bb..94ecdd2 100644 > --- a/lib/librte_eal/common/eal_common_log.c > +++ b/lib/librte_eal/common/eal_common_log.c > @@ -56,29 +56,11 @@ > #include > #include > #include > -#include > > #include "eal_private.h" > > #define LOG_ELT_SIZE 2048 We don't need LOG_ELT_SIZE. > > -#define LOG_HISTORY_MP_NAME "log_history" > - > -STAILQ_HEAD(log_history_list, log_history); > - > -/** > - * The structure of a message log in the log history. > - */ > -struct log_history { > - STAILQ_ENTRY(log_history) next; > - unsigned size; > - char buf[0]; > -}; > - > -static struct rte_mempool *log_history_mp = NULL; > -static unsigned log_history_size = 0; > -static struct log_history_list log_history; > - > /* global log structure */ > struct rte_logs rte_logs = { > .type = ~0, > @@ -86,10 +68,7 @@ struct rte_logs rte_logs = { > .file = NULL, > }; > > -static rte_spinlock_t log_dump_lock = RTE_SPINLOCK_INITIALIZER; > -static rte_spinlock_t log_list_lock = RTE_SPINLOCK_INITIALIZER; > static FILE *default_log_stream; > -static int history_enabled = 1; > > /** > * This global structure stores some informations about the message > @@ -106,59 +85,14 @@ static RTE_DEFINE_PER_LCORE(struct log_cur_msg, log_cur_msg); > /* default logs */ > > int > -rte_log_add_in_history(const char *buf, size_t size) > +rte_log_add_in_history(const char *buf __rte_unused, size_t size __rte_unused) > { > - struct log_history *hist_buf = NULL; > - static const unsigned hist_buf_size = LOG_ELT_SIZE - sizeof(*hist_buf); > - void *obj; > - > - if (history_enabled == 0) > - return 0; > - > - rte_spinlock_lock(&log_list_lock); > - > - /* get a buffer for adding in history */ > - if (log_history_size > RTE_LOG_HISTORY) { > - hist_buf = STAILQ_FIRST(&log_history); > - if (hist_buf) { > - STAILQ_REMOVE_HEAD(&log_history, next); > - log_history_size--; > - } > - } > - else { > - if (rte_mempool_mc_get(log_history_mp, &obj) < 0) > - obj = NULL; > - hist_buf = obj; > - } > - > - /* no buffer */ > - if (hist_buf == NULL) { > - rte_spinlock_unlock(&log_list_lock); > - return -ENOBUFS; > - } > - > - /* not enough room for msg, buffer go back in mempool */ > - if (size >= hist_buf_size) { > - rte_mempool_mp_put(log_history_mp, hist_buf); > - rte_spinlock_unlock(&log_list_lock); > - return -ENOBUFS; > - } > - > - /* add in history */ > - memcpy(hist_buf->buf, buf, size); > - hist_buf->buf[size] = hist_buf->buf[hist_buf_size-1] = '\0'; > - hist_buf->size = size; > - STAILQ_INSERT_TAIL(&log_history, hist_buf, next); > - log_history_size++; > - rte_spinlock_unlock(&log_list_lock); > - > return 0; > } > > void > -rte_log_set_history(int enable) > +rte_log_set_history(int enable __rte_unused) > { > - history_enabled = enable; > } Maybe a warning here for the people who did not read the deprecation notices ? -- David Marchand