public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: <Parthiban.Veerasooran@microchip.com>
To: <kuba@kernel.org>
Cc: <andrew+netdev@lunn.ch>, <davem@davemloft.net>,
	<edumazet@google.com>, <pabeni@redhat.com>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<UNGLinuxDriver@microchip.com>, <jacob.e.keller@intel.com>
Subject: Re: [PATCH net v3 2/2] net: ethernet: oa_tc6: fix tx skb race condition between reference pointers
Date: Mon, 16 Dec 2024 04:16:18 +0000	[thread overview]
Message-ID: <26aaf32b-7085-4ada-8bd0-a930b8bb51a8@microchip.com> (raw)
In-Reply-To: <20241213184253.7c8203ce@kernel.org>

Hi Jakub,

On 14/12/24 8:12 am, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, 13 Dec 2024 10:35:03 +0000 Parthiban.Veerasooran@microchip.com
> wrote:
>>>> start_xmit runs in BH / softirq context. You can't take sleeping locks.
>>>> The lock has to be a spin lock. You could possibly try to use the
>>>> existing spin lock of the tx queue (__netif_tx_lock()) but that may be
>>>> more challenging to do cleanly from within a library..
>>> Thanks for the input. Yes, it looks like implementing a spin lock would
>>> be a right choice. I will implement it and do the testing as you
>>> suggested below and share the feedback.
>> I tried using spin_lock_bh() variants (as the softirq involved) on both
>> start_xmit() and spi_thread() where the critical regions need to be
>> protected and tested by enabling the Kconfigs in the
>> kernel/configs/debug.config. Didn't notice any warnings in the dmesg log.
>>
>> Note: Prior to the above test, purposefully I tried with spin_lock()
>> variants on both the sides to check/simulate for the warnings using
>> Kconfigs kernel/configs/debug.config. Got some warnings in the dmesg
>> regarding deadlock which clarified the expected behavior. And then I
>> proceeded with the above fix and it worked as expected.
>>
>> If you agree, I will prepare the next version with this fix and post.
> 
> Go ahead.
Thanks for your comment. It is already posted for review.

https://patchwork.kernel.org/project/netdevbpf/patch/20241213123159.439739-3-parthiban.veerasooran@microchip.com/

Best regards,
Parthiban V


      reply	other threads:[~2024-12-16  4:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-04 13:35 [PATCH net v3 0/2] Fixes on the OPEN Alliance TC6 10BASE-T1x MAC-PHY support generic lib Parthiban Veerasooran
2024-12-04 13:35 ` [PATCH net v3 1/2] net: ethernet: oa_tc6: fix infinite loop error when tx credits becomes 0 Parthiban Veerasooran
2024-12-04 13:35 ` [PATCH net v3 2/2] net: ethernet: oa_tc6: fix tx skb race condition between reference pointers Parthiban Veerasooran
2024-12-10  0:11   ` Jakub Kicinski
2024-12-12 12:33     ` Parthiban.Veerasooran
2024-12-13 10:35       ` Parthiban.Veerasooran
2024-12-14  2:42         ` Jakub Kicinski
2024-12-16  4:16           ` Parthiban.Veerasooran [this message]

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=26aaf32b-7085-4ada-8bd0-a930b8bb51a8@microchip.com \
    --to=parthiban.veerasooran@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jacob.e.keller@intel.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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