Re: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
On Thu, Feb 11, 2021 at 01:22:35PM +, Stefan Chulski wrote: > > Ditto. > > > > I don't think these need to be fixed in the net tree, but it would still be > > nice > > to fix the problem. Please do so, as an initial patch in your series - so > > we can > > then backport if it turns out to eventually be necessary. > > > > Thanks. > > My series already has 15 patches and patchwork not happy about series with > over 15 patches. > Maybe I can send this as separate patch to net-next(or net) first and base > this series on this net-next tree with this patch? In that case, send the fixes as a separate series and get that merged first. It shouldn't take very long to get the fixes merged. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
On Thu, Feb 11, 2021 at 01:02:14PM +, Stefan Chulski wrote:
> > On Thu, Feb 11, 2021 at 12:48:55PM +0200, stef...@marvell.com wrote:
> > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > index 761f745..8b4073c 100644
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -1133,14 +1133,19 @@ static inline void
> > > mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec) static
> > > void mvpp2_interrupts_mask(void *arg) {
> > > struct mvpp2_port *port = arg;
> > > + int cpu = smp_processor_id();
> > > + u32 thread;
> > >
> > > /* If the thread isn't used, don't do anything */
> > > - if (smp_processor_id() > port->priv->nthreads)
> > > + if (cpu > port->priv->nthreads)
> > > return;
> >
> > What happened to a patch fixing this? Did I miss it? Was it submitted
> > independently to the net tree?
>
> Some reviewers asked to remove this from the series. I would send it as
> separate patch to net.
It is not a regression, and although it is a fix, as you explained when
I first raised it, it isn't a condition that can be reached due to:
priv->nthreads = min_t(unsigned int, num_present_cpus(),
MVPP2_MAX_THREADS);
and I don't think we support a dynamic present CPU mask on any platform
that is currently supported by this driver.
If we did, then it would be possible for the off-by-one issue to be
triggered.
No matter what, it should happen _before_ this patch set is merged.
Trying to do it afterwards guarantees more pain if stable trees decide
they want to backport the fix.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
RE: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
> Ditto. > > I don't think these need to be fixed in the net tree, but it would still be > nice > to fix the problem. Please do so, as an initial patch in your series - so we > can > then backport if it turns out to eventually be necessary. > > Thanks. My series already has 15 patches and patchwork not happy about series with over 15 patches. Maybe I can send this as separate patch to net-next(or net) first and base this series on this net-next tree with this patch? Regards, Stefan.
RE: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
> -Original Message-
> From: Russell King - ARM Linux admin
> Sent: Thursday, February 11, 2021 2:50 PM
> To: Stefan Chulski
> Cc: net...@vger.kernel.org; thomas.petazz...@bootlin.com;
> da...@davemloft.net; Nadav Haklai ; Yan
> Markman ; linux-kernel@vger.kernel.org;
> k...@kernel.org; m...@semihalf.com; and...@lunn.ch;
> aten...@kernel.org; devicet...@vger.kernel.org; robh...@kernel.org;
> sebastian.hesselba...@gmail.com; gregory.clem...@bootlin.com; linux-
> arm-ker...@lists.infradead.org
> Subject: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non
> occupied descriptor threshold
>
> External Email
>
> --
> On Thu, Feb 11, 2021 at 12:48:55PM +0200, stef...@marvell.com wrote:
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > index 761f745..8b4073c 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -1133,14 +1133,19 @@ static inline void
> > mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec) static
> > void mvpp2_interrupts_mask(void *arg) {
> > struct mvpp2_port *port = arg;
> > + int cpu = smp_processor_id();
> > + u32 thread;
> >
> > /* If the thread isn't used, don't do anything */
> > - if (smp_processor_id() > port->priv->nthreads)
> > + if (cpu > port->priv->nthreads)
> > return;
>
> What happened to a patch fixing this? Did I miss it? Was it submitted
> independently to the net tree?
Some reviewers asked to remove this from the series. I would send it as
separate patch to net.
Regards.
