Re: [net-next PATCH v2 3/4] netdev: replace napi_reschedule with napi_schedule

From: Christian Marangi <ansuelsmth@gmail.com>
To: Eric Dumazet <edumazet@google.com>
Cc: "Jakub Kicinski" <kuba@kernel.org>,
	"Jason Gunthorpe" <jgg@ziepe.ca>,
	"Leon Romanovsky" <leon@kernel.org>,
	"Wolfgang Grandegger" <wg@grandegger.com>,
	"Marc Kleine-Budde" <mkl@pengutronix.de>,
	"David S. Miller" <davem@davemloft.net>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Chris Snook" <chris.snook@gmail.com>,
	"Raju Rangoju" <rajur@chelsio.com>,
	"Jeroen de Borst" <jeroendb@google.com>,
	"Praveen Kaligineedi" <pkaligineedi@google.com>,
	"Shailend Chand" <shailend@google.com>,
	"Douglas Miller" <dougmill@linux.ibm.com>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	"Christophe Leroy" <christophe.leroy@csgroup.eu>,
	"Nick Child" <nnac123@linux.ibm.com>,
	"Haren Myneni" <haren@linux.ibm.com>,
	"Rick Lindsley" <ricklind@linux.ibm.com>,
	"Dany Madden" <danymadden@us.ibm.com>,
	"Thomas Falcon" <tlfalcon@linux.ibm.com>,
	"Tariq Toukan" <tariqt@nvidia.com>,
	"Alexandre Torgue" <alexandre.torgue@foss.st.com>,
	"Jose Abreu" <joabreu@synopsys.com>,
	"Maxime Coquelin" <mcoquelin.stm32@gmail.com>,
	"Krzysztof Halasa" <khalasa@piap.pl>,
	"Kalle Valo" <kvalo@kernel.org>,
	"Jeff Johnson" <quic_jjohnson@quicinc.com>,
	"Gregory Greenman" <gregory.greenman@intel.com>,
	"Chandrashekar Devegowda" <chandrashekar.devegowda@intel.com>,
	"Intel Corporation" <linuxwwan@intel.com>,
	"Chiranjeevi Rapolu" <chiranjeevi.rapolu@linux.intel.com>,
	"Liu Haijun" <haijun.liu@mediatek.com>,
	"M Chetan Kumar" <m.chetan.kumar@linux.intel.com>,
	"Ricardo Martinez" <ricardo.martinez@linux.intel.com>,
	"Loic Poulain" <loic.poulain@linaro.org>,
	"Sergey Ryazanov" <ryazanov.s.a@gmail.com>,
	"Johannes Berg" <johannes@sipsolutions.net>,
	"Yuanjun Gong" <ruc_gongyuanjun@163.com>,
	"Simon Horman" <horms@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Ziwei Xiao" <ziweixiao@google.com>,
	"Rushil Gupta" <rushilg@google.com>,
	"Coco Li" <lixiaoyan@google.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Junfeng Guo" <junfeng.guo@intel.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Wei Fang" <wei.fang@nxp.com>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
	"Yuri Karpov" <YKarpov@ispras.ru>,
	"Zhengchao Shao" <shaozhengchao@huawei.com>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Zheng Zengkai" <zhengzengkai@huawei.com>,
	"Lee Jones" <lee@kernel.org>,
	"Maximilian Luz" <luzmaximilian@gmail.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	"Dawei Li" <set_pte_at@outlook.com>,
	Anjaneyulu <pagadala.yesu.anjaneyulu@intel.com>,
	"Benjamin Berg" <benjamin.berg@intel.com>,
	linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-can@vger.kernel.org, netdev@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org, ath10k@lists.infradead.org,
	linux-wireless@vger.kernel.org
Subject: Re: [net-next PATCH v2 3/4] netdev: replace napi_reschedule with napi_schedule
Date: Fri, 6 Oct 2023 20:52:57 +0200	[thread overview]
Message-ID: <65205789.5d0a0220.7e49b.ccb0@mx.google.com> (raw)
In-Reply-To: <CANn89iJQ50AdXP2C1YB2pGjE02WCJ-QCsZqE1yGXtcGsfLA0Jw@mail.gmail.com>

On Thu, Oct 05, 2023 at 06:41:03PM +0200, Eric Dumazet wrote:
> On Thu, Oct 5, 2023 at 6:32 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Thu, 5 Oct 2023 18:11:56 +0200 Eric Dumazet wrote:
> > > OK, but I suspect some users of napi_reschedule() might not be race-free...
> >
> > What's the race you're thinking of?
> 
> This sort of thing... the race is in fl_starving() though...
> 
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c
> b/drivers/net/ethernet/chelsio/cxgb4/sge.c
> index 98dd78551d89..b5ff2e1a9975 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
> @@ -4261,7 +4261,7 @@ static void sge_rx_timer_cb(struct timer_list *t)
> 
>                         if (fl_starving(adap, fl)) {
>                                 rxq = container_of(fl, struct sge_eth_rxq, fl);
> -                               if (napi_reschedule(&rxq->rspq.napi))
> +                               if (napi_schedule(&rxq->rspq.napi))
>                                         fl->starving++;
>                                 else
>                                         set_bit(id, s->starving_fl);

Ehhh problem is that this is a simple rename so if any race is present,
it's already there and not caused by this rename :(

Don't know maybe this is out of scope and should be investigated with a
bug report?

Maybe this should be changed to prep/__schedule to prevent any kind of
race? But doing so doesn't prevent any kind of ""starving""?

-- 
	Ansuel

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k