From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752208AbcEJSdP (ORCPT <rfc822;w@1wt.eu>);
	Tue, 10 May 2016 14:33:15 -0400
Received: from mout.gmx.net ([212.227.15.15]:61796 "EHLO mout.gmx.net"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751642AbcEJSdM (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Tue, 10 May 2016 14:33:12 -0400
Subject: Re: [PATCH v2 1/2] net: nps_enet: Sync access to packet sent flag
To: Elad Kanfi <eladkan@mellanox.com>, Lino Sanfilippo <lsanfil@marvell.com>,
        David Miller <davem@davemloft.net>
References: <1461763110-15263-1-git-send-email-eladkan@mellanox.com>
 <1461763110-15263-2-git-send-email-eladkan@mellanox.com>
 <20160428.171120.2028974436439452264.davem@davemloft.net>
 <HE1PR0501MB2026E62285B397A5C7CC9705CD790@HE1PR0501MB2026.eurprd05.prod.outlook.com>
 <572736D6.5030902@marvell.com>
 <HE1PR0501MB20265121CBDB2D48F66D408CCD7F0@HE1PR0501MB2026.eurprd05.prod.outlook.com>
Cc: Noam Camus <noamca@mellanox.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "abrodkin@synopsys.com" <abrodkin@synopsys.com>,
        Tal Zilcer <talz@mellanox.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Message-ID: <57322951.3040109@gmx.de>
Date: Tue, 10 May 2016 20:32:49 +0200
User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101
 Thunderbird/38.7.2
MIME-Version: 1.0
In-Reply-To: <HE1PR0501MB20265121CBDB2D48F66D408CCD7F0@HE1PR0501MB2026.eurprd05.prod.outlook.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
X-Provags-ID: V03:K0:rRtze8E++YX4gN74i8dzDJbGsbeb4Q9m9NvqKQco06PSdNakN1c
 Q6jiPXSK83LzuKAkefUL09r6f8TIQj8owh5ELLBFarKQIb7PfoWzi+VqDyLtfaLV3V49C6y
 HnJAEdORKWWojYrldVci7gfFR2lASsuWFGxE9kINa0OjR+N6YRGR9YkKqF0pL+YHCwHkTnj
 8cRkA5hWrpgIRnxkG/3Nw==
X-UI-Out-Filterresults: notjunk:1;V01:K0:3HozhC/b2DI=:pUrGjXVQd+uOtev81kEO/y
 1iJI226WwITUnzin826pfzplCXHxHiQz8BZ99lMpPt5TglS63mU50zfYQA20Veloq/JdN/HGy
 6sjQu1mMgBMpyiOSSFq61BV5hdHPIjQbixWghTNd0eK+9GTJ6dwc7lSK6mAfHcAUuFgxqOAMO
 WXlDYvwGadCJQlBariLCB8WI7fCDFPBFjMd5DwJ2yLxoLCH5VTOrOVaBgwvPnRqepMOerFpK2
 RpCUqMOxo+fuJx0IAElXKBTUsXIQpYrkMJyh8pGRUlRa/A7pAZM8MAZbtkG5PbILYeMtC9G8p
 io/TFCHmxvDkuS4fS1Oosl37G6TPFfIoPmsH5JvHPRuvG4uOvKus9AQvINqcnfiNFHManQ+/k
 2tIWELg6dkHDNJh0+F/4oPq7tSKxAw8/Hm6bf0jCJ49Q3mQxvAC/jjmGN8+vtdmDuBHTkADdh
 ynsmWZlD+ndS7tHym4T8rxlv+qLWOXvaLWp5bEpiRZdhGcY9S3hLkhvFci+FU4MUsjltcUTlB
 EPE3F6jvuAhC3kIpJzVCcc6pT4l7P95QKPk6YI8yAex1QYUBsMJNoEs1eqeIq/0IScX0iqdSo
 nJcpAdxDYsGRg4au5S58srKpB7EYLmkwDgarwBCL01m0UT0he3xjNTVuhnWnLzUzuvdRq9/qX
 8mYaIIp2T3W0Y3DEGwbrA6YGaUsrQDcaOICnHCs69nBt8l7QRIdUKD8Pmjve5llhz0xemzdBJ
 Gy0HaWsi0Dck3KrZI5vv5ShsFm3GRb7UPOlAgVXoN6RIHu22U8hR8qAs5fYqTGM7yWUKcmt7R
 /ZMVl0i
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Elad,

On 08.05.2016 15:44, Elad Kanfi wrote:

>   
> After reviewing the code and your suggestion, it seems that we can do without the flag tx_packet_sent and therefor the first issue becomes irrelevant.
> The indication that a packet was sent is (tx_skb != NULL) , and the sequence will be:
> 
> CPU A:
> 1. tx_skb = skb
> 2. wmb() /* make sure tx_skb reaches the RAM before the HW is informed and the IRQ is fired */
> 3. nps_enet_reg_set(priv, NPS_ENET_REG_TX_CTL, tx_ctrl.value); /* send frame */
> 
> CPU B:
> 1. read tx_skb 
> 2. if( tx_skb != NULL ) handle tx_skb
> 3. tx_skb = NULL 
> 
> 

Ok, without the tx_packet_sent flag the code becomes simpler. But it
does not mean that we can toss the smp_rmb in the irq handler
completely. We still have to use a read barrier there to ensure that we
see the most recent value of tx_skb. E.g like this:

if (priv->tx_skb != NULL ) {
	smp_rmb()
	/ * handle tx_skb */
}

With both barriers in place the code should work as expected.

Regards,
Lino