| Message ID | 4685487.eCM9t2Cvuq@xps13 (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 6B69A2BE3; Fri, 22 Apr 2016 23:14:59 +0200 (CEST) Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by dpdk.org (Postfix) with ESMTP id C99232BD9 for <dev@dpdk.org>; Fri, 22 Apr 2016 23:14:58 +0200 (CEST) Received: by mail-wm0-f50.google.com with SMTP id u206so44427505wme.1 for <dev@dpdk.org>; Fri, 22 Apr 2016 14:14:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:organization:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=VSZKBWPp/xo3FEdjlVoTIOFBP5vtOawGxw2OaUyATyY=; b=yZ/DbrngRlYdqIFpBC4DPjFlryQQo1S2WQ/EcA/Cbi3OchSG63wTtd78DH15PUW7Yk kliDtwilAn7LBKoXlHOdi6z/tJAW77duU9cnXz2+cGm2cPIhrNR/hwsQQ+PC4Kj6/evH 9TPA62TVXPnlojRMNC1nk8tI+/pH4lGdZpr8K7EbF9+hPtHwq1rQ6BzQ2uJh1HuU/rma p/JLu6W7gUQBxGuvmMzPiqgNVPrZO0E/KVrempIC8jugJIO7N4onz/3NECDxbHRV/8eg qUSNATXNa26nHZ7J6Y1Zl0drrxiL1u59oG8VAztG/lRPdo9Vaurjw/DN2s6RcADVshXd r6ag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding; bh=VSZKBWPp/xo3FEdjlVoTIOFBP5vtOawGxw2OaUyATyY=; b=RzUcRq0/kV/S0SWZYTfcBhAUySOTeAwztnTRREeYbJ3M9JgrsWL5hXs6E8EFd4KOVc RzTHT5xqO69oO7URzvXVfG4n1WuLrmTJD/WzaNOxSd7BjHXvMej1QZeRFXFi05pJofQZ 2aQmSF4yB9n0MHI2lr9cTxP0qoHWkBOw9ZcpHTa0Z7rIY/rOyQQGrRpnQTPYZyFFdP9a ZfJB86Fh3aDCFMHu9MBuS/1T+XbXkmNBO6mLHQvDV85/+XnRYJATMHuSK5/0C/kLtdCA pMIuO+r4FqOGgzRDQBSCsA5tXWboKPWtA1UAgBNCH6ZU+L5MMzfFOl9HjQ77dUa1e1sY mjmQ== X-Gm-Message-State: AOPr4FWLYDOqBAMtxgKw13pqpJMSG0p6bxiyG2lTYOsWFKb5dXS+UtAt/XF7mkA7luPdJOC/ X-Received: by 10.28.187.5 with SMTP id l5mr5548165wmf.17.1461359698589; Fri, 22 Apr 2016 14:14:58 -0700 (PDT) Received: from xps13.localnet (APoitiers-658-1-63-44.w86-221.abo.wanadoo.fr. [86.221.58.44]) by smtp.gmail.com with ESMTPSA id o73sm5134040wme.16.2016.04.22.14.14.57 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 22 Apr 2016 14:14:57 -0700 (PDT) From: Thomas Monjalon <thomas.monjalon@6wind.com> To: Yuanhan Liu <yuanhan.liu@linux.intel.com> Cc: dev@dpdk.org Date: Fri, 22 Apr 2016 23:14:35 +0200 Message-ID: <4685487.eCM9t2Cvuq@xps13> Organization: 6WIND User-Agent: KMail/4.14.10 (Linux/4.1.6-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <20160422195110.GE7603@yliu-dev.sh.intel.com> References: <1461332640-24273-1-git-send-email-thomas.monjalon@6wind.com> <1461332640-24273-5-git-send-email-thomas.monjalon@6wind.com> <20160422195110.GE7603@yliu-dev.sh.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Subject: Re: [dpdk-dev] [PATCH 4/4] eal: add assert macro for debug X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Thomas Monjalon
April 22, 2016, 9:14 p.m. UTC
2016-04-22 12:51, Yuanhan Liu: > On Fri, Apr 22, 2016 at 03:44:00PM +0200, Thomas Monjalon wrote: > > --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c > > +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c > > @@ -662,8 +662,8 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > > > > PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx); > > > > - VMXNET3_ASSERT(rcd->len <= rxd->len); > > - VMXNET3_ASSERT(rbi->m); > > + RTE_ASSERT(rcd->len <= rxd->len); > > + RTE_ASSERT(rbi->m); > > Seems this piece of code introduces a build error: > > drivers/net/vmxnet3/vmxnet3_rxtx.c: In function ‘vmxnet3_recv_pkts’: > drivers/net/vmxnet3/vmxnet3_rxtx.c:635:18: error: variable ‘rxd’ set but not used > Vmxnet3_RxDesc *rxd; > ^ Indeed it can be seen with gcc when debug is disabled. It can be fixed this way:
Comments
On Fri, Apr 22, 2016 at 11:14:35PM +0200, Thomas Monjalon wrote: > 2016-04-22 12:51, Yuanhan Liu: > > On Fri, Apr 22, 2016 at 03:44:00PM +0200, Thomas Monjalon wrote: > > > --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c > > > +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c > > > @@ -662,8 +662,8 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > > > > > > PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx); > > > > > > - VMXNET3_ASSERT(rcd->len <= rxd->len); > > > - VMXNET3_ASSERT(rbi->m); > > > + RTE_ASSERT(rcd->len <= rxd->len); > > > + RTE_ASSERT(rbi->m); > > > > Seems this piece of code introduces a build error: > > > > drivers/net/vmxnet3/vmxnet3_rxtx.c: In function ‘vmxnet3_recv_pkts’: > > drivers/net/vmxnet3/vmxnet3_rxtx.c:635:18: error: variable ‘rxd’ set but not used > > Vmxnet3_RxDesc *rxd; > > ^ > > Indeed it can be seen with gcc when debug is disabled. > It can be fixed this way: > > --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c > +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c > @@ -658,6 +658,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > idx = rcd->rxdIdx; > ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1); > rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx; > + RTE_SET_USED(rxd); /* used only for assert when enabled */ How about adding the __rte_unused tag at where we declare it? --yliu
On Fri, 22 Apr 2016 15:08:50 -0700 Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote: > On Fri, Apr 22, 2016 at 11:14:35PM +0200, Thomas Monjalon wrote: > > 2016-04-22 12:51, Yuanhan Liu: > > > On Fri, Apr 22, 2016 at 03:44:00PM +0200, Thomas Monjalon wrote: > > > > --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c > > > > +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c > > > > @@ -662,8 +662,8 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > > > > > > > > PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx); > > > > > > > > - VMXNET3_ASSERT(rcd->len <= rxd->len); > > > > - VMXNET3_ASSERT(rbi->m); > > > > + RTE_ASSERT(rcd->len <= rxd->len); > > > > + RTE_ASSERT(rbi->m); > > > > > > Seems this piece of code introduces a build error: > > > > > > drivers/net/vmxnet3/vmxnet3_rxtx.c: In function ‘vmxnet3_recv_pkts’: > > > drivers/net/vmxnet3/vmxnet3_rxtx.c:635:18: error: variable ‘rxd’ set but not used > > > Vmxnet3_RxDesc *rxd; > > > ^ > > > > Indeed it can be seen with gcc when debug is disabled. > > It can be fixed this way: > > > > --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c > > +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c > > @@ -658,6 +658,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > > idx = rcd->rxdIdx; > > ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1); > > rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx; > > + RTE_SET_USED(rxd); /* used only for assert when enabled */ > > How about adding the __rte_unused tag at where we declare it? > > --yliu Why not just kill the useless assert's all together? They really only helped during the short time developer is debugging this code.
2016-04-22 15:42, Stephen Hemminger: > On Fri, 22 Apr 2016 15:08:50 -0700 > Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote: > > On Fri, Apr 22, 2016 at 11:14:35PM +0200, Thomas Monjalon wrote: > > > rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx; > > > + RTE_SET_USED(rxd); /* used only for assert when enabled */ > > > > How about adding the __rte_unused tag at where we declare it? It is not really unused. And adding a SET_USED line allows to put a comment in the context below the assignment. > Why not just kill the useless assert's all together? They really only helped > during the short time developer is debugging this code. They also provide some kind of comments and can help when refactoring. Anyway, removing the assert would deserve another patch.
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c @@ -658,6 +658,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) idx = rcd->rxdIdx; ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1); rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx; + RTE_SET_USED(rxd); /* used only for assert when enabled */ rbi = rxq->cmd_ring[ring_idx].buf_info + idx; PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx);