From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f171.google.com (mail-wi0-f171.google.com [209.85.212.171]) by dpdk.org (Postfix) with ESMTP id F15845A8A for ; Tue, 14 Apr 2015 14:58:09 +0200 (CEST) Received: by wiax7 with SMTP id x7so87937058wia.0 for ; Tue, 14 Apr 2015 05:58:09 -0700 (PDT) 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:date :message-id:subject:from:to:cc:content-type; bh=aRHlW+8JgxcYp7obZ8xMJp4+ezSo55kfeMkJj1lY6g0=; b=IsLUp/GyPAm7lgqI2jPWSj5moM8v+vS2Uh74+BH5+mWo1rDXyL5U82ukXyfoXfP3fi fTuiQ/JyqDWKq8te80Wb4ns6qEjTOYXGoe2P7Y+RkMxf8wTcHp+Loii8meLzue5t39cF 9sZnQT8hSP7VFs+ZKZT4L4LWwWve4mcLD+wF+zimYoUj1+GtoRzjgWR2mMlPSTGdCHbm 9pCyBoBSoWxIhwVczCCjoHW9y2wMZBrH4nDKvIwjfSkqS6A/uFDwAoKvV6UKMIBDFuMH VCEoZxPZp2PMyLjqHphFSKj4FfLyyw0BeQR0q3A+88Mx6IL6nsNvKXEeOpUmUXvZGLx6 attQ== X-Gm-Message-State: ALoCoQkvDgGQLi6dhf3PNSm1Vo+5Ttl2AxHxVd4hmhq6E1dEvNFBo1JYOn1orOAKkofkhJAmhwBj MIME-Version: 1.0 X-Received: by 10.180.96.196 with SMTP id du4mr31775088wib.77.1429016289829; Tue, 14 Apr 2015 05:58:09 -0700 (PDT) Received: by 10.194.153.130 with HTTP; Tue, 14 Apr 2015 05:58:09 -0700 (PDT) In-Reply-To: <20150413180958.506cbac6@urahara> References: <1418824622-9212-1-git-send-email-rolette@infiniteio.com> <20150413180958.506cbac6@urahara> Date: Tue, 14 Apr 2015 07:58:09 -0500 Message-ID: From: Jay Rolette To: Stephen Hemminger Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: DPDK Subject: Re: [dpdk-dev] [PATCH] Fixed spam from kni_allocate_mbufs() when no mbufs are free. If mbufs exhausted, 'out of memory' message logged at EXTREMELY high rates. Now logs no more than once per 10 mins 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: Tue, 14 Apr 2015 12:58:10 -0000 Hi Stephen, Thanks for the feedback. Comments and questions inline below. Jay On Mon, Apr 13, 2015 at 8:09 PM, Stephen Hemminger < stephen@networkplumber.org> wrote: > On Wed, 17 Dec 2014 07:57:02 -0600 > Jay Rolette wrote: > > > Signed-off-by: Jay Rolette > > --- > > lib/librte_kni/rte_kni.c | 21 ++++++++++++++++++++- > > 1 file changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c > > index fdb7509..f89319c 100644 > > --- a/lib/librte_kni/rte_kni.c > > +++ b/lib/librte_kni/rte_kni.c > > @@ -40,6 +40,7 @@ > > #include > > #include > > > > +#include > > #include > > #include > > #include > > @@ -61,6 +62,9 @@ > > > > #define KNI_MEM_CHECK(cond) do { if (cond) goto kni_fail; } while (0) > > > > +// Configure how often we log "out of memory" messages (in seconds) > > +#define KNI_SPAM_SUPPRESSION_PERIOD 60*10 > > + > > /** > > * KNI context > > */ > > @@ -592,6 +596,10 @@ kni_free_mbufs(struct rte_kni *kni) > > static void > > kni_allocate_mbufs(struct rte_kni *kni) > > { > > + static uint64_t no_mbufs = 0; > > + static uint64_t spam_filter = 0; > > + static uint64_t delayPeriod = 0; > > + > > int i, ret; > > struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM]; > > > > @@ -620,7 +628,18 @@ kni_allocate_mbufs(struct rte_kni *kni) > > pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool); > > if (unlikely(pkts[i] == NULL)) { > > /* Out of memory */ > > - RTE_LOG(ERR, KNI, "Out of memory\n"); > > + no_mbufs++; > > + > > + // Memory leak or need to tune? Regardless, if we > get here once, > > + // we will get here a *lot*. Don't spam the logs! > > + now = rte_get_tsc_cycles(); > > + if (!delayPeriod) > > + delayPeriod = rte_get_tsc_hz() * > KNI_SPAM_SUPPRESSION_PERIOD; > > + > > + if (!spam_filter || (now - spam_filter) > > delayPeriod) { > > + RTE_LOG(ERR, KNI, "No mbufs available > (%llu)\n", (unsigned long long)no_mbufs); > > + spam_filter = now; > > + } > > break; > > } > > } > > I agree whole completely with the intent of this. > But just remove the log message completely. It doesn't > help at all, use a statistic instead. > I'm fine with removing the log message completely. Can you point me to where DPDK keeps stats generally? Stats like this are only useful if they are accessible from some sort of monitoring process. There aren't any stats in struct rte_kni right now. If you want to do ratelimiting, then it is better to create > a common function (see net_ratelimit() in Linux kernel) to > have all code use the same method, rather than reinventing private > code to do it. > I'll remove it. > Minor style complaints: > * don't use camelCase, it goes against the style of the rest of the code. > ok > * don't use C++ style comments. > I didn't. I used C99 style comments :) > * always use rte_cycles() not TSC for things like this. > I don't see rte_cycles() defined anywhere. Did you mean some other function? Please resubmit removing the log message and adding a statistic. >