From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
To: Pavel Machek <pavel@ucw.cz>
Cc: bh74.an@samsung.com, ks.giri@samsung.com,
vipul.pandya@samsung.com, peppe.cavallaro@st.com,
alexandre.torgue@st.com, romieu@fr.zoreil.com,
davem@davemloft.net, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH v2 2/2] net: ethernet: stmmac: remove private tx queue lock
Date: Thu, 15 Dec 2016 20:37:03 +0100 [thread overview]
Message-ID: <626fc3ef-ba18-ed99-aea1-2f737425b199@gmx.de> (raw)
In-Reply-To: <20161215094517.GA406@amd>
Hi,
On 15.12.2016 10:45, Pavel Machek wrote:
> Hi!
>
>> The driver uses a private lock for synchronization of the xmit function and
>> the xmit completion handler, but since the NETIF_F_LLTX flag is not set,
>> the xmit function is also called with the xmit_lock held.
>>
>> On the other hand the completion handler uses the reverse locking order by
>> first taking the private lock and (in case that the tx queue had been
>> stopped) then the xmit_lock.
>>
>> Improve the locking by removing the private lock and using only the
>> xmit_lock for synchronization instead.
>
> Do you have stmmac hardware to test on?
>
Unfortunately not (I mentioned that the patch I send was only compile tested in
the first version but I think I forgot to do so in the last version).
> I believe something is very wrong with the locking there. In
> particular... scheduling the stmmac_tx_timer() function to run often
> should not do anything bad if locking is correct... but it breaks the
> driver rather quickly. [Example patch below, needs applying to two
> places in net-next.]
>
Do you get this result only after the private lock is removed? Or has this problem
been there before? And how exactly does the failure look like?
Regards,
Lino
next prev parent reply other threads:[~2016-12-15 19:44 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-08 23:55 Remove private tx queue locks Lino Sanfilippo
2016-12-08 23:55 ` [PATCH v2 1/2] net: ethernet: sxgbe: remove private tx queue lock Lino Sanfilippo
2016-12-08 23:55 ` [PATCH v2 2/2] net: ethernet: stmmac: " Lino Sanfilippo
2016-12-15 9:45 ` Pavel Machek
2016-12-15 10:08 ` Giuseppe CAVALLARO
2016-12-15 10:45 ` Pavel Machek
2016-12-15 19:37 ` Lino Sanfilippo [this message]
2016-12-15 22:03 ` Pavel Machek
[not found] ` <CAD5ja63O5jFtt=RyXTBxRxKSaJVfyL+kFeJwwen-pt0ifQARYQ@mail.gmail.com>
2016-12-15 19:42 ` Lino Sanfilippo
2016-12-11 4:27 ` Remove private tx queue locks David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=626fc3ef-ba18-ed99-aea1-2f737425b199@gmx.de \
--to=linosanfilippo@gmx.de \
--cc=alexandre.torgue@st.com \
--cc=bh74.an@samsung.com \
--cc=davem@davemloft.net \
--cc=ks.giri@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=peppe.cavallaro@st.com \
--cc=romieu@fr.zoreil.com \
--cc=vipul.pandya@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).