Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932385AbcLGNTw (ORCPT ); Wed, 7 Dec 2016 08:19:52 -0500 Received: from mx0a-0016f401.pphosted.com ([67.231.148.174]:40403 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752976AbcLGNSv (ORCPT ); Wed, 7 Dec 2016 08:18:51 -0500 Subject: Re: [RFC] Re: Re: stmmac ethernet in kernel 4.9-rc6: coalescing related pauses. To: Pavel Machek , Lino Sanfilippo References: <20161123105125.GA26394@amd> <20161124085506.GA25007@amd> <20161124.110416.198867271899443489.davem@davemloft.net> <20161124212540.GA24796@amd> <20161202084511.GA32294@amd> <20161207123140.GA8785@amd> CC: Giuseppe CAVALLARO , , David Miller , , From: Lino Sanfilippo Message-ID: Date: Wed, 7 Dec 2016 14:18:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161207123140.GA8785@amd> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-12-07_04:,, signatures=0 X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=3 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1612070216 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1203 Lines: 45 Hi, On 07.12.2016 13:31, Pavel Machek wrote: > On Fri 2016-12-02 15:05:21, Lino Sanfilippo wrote: >> Hi, >> >> >>> >>> There's nothing that protect stmmac_poll() from running concurently >>> with stmmac_dma_interrupt(), right? >>> >> >> could it be that there is also another issue concerned locking?: >> The tx completion handler takes the xmit_lock in case that the >> netif_queue is stopped. This is AFAICS unnecessary, since both >> xmit and completion handler are already synchronized by the private >> tx lock. But it is IMHO also dangerous: >> >> In the xmit handler we have the locking order >> 1. xmit_lock >> 2. private tx lock >> >> while in the completion handler its the reverse: >> >> 1. private tx lock >> 2. xmit lock. >> >> Do I miss something? > > No, it seems you are right. Something like this? > > Hmm. And can priv->tx_lock be removed, as we already rely on > netif_tx_lock? > David wanted indeed the private lock to be removed completely. See this thread. https://marc.info/?l=linux-netdev&m=148072001900620&w=2 If you can wait a bit, I already have patches prepared to fix this issue in both drivers. I want to send them as soon as I am at home (a few hours). Regards, Lino