From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qg0-f53.google.com (mail-qg0-f53.google.com [209.85.192.53]) by dpdk.org (Postfix) with ESMTP id D3DE811D4 for ; Thu, 9 Jun 2016 17:01:48 +0200 (CEST) Received: by mail-qg0-f53.google.com with SMTP id v48so3177175qgd.2 for ; Thu, 09 Jun 2016 08:01:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=SymFjg/mGxkhcEqFpTO0/jUrrMOfgMtW3DigTWmw2Po=; b=G/yNgZFAlT6Qf2QpWz3YZitNq9WTLouAxD5i7LmMrp2riWcFKXyBhnQSlm3Jn5bnfm Ou7o01Tb67/NW2Khu2cxRSyAkhiY36MB4j9Mev3RF62Xsc6VgW2J0bIEh26Xe5Xe8/EK YbadZm3fh+rtcQQWRp3QvYSq9xFG1i34u4unstZgBOOkQPquoWFLcIUe0V1VcR2b2e6A ELhSX9kxXS1IDbSaZcolAlQOEG3x6JmqdC9vmTT5W9zKvoMM8u18bARzQfCUvV0JYscL /ged/nrJQljp9Gdckn3qQwrA9EXqBhue7dGF4M+Nmq9QEXgwBtWmap4tA4M+fKYQR9P6 M72Q== 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=SymFjg/mGxkhcEqFpTO0/jUrrMOfgMtW3DigTWmw2Po=; b=S2gkaIAtl31mJYE9MjW5W5Wo+HW4f+85pMDOEEicloBWhMY54w0aqz/eDObh/tkvaA OkNbbw6HDwpQr58inBqOFrTrGtYPIsqtxE6UDcdknjq6ertU1l5c5R4T7l2naldivtUG g5PlcNXYtSAvD+bRr1O16NyWsWpNYIJAvYdZpTsv49OQ2NCxtuJzzjjWy59Q+Z6+ar1p IoFJ0SSd75OPqyOVC5kQfPrYjdJBL7gSQ/bkAFUdim9cOw+V8kloaDf9z0mSrXLl02O2 oDLuU+/IXh+Z12kMbCSGa8lQ81uafo/udKvoBuE3Tu/KFNQp48fRmCy28LeMITy8IWK+ 6vng== X-Gm-Message-State: ALyK8tJpILB9RC4j1lJbWP7//qtfYmwEzJA6mLZYAy8oFzEM+2EDDzHbXz/mWYT9EJm0wsWDhq/fjFbfMEGwDEXJ X-Received: by 10.140.40.239 with SMTP id x102mr1727974qgx.93.1465484508174; Thu, 09 Jun 2016 08:01:48 -0700 (PDT) MIME-Version: 1.0 Received: by 10.55.162.9 with HTTP; Thu, 9 Jun 2016 08:01:28 -0700 (PDT) In-Reply-To: References: <1465481396-23968-1-git-send-email-thomas.monjalon@6wind.com> From: Christian Ehrhardt Date: Thu, 9 Jun 2016 17:01:28 +0200 Message-ID: To: David Marchand Cc: Thomas Monjalon , "dev@dpdk.org" Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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 15:01:49 -0000 Hi, in I totally like it - thanks Thomas for picking that up. I just wanted to mention that the Makefile still refers to mempool, but David beat me in time and Detail a lot. I'll certainly try to follow and help the bit I can. Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd On Thu, Jun 9, 2016 at 4:45 PM, David Marchand wrote: > 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 >