From mboxrd@z Thu Jan  1 00:00:00 1970
From: Miquel RAYNAL <miquel.raynal@free-electrons.com>
Subject: Re: [PATCH net-next v2] net: mvpp2: add ethtool GOP statistics
Date: Mon, 6 Nov 2017 23:45:23 +0100
Message-ID: <20171106234523.7e170a71@xps13>
References: <20171103110425.16097-1-miquel.raynal@free-electrons.com>
        <09d456be927a46c0970bd677651a4098@IL-EXCH01.marvell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
        Antoine Tenart <antoine.tenart@free-electrons.com>,
        "David S. Miller" <davem@davemloft.net>, netdev@vger.kernel.org
To: Stefan Chulski <stefanc@marvell.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail.free-electrons.com ([62.4.15.54]:57200 "EHLO
        mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750728AbdKFWpZ (ORCPT
        <rfc822;netdev@vger.kernel.org>); Mon, 6 Nov 2017 17:45:25 -0500
In-Reply-To: <09d456be927a46c0970bd677651a4098@IL-EXCH01.marvell.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hi Stefan,

+David Miller/Net ML

> > @@ -6844,6 +7023,10 @@ static int mvpp2_open(struct net_device
> > *dev)
> > 
> >  	mvpp2_start_dev(port);
> > 
> > +	/* Start hardware statistics gathering */
> > +	queue_delayed_work(priv->stats_queue, &priv->stats_work,
> > +			   MVPP2_MIB_COUNTERS_STATS_DELAY);
> > +
> >  	return 0;
> > 
> >  err_free_link_irq:
> > @@ -6888,6 +7071,9 @@ static int mvpp2_stop(struct net_device *dev)
> >  	mvpp2_cleanup_rxqs(port);
> >  	mvpp2_cleanup_txqs(port);
> > 
> > +	cancel_delayed_work_sync(&priv->stats_work);
> > +	flush_workqueue(priv->stats_queue);
> > +  
> 
> Hi Miquel,
> 
> I think there are bug here.
> priv is common for all ports on same CPN and they have same
> priv->stats_work.
> 
> For example on A7K board with 3 Ports. queue_delayed_work and
> cancel_delayed_work_sync called for each port stop and start
> procedure. For example:
> If Port0 and Port1 were started, then if only Port0 stopped, delayed
> work would be canceled for both ports.


Thanks for spotting it, you are right this is a bug since I moved
starting/stopping the queues in the opening and close procedure of the
ports (to avoid using CPU time while no interface is actually up).

Maybe I should have a work per port, it would be easier to
handle.

Thank you,
Miquèl