From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f195.google.com (mail-wr0-f195.google.com [209.85.128.195]) by dpdk.org (Postfix) with ESMTP id 522691B21E for ; Wed, 14 Feb 2018 14:52:42 +0100 (CET) Received: by mail-wr0-f195.google.com with SMTP id k32so16758wrk.4 for ; Wed, 14 Feb 2018 05:52:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=xZ7+92GzMCasCKXel5IIGs9vJG7z38i2k6FKuKBtMEw=; b=Xvy45XE1+ftsXOoPSoLF82cUPY/8MvV+xHSjyQ2jGfoc5tQwtZ87vhW/53Q5hKnpyc XyIVgom7Lftq0oxv7AbxWCVK4qh93U6ow+acwSG8vaUPMuAfHlKSpA4mzAMrNjSpn/Jo H736NfFkSPLYO7AZZdtnkGfkgWNLOtndl59uD5lmjWRiBPK8Wi2dQgzyg7qVveapsUli 4qTGVyq0f1IM5iotLhH2pUEPji59WXrr2pbmOVHr25EzPGylLbVtcRs5jZ2w0W7AhPFb OF6WpxrJw4xqaAHONEEhFKij2ejE32TuiAEtz275gUBVCmXVvPG1sU7Y9yRR80Vgxabn 0QGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=xZ7+92GzMCasCKXel5IIGs9vJG7z38i2k6FKuKBtMEw=; b=JwHbNvFXPot24LulKUqAM9niYh85qIRSeMAf8ZOOMBmN7tzX/md89Pbx6AjNlZG+A1 bNitmt95OFcdf3twgGHFunankt5pn4fD7nnTQYJGf53+DBUK8ZDY/OgE8tB87ZGMuAEa Bn3AFAKP5R0wqDwVHidinKzw02rOatCLczfb/w8a9N00/YsJpI5syu2I3egeNpKblZX1 wZjfn7/jHWHeABsoaKHmyYBC5+uD4IYVxj0210MHiLBI0aHcRCpEbPCrWEPMJ5MMvkJT v31DTXg/FakE8yK9CoVN/i7s1CNOURdhvH7GNiOsGv5SLoqHgzqf+AfrvHEQSPjeALN4 wS1g== X-Gm-Message-State: APf1xPBR6UsHSsHG9wsChBKHYszS5YKopqAxS9UDX7uCMr4S/e1x0gmQ qImcB0d2jKl8RKwQ5cZrGG5G6kRX X-Google-Smtp-Source: AH8x226LS++/b1hM5+HwUQzhjsqNFV2L4Kl/7Z6bht4jK7J/rBokUt4SAdq/XWL+00v1tgnPeZwxsA== X-Received: by 10.223.199.12 with SMTP id k12mr4777797wrg.261.1518616361723; Wed, 14 Feb 2018 05:52:41 -0800 (PST) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id i11sm7021958wre.36.2018.02.14.05.52.40 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 14 Feb 2018 05:52:40 -0800 (PST) Date: Wed, 14 Feb 2018 14:52:27 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Matan Azrad Cc: dev@dpdk.org Message-ID: <20180214135227.o7lddtw5jmnnec7u@bidouze.vm.6wind.com> References: <1518562772-29823-1-git-send-email-matan@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1518562772-29823-1-git-send-email-matan@mellanox.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] net/failsafe: fix Rx interrupt reinstallation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Feb 2018 13:52:42 -0000 Hi Matan, On Tue, Feb 13, 2018 at 10:59:32PM +0000, Matan Azrad wrote: > Fail-safe dev_start() operation can be called by both the application > and the hot-plug alarm mechanism. > > The installation of Rx interrupt are triggered from dev_start() in any > time it is called while actually the Rx interrupt should be installed > only by the application calls. > > So, each plug-in event causes reinstallation which causes memory leak. > > Trigger the Rx interrupt installation only for application calls. > > Fixes: 9e0360aebf23 ("net/failsafe: register as Rx interrupt mode") > > Signed-off-by: Matan Azrad > --- > drivers/net/failsafe/failsafe_ops.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c > index 057e435..bbbd335 100644 > --- a/drivers/net/failsafe/failsafe_ops.c > +++ b/drivers/net/failsafe/failsafe_ops.c > @@ -181,10 +181,12 @@ > int ret; > > fs_lock(dev, 0); > - ret = failsafe_rx_intr_install(dev); > - if (ret) { > - fs_unlock(dev, 0); > - return ret; > + if (PRIV(dev)->alarm_lock == 0) { I dislike having to rely on unrelated context of execution to decide a code-path. I'd prefer to make interrupt installation dependent on the interrupt state instead. I think it should be possible to forbid reinstallation within failsafe_rx_intr_install directly, e.g. diff --git a/drivers/net/failsafe/failsafe_intr.c b/drivers/net/failsafe/failsafe_intr.c index f6ff04dc8..46c3aa5f2 100644 --- a/drivers/net/failsafe/failsafe_intr.c +++ b/drivers/net/failsafe/failsafe_intr.c @@ -523,7 +523,8 @@ failsafe_rx_intr_install(struct rte_eth_dev *dev) const struct rte_intr_conf *const intr_conf = &priv->dev->data->dev_conf.intr_conf; - if (intr_conf->rxq == 0) + if (intr_conf->rxq == 0 || + dev->intr_handle != NULL) return 0; if (fs_rx_intr_vec_install(priv) < 0) return -rte_errno; This way the logic is self-dependent and the check limited to this component. There might be better way to do this, it's only an example to explain my point. > + ret = failsafe_rx_intr_install(dev); > + if (ret) { > + fs_unlock(dev, 0); > + return ret; > + } > } > FOREACH_SUBDEV(sdev, i, dev) { > if (sdev->state != DEV_ACTIVE) > -- > 1.9.5 > -- Gaëtan Rivet 6WIND