* [PATCH net-next v7 01/18] net: ethernet: cortina: Keep shared free queue parent-owned
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 02/18] net: ethernet: cortina: Keep ports bound for shared IRQ Linus Walleij
` (16 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The software free queue is shared by both Ethernet ports, but teardown is
not ordered around that ownership. On parent unbind or module unload, the
parent remove callback runs before devres depopulates the child devices. It
therefore destroys the queue while both child netdevs are still registered
and can be doing DMA. Devres then removes each child, and both child remove
callbacks destroy the same queue again, resulting in up to three cleanup
attempts.
Removal before the first interface has been opened is broken as well. The
shared block initialization assigns a nonzero queue order, but allocation
is deferred until open. Cleanup consequently derives a nonzero page count
and dereferences the NULL queue ring.
Depopulate both port devices before cleaning up the queue, and perform that
cleanup only from the parent. Disable the free-queue interrupt after the
managed handlers have drained, since its threaded handler can re-enable it.
Guard cleanup for paths where no queue was allocated.
Hold parent-managed enables on the port clocks across child removal, IRQ
draining and shared queue cleanup, so teardown never relies on clocks
already released by the children.
Port 1 owns the shared refill interrupt, so prevent the children from being
unbound independently. Unregister each netdev before detaching its PHY so
ndo_stop can still access netdev->phydev.
Reported-by: Myeonghun Pak <mhun512@gmail.com>
Closes: https://lore.kernel.org/netdev/20260917192835.58126-1-mhun512@gmail.com/
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 40 +++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 2fe7fd0202d2..0b05a1552983 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1004,6 +1004,9 @@ static void geth_cleanup_freeq(struct gemini_ethernet *geth)
unsigned int pages = len >> fpp_order;
unsigned int pn;
+ if (!geth->freeq_ring)
+ return;
+
writew(readw(geth->base + GLOBAL_SWFQ_RWPTR_REG),
geth->base + GLOBAL_SWFQ_RWPTR_REG + 2);
writel(0, geth->base + GLOBAL_SW_FREEQ_BASE_SIZE_REG);
@@ -2387,11 +2390,10 @@ static irqreturn_t gemini_port_irq(int irq, void *data)
static void gemini_port_remove(struct gemini_ethernet_port *port)
{
if (port->netdev) {
- phy_disconnect(port->netdev->phydev);
unregister_netdev(port->netdev);
+ phy_disconnect(port->netdev->phydev);
}
clk_disable_unprepare(port->pclk);
- geth_cleanup_freeq(port->geth);
}
static void gemini_ethernet_init(struct gemini_ethernet *geth)
@@ -2635,17 +2637,25 @@ static struct platform_driver gemini_ethernet_port_driver = {
.driver = {
.name = "gemini-ethernet-port",
.of_match_table = gemini_ethernet_port_of_match,
+ .suppress_bind_attrs = true,
},
.probe = gemini_ethernet_port_probe,
.remove = gemini_ethernet_port_remove,
};
+static void gemini_ethernet_disable_clk(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
static int gemini_ethernet_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct gemini_ethernet *geth;
unsigned int retry = 5;
+ unsigned int clocks = 0;
u32 val;
+ int ret;
/* Global registers */
geth = devm_kzalloc(dev, sizeof(*geth), GFP_KERNEL);
@@ -2656,6 +2666,30 @@ static int gemini_ethernet_probe(struct platform_device *pdev)
return PTR_ERR(geth->base);
geth->dev = dev;
+ /* Keep both port clocks enabled until the shared queue is torn down. */
+ for_each_available_child_of_node_scoped(dev->of_node, child) {
+ struct clk *clk;
+
+ if (!of_device_is_compatible(child, "cortina,gemini-ethernet-port"))
+ continue;
+
+ clk = devm_get_clk_from_child(dev, child, "PCLK");
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk),
+ "failed to get port clock\n");
+
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+ ret = devm_add_action_or_reset(dev, gemini_ethernet_disable_clk,
+ clk);
+ if (ret)
+ return ret;
+ clocks++;
+ }
+ if (!clocks)
+ return -ENODEV;
+
/* Wait for ports to stabilize */
do {
udelay(2);
@@ -2683,6 +2717,8 @@ static void gemini_ethernet_remove(struct platform_device *pdev)
{
struct gemini_ethernet *geth = platform_get_drvdata(pdev);
+ devm_of_platform_depopulate(&pdev->dev);
+ writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG);
geth_cleanup_freeq(geth);
geth->initialized = false;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 02/18] net: ethernet: cortina: Keep ports bound for shared IRQ
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 01/18] net: ethernet: cortina: Keep shared free queue parent-owned Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-11 23:10 ` netdev-bot+sashiko
2026-11-10 22:13 ` [PATCH net-next v7 03/18] net: ethernet: cortina: Drain free queue IRQ before resize Linus Walleij
` (15 subsequent siblings)
17 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The software free queue interrupt is routed through port 1 even on
boards which only connect a PHY to port 0. Since PHY setup was moved
into probe, the PHY-less port fails to bind and releases the threaded
handler needed by the shared queue.
Publishing the port before PHY setup also leaves a dangling parent pointer
when probe fails and devres frees the netdev private data. Opening the
sibling can then dereference freed memory.
Keep children bound as IRQ-only ports when no PHY is described or when PHY
setup or netdev registration fails, so an unusable interface does not
disable its sibling. Disconnect any attached PHY after registration
failure. Continue to propagate deferred-probe and mandatory resource
errors.
Publish a port to the parent only after its IRQ and optional netdev are
ready, and protect port lifetime with RTNL. Refuse shared queue setup
until both ports have completed initialization.
Keep shared hardware initialization parent-owned across individual child
removal so recreating one child cannot reset a live sibling. Mask
firmware interrupt state before child IRQ handlers are registered, and
use the allocated ring rather than an order sentinel to decide whether
the queue still needs initial setup.
Fixes: 3e813d61401a ("net: gemini: Clean up phy registration")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 126 +++++++++++++++++++++++++---------
1 file changed, 94 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 0b05a1552983..22c3b6b8e2db 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -25,6 +25,7 @@
#include <linux/interrupt.h>
#include <linux/reset.h>
#include <linux/clk.h>
+#include <linux/rtnetlink.h>
#include <linux/of.h>
#include <linux/of_mdio.h>
#include <linux/of_net.h>
@@ -155,6 +156,7 @@ struct gemini_ethernet_port {
struct gemini_ethernet {
struct device *dev;
void __iomem *base;
+ /* Protected by RTNL. */
struct gemini_ethernet_port *port0;
struct gemini_ethernet_port *port1;
bool initialized;
@@ -1051,10 +1053,17 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
u32 en;
int ret;
+ ASSERT_RTNL();
+
+ /* The software free queue interrupt is routed through port 1. */
+ if (!geth->initialized || !geth->port0 || !geth->port1)
+ return -ENODEV;
+
if (netdev->dev_id == 0)
- other_netdev = geth->port1->netdev;
+ other_port = geth->port1;
else
- other_netdev = geth->port0->netdev;
+ other_port = geth->port0;
+ other_netdev = other_port ? other_port->netdev : NULL;
if (other_netdev && netif_running(other_netdev))
return -EBUSY;
@@ -1065,7 +1074,6 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
new_size,
port->rxq_order);
if (other_netdev) {
- other_port = netdev_priv(other_netdev);
new_size += 1 << (other_port->rxq_order + 1);
netdev_dbg(other_netdev, "port %d size: %d order %d\n",
other_netdev->dev_id,
@@ -1076,7 +1084,7 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
new_order = min(15, ilog2(new_size - 1) + 1);
dev_dbg(geth->dev, "set shared queue to size %d order %d\n",
new_size, new_order);
- if (geth->freeq_order == new_order)
+ if (geth->freeq_ring && geth->freeq_order == new_order)
return 0;
spin_lock_irqsave(&geth->irq_lock, flags);
@@ -2387,8 +2395,35 @@ static irqreturn_t gemini_port_irq(int irq, void *data)
return ret;
}
+static void gemini_port_clear(struct gemini_ethernet_port *port)
+{
+ struct gemini_ethernet *geth = port->geth;
+
+ ASSERT_RTNL();
+
+ if (!port->id && geth->port0 == port)
+ geth->port0 = NULL;
+ else if (port->id && geth->port1 == port)
+ geth->port1 = NULL;
+ else
+ return;
+}
+
+static void gemini_ethernet_disable_irqs(struct gemini_ethernet *geth)
+{
+ writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_0_REG);
+ writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_1_REG);
+ writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_2_REG);
+ writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_3_REG);
+ writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG);
+}
+
static void gemini_port_remove(struct gemini_ethernet_port *port)
{
+ rtnl_lock();
+ gemini_port_clear(port);
+ rtnl_unlock();
+
if (port->netdev) {
unregister_netdev(port->netdev);
phy_disconnect(port->netdev->phydev);
@@ -2398,19 +2433,15 @@ static void gemini_port_remove(struct gemini_ethernet_port *port)
static void gemini_ethernet_init(struct gemini_ethernet *geth)
{
+ ASSERT_RTNL();
+
/* Only do this once both ports are online */
if (geth->initialized)
return;
- if (geth->port0 && geth->port1)
- geth->initialized = true;
- else
+ if (!geth->port0 || !geth->port1)
return;
- writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_0_REG);
- writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_1_REG);
- writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_2_REG);
- writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_3_REG);
- writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG);
+ gemini_ethernet_disable_irqs(geth);
/* Interrupt config:
*
@@ -2442,10 +2473,20 @@ static void gemini_ethernet_init(struct gemini_ethernet *geth)
writel(0, geth->base + GLOBAL_HWFQ_RWPTR_REG);
geth->freeq_frag_order = DEFAULT_RX_BUF_ORDER;
- /* This makes the queue resize on probe() so that we
- * set up and enable the queue IRQ. FIXME: fragile.
- */
- geth->freeq_order = 1;
+ geth->initialized = true;
+}
+
+static void gemini_port_publish(struct gemini_ethernet_port *port)
+{
+ struct gemini_ethernet *geth = port->geth;
+
+ ASSERT_RTNL();
+
+ if (!port->id)
+ geth->port0 = port;
+ else
+ geth->port1 = port;
+ gemini_ethernet_init(geth);
}
static void gemini_port_save_mac_addr(struct gemini_ethernet_port *port)
@@ -2541,15 +2582,6 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev)
reset_control_reset(port->reset);
usleep_range(100, 500);
- /* Assign pointer in the main state container */
- if (!id)
- geth->port0 = port;
- else
- geth->port1 = port;
-
- /* This will just be done once both ports are up and reset */
- gemini_ethernet_init(geth);
-
platform_set_drvdata(pdev, port);
/* Set up and register the netdev */
@@ -2600,17 +2632,41 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev)
if (ret)
goto unprepare;
+ if (!of_property_present(np, "phy-handle") &&
+ !of_phy_is_fixed_link(np)) {
+ dev_info(dev, "no PHY\n");
+ goto irq_only;
+ }
+
ret = gmac_setup_phy(netdev);
if (ret) {
- netdev_err(netdev,
- "PHY init failed\n");
- goto unprepare;
+ dev_err(dev, "PHY init failed: %d\n", ret);
+ if (ret == -EPROBE_DEFER)
+ goto unprepare;
+ goto irq_only;
}
- ret = register_netdev(netdev);
- if (ret)
- goto unprepare;
+ rtnl_lock();
+ ret = register_netdevice(netdev);
+ if (!ret)
+ gemini_port_publish(port);
+ rtnl_unlock();
+ if (ret) {
+ phy_disconnect(netdev->phydev);
+ dev_err(dev, "netdev registration failed: %d\n", ret);
+ if (ret == -EPROBE_DEFER)
+ goto unprepare;
+ goto irq_only;
+ }
+
+ return 0;
+irq_only:
+ dev_info(dev, "keeping port for shared IRQ\n");
+ port->netdev = NULL;
+ rtnl_lock();
+ gemini_port_publish(port);
+ rtnl_unlock();
return 0;
unprepare:
@@ -2706,6 +2762,9 @@ static int gemini_ethernet_probe(struct platform_device *pdev)
spin_lock_init(&geth->irq_lock);
spin_lock_init(&geth->freeq_lock);
+ /* Quiesce firmware state before child IRQ handlers are registered. */
+ gemini_ethernet_disable_irqs(geth);
+
/* The children will use this */
platform_set_drvdata(pdev, geth);
@@ -2717,10 +2776,13 @@ static void gemini_ethernet_remove(struct platform_device *pdev)
{
struct gemini_ethernet *geth = platform_get_drvdata(pdev);
+ rtnl_lock();
+ geth->initialized = false;
+ rtnl_unlock();
+
devm_of_platform_depopulate(&pdev->dev);
writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG);
geth_cleanup_freeq(geth);
- geth->initialized = false;
}
static const struct of_device_id gemini_ethernet_of_match[] = {
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH net-next v7 02/18] net: ethernet: cortina: Keep ports bound for shared IRQ
2026-11-10 22:13 ` [PATCH net-next v7 02/18] net: ethernet: cortina: Keep ports bound for shared IRQ Linus Walleij
@ 2026-11-11 23:10 ` netdev-bot+sashiko
0 siblings, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-11-11 23:10 UTC (permalink / raw)
To: linusw
Cc: ulli.kroll, andrew+netdev, davem, kuba, pabeni, mirq-linux,
mhun512, edumazet, netdev
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] In gemini_ethernet_port_probe()
(drivers/net/ethernet/cortina/gemini.c), the new `if (ret ==…
--
net: ethernet: cortina: Keep ports bound for shared IRQ
The software free queue interrupt is routed through port 1, so this patch
keeps Gemini child ports bound as IRQ-only ports when no PHY is described
or when PHY setup or netdev registration fails. It also publishes a port
to the parent only after it is initialized, under RTNL. Global interrupt
masking moves into the parent probe.
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index 0b05a15529832..22c3b6b8e2db5 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
[ ... ]
> @@ -2600,17 +2632,41 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev)
> if (ret)
> goto unprepare;
>
> + if (!of_property_present(np, "phy-handle") &&
> + !of_phy_is_fixed_link(np)) {
> + dev_info(dev, "no PHY\n");
> + goto irq_only;
> + }
> +
> ret = gmac_setup_phy(netdev);
> if (ret) {
> - netdev_err(netdev,
> - "PHY init failed\n");
> - goto unprepare;
> + dev_err(dev, "PHY init failed: %d\n", ret);
> + if (ret == -EPROBE_DEFER)
> + goto unprepare;
> + goto irq_only;
> }
[Severity: Medium]
Can gmac_setup_phy() ever return -EPROBE_DEFER here?
It calls of_phy_get_and_connect() and turns any NULL into -ENODEV:
drivers/net/ethernet/cortina/gemini.c:gmac_setup_phy() {
phy = of_phy_get_and_connect(netdev,
dev->of_node,
gmac_adjust_link);
if (!phy)
return -ENODEV;
...
}
of_phy_get_and_connect() in drivers/net/mdio/of_mdio.c returns NULL for
every failure. That includes of_phy_register_fixed_link() errors. It also
includes of_phy_connect() returning NULL when of_phy_find_device() has not
found the PHY yet. Apart from -ENODEV, gmac_setup_phy() only returns
-EINVAL, for an unsupported interface mode.
Say the MDIO bus probes after the port. Most Gemini boards reference a
virtual,mdio-gpio bus through phy-handle, and CONFIG_MDIO_GPIO=m makes a
late bus more likely. Wouldn't this path then always go to irq_only, set
port->netdev to NULL and return 0?
After that the port is bound without a netdev, and the driver core won't
retry the probe. suppress_bind_attrs = true blocks rebinding from sysfs.
fw_devlink does not order probes on phy-handle, so nothing holds the port
back until the PHY exists.
The -EPROBE_DEFER check after register_netdevice() looks unreachable in
practice too:
if (ret) {
phy_disconnect(netdev->phydev);
dev_err(dev, "netdev registration failed: %d\n", ret);
if (ret == -EPROBE_DEFER)
goto unprepare;
goto irq_only;
}
The commit message says:
Continue to propagate deferred-probe and mandatory resource
errors.
For PHY setup, though, no deferral value seems to reach these checks.
Could gmac_setup_phy() return -EPROBE_DEFER itself? One way would be to
parse phy-handle and return -EPROBE_DEFER when of_phy_find_device()
returns NULL.
Before this patch, the same case returned -ENODEV and the interface was
also lost. So the outcome is no worse, but the new deferral branches don't
seem to do what the commit message describes.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-gemini-ethernet-fixes-3-v7-0-9c32a1a46aec%40kernel.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH net-next v7 03/18] net: ethernet: cortina: Drain free queue IRQ before resize
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 01/18] net: ethernet: cortina: Keep shared free queue parent-owned Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 02/18] net: ethernet: cortina: Keep ports bound for shared IRQ Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 04/18] net: ethernet: cortina: Normalize free queue pointers on rebuild Linus Walleij
` (14 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The software free queue interrupt remains registered while both netdevs
are down. Merely masking the device source does not stop a threaded
handler which was already woken. That handler can refill the queue while
resize frees its ring and page metadata, or re-enable the source after
teardown.
The free queue interrupt is routed through port 1, but both port IRQs
register the global handler. Ignore the source on port 0, then disable the
port 1 Linux IRQ for the entire resize. disable_irq() drains hard and
threaded handlers and prevents a new thread from being woken.
After the drain, mask the device source and rebuild the queue. If setup
succeeds, re-enable the source before the Linux IRQ. On failure, keep the
source masked, clear freed queue state and restore the Linux IRQ so a later
open retries setup safely.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 53 +++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 22c3b6b8e2db..5fdbe987dc97 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -986,6 +986,8 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
}
kfree(geth->freeq_pages);
+ geth->freeq_pages = NULL;
+ geth->num_freeq_pages = 0;
err_freeq:
dma_free_coherent(geth->dev,
sizeof(*geth->freeq_ring) << geth->freeq_order,
@@ -1026,20 +1028,37 @@ static void geth_cleanup_freeq(struct gemini_ethernet *geth)
}
kfree(geth->freeq_pages);
+ geth->freeq_pages = NULL;
+ geth->num_freeq_pages = 0;
dma_free_coherent(geth->dev,
sizeof(*geth->freeq_ring) << geth->freeq_order,
geth->freeq_ring, geth->freeq_dma_base);
+ geth->freeq_ring = NULL;
+}
+
+static void geth_set_freeq_irq(struct gemini_ethernet *geth, bool enable)
+{
+ unsigned long flags;
+ u32 val;
+
+ spin_lock_irqsave(&geth->irq_lock, flags);
+ val = readl(geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG);
+ if (enable)
+ val |= SWFQ_EMPTY_INT_BIT;
+ else
+ val &= ~SWFQ_EMPTY_INT_BIT;
+ writel(val, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG);
+ spin_unlock_irqrestore(&geth->irq_lock, flags);
}
/**
* geth_resize_freeq() - resize the software queue depth
* @port: the port requesting the change
*
- * This gets called at least once during probe() so the device queue gets
- * "resized" from the hardware defaults. Since both ports/net devices share
- * the same hardware queue, some synchronization between the ports is
- * needed.
+ * This is called when a port is opened or its RX ring size changes. Since
+ * both ports/net devices share the same hardware queue, some synchronization
+ * between the ports is needed.
*/
static int geth_resize_freeq(struct gemini_ethernet_port *port)
{
@@ -1049,8 +1068,6 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
struct net_device *other_netdev;
unsigned int new_size = 0;
unsigned int new_order;
- unsigned long flags;
- u32 en;
int ret;
ASSERT_RTNL();
@@ -1087,13 +1104,8 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
if (geth->freeq_ring && geth->freeq_order == new_order)
return 0;
- spin_lock_irqsave(&geth->irq_lock, flags);
-
- /* Disable the software queue IRQs */
- en = readl(geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG);
- en &= ~SWFQ_EMPTY_INT_BIT;
- writel(en, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG);
- spin_unlock_irqrestore(&geth->irq_lock, flags);
+ disable_irq(geth->port1->irq);
+ geth_set_freeq_irq(geth, false);
/* Drop the old queue */
if (geth->freeq_ring)
@@ -1103,14 +1115,10 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
geth->freeq_order = new_order;
ret = geth_setup_freeq(geth);
- /* Restart the interrupts - NOTE if this is the first resize
- * after probe(), this is where the interrupts get turned on
- * in the first place.
- */
- spin_lock_irqsave(&geth->irq_lock, flags);
- en |= SWFQ_EMPTY_INT_BIT;
- writel(en, geth->base + GLOBAL_INTERRUPT_ENABLE_4_REG);
- spin_unlock_irqrestore(&geth->irq_lock, flags);
+ /* Enable the queue interrupt only after successful setup. */
+ if (!ret)
+ geth_set_freeq_irq(geth, true);
+ enable_irq(geth->port1->irq);
return ret;
}
@@ -2373,6 +2381,9 @@ static irqreturn_t gemini_port_irq(int irq, void *data)
irqreturn_t ret = IRQ_NONE;
u32 val, en;
+ if (!port->id)
+ return IRQ_NONE;
+
geth = port->geth;
spin_lock(&geth->irq_lock);
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 04/18] net: ethernet: cortina: Normalize free queue pointers on rebuild
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (2 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 03/18] net: ethernet: cortina: Drain free queue IRQ before resize Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-11 23:10 ` netdev-bot+sashiko
2026-11-10 22:13 ` [PATCH net-next v7 05/18] net: ethernet: cortina: Correct free queue DMA mappings Linus Walleij
` (13 subsequent siblings)
17 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The free queue read pointer is owned by hardware and can retain a
position from the previous ring when the queue is rebuilt at a smaller
order. Using an unmasked pointer to index the replacement page table can
then access it out of bounds. A replacement ring which does not contain
the raw retained pointer can also let hardware address past the new
allocation.
Mask software page indices to the active queue order. Keep the
replacement ring large enough for the retained hardware pointer and
initialize the software-owned write pointer from it before filling the
ring. This represents an empty queue without trying to write the
read-only pointer.
A requested shrink may therefore retain a larger shared queue. A high
pointer inherited from firmware can impose the same allocation floor.
If allocation fails, leave the queue disabled and return the error;
retrying below that floor would not safely contain the retained pointer.
Removing this restriction requires verified hardware pointer-reset or
truncation behaviour.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 5fdbe987dc97..fa51f2b54073 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -887,6 +887,7 @@ static unsigned int geth_fill_freeq(struct gemini_ethernet *geth, bool refill)
rw.bits32 = readl(geth->base + GLOBAL_SWFQ_RWPTR_REG);
pn = (refill ? rw.bits.wptr : rw.bits.rptr) >> fpp_order;
+ pn &= m_pn;
epn = (rw.bits.rptr >> fpp_order) - 1;
epn &= m_pn;
@@ -936,6 +937,7 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
union dma_skb_size skbsz;
unsigned int filled;
unsigned int pn;
+ u16 rptr;
geth->freeq_ring = dma_alloc_coherent(geth->dev,
sizeof(*geth->freeq_ring) << geth->freeq_order,
@@ -958,6 +960,10 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
if (!geth_freeq_alloc_map_page(geth, pn))
goto err_freeq_alloc;
+ /* Match the software pointer to the retained hardware pointer. */
+ rptr = GET_RPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG);
+ SET_WPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG, rptr & (len - 1));
+
filled = geth_fill_freeq(geth, false);
if (!filled)
goto err_freeq_alloc;
@@ -1068,6 +1074,7 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
struct net_device *other_netdev;
unsigned int new_size = 0;
unsigned int new_order;
+ u16 rptr;
int ret;
ASSERT_RTNL();
@@ -1099,6 +1106,10 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
}
new_order = min(15, ilog2(new_size - 1) + 1);
+ rptr = GET_RPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG);
+ if (rptr >= BIT(15))
+ return -EIO;
+ new_order = max_t(unsigned int, new_order, fls(rptr));
dev_dbg(geth->dev, "set shared queue to size %d order %d\n",
new_size, new_order);
if (geth->freeq_ring && geth->freeq_order == new_order)
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH net-next v7 04/18] net: ethernet: cortina: Normalize free queue pointers on rebuild
2026-11-10 22:13 ` [PATCH net-next v7 04/18] net: ethernet: cortina: Normalize free queue pointers on rebuild Linus Walleij
@ 2026-11-11 23:10 ` netdev-bot+sashiko
0 siblings, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-11-11 23:10 UTC (permalink / raw)
To: linusw
Cc: ulli.kroll, andrew+netdev, davem, kuba, pabeni, mirq-linux,
mhun512, edumazet, netdev
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] geth_resize_freeq() now sets a minimum queue order from the
hardware read pointer: `new_order = max_t(unsigned int, new_order,…
--
net: ethernet: cortina: Normalize free queue pointers on rebuild
This commit masks the software free queue page indices to the active
queue order. It also sets the software write pointer from the retained
hardware read pointer before a rebuilt ring is filled. Finally,
geth_resize_freeq() now raises the shared free queue order so the
replacement ring is big enough to hold the retained read pointer.
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index 5fdbe987dc975..fa51f2b54073f 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
[ ... ]
> @@ -1099,6 +1106,10 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
> }
>
> new_order = min(15, ilog2(new_size - 1) + 1);
> + rptr = GET_RPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG);
> + if (rptr >= BIT(15))
> + return -EIO;
> + new_order = max_t(unsigned int, new_order, fls(rptr));
[Severity: Medium]
Can this floor push the shared free queue up to order 15 whatever ring
sizes were requested?
Any retained read pointer from 16384 to 32767 gives fls(rptr) == 15. At
order 15 the ring has 32768 descriptors. With 4K pages and 2K fragments,
geth_setup_freeq() then needs 16384 pages (about 64 MiB), allocated one
at a time:
geth_resize_freeq()
geth_setup_freeq()
for (pn = 0; pn < pages; pn++)
geth_freeq_alloc_map_page()
page = alloc_page(GFP_ATOMIC);
This runs in process context under RTNL, but GFP_ATOMIC cannot do direct
reclaim. Gemini boards usually have 32 to 128 MiB of RAM. A later patch
in this series, "Scale Gemini RX queues to system memory", seems to
address the memory pressure from these queues.
The pointer can be inherited from firmware, or carried over from an
earlier driver instance across rmmod/insmod. If the allocation then
fails, freeq_ring stays NULL. DMA is stopped, so the hardware read
pointer does not move.
Won't every later gmac_open() or ethtool -G compute the same floor and
fail again? gmac_open() treats any error other than -EBUSY as fatal:
if (err && (err != -EBUSY)) {
netdev_err(netdev, "could not resize freeq\n");
goto err_stop_phy;
}
Both ports share this queue. Would networking on both ports stay down
until the hardware pointer changes? In the same way, a retained pointer
of BIT(15) or more makes every open return -EIO.
The commit message says:
A requested shrink may therefore retain a larger shared queue. A high
pointer inherited from firmware can impose the same allocation floor.
If allocation fails, leave the queue disabled and return the error;
It does not say that the floor can reach 64 MiB of GFP_ATOMIC page
allocations. It also does not say that the failure persists across
retries.
The design also assumes the read pointer cannot be reset. The commit
message itself says this is unverified. gemini_ethernet_init() already
writes the whole register:
writel(0, geth->base + GLOBAL_SWFQ_RWPTR_REG);
If that write does reset the read pointer, is the floor still needed for
the firmware-inherited case? If it does not, does that write have any
effect on the read pointer half?
> dev_dbg(geth->dev, "set shared queue to size %d order %d\n",
> new_size, new_order);
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-gemini-ethernet-fixes-3-v7-0-9c32a1a46aec%40kernel.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH net-next v7 05/18] net: ethernet: cortina: Correct free queue DMA mappings
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (3 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 04/18] net: ethernet: cortina: Normalize free queue pointers on rebuild Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-11 23:10 ` netdev-bot+sashiko
2026-11-10 22:13 ` [PATCH net-next v7 06/18] net: ethernet: cortina: Index free queue fragments with XArray Linus Walleij
` (12 subsequent siblings)
17 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The free queue maps complete pages and splits each mapping between its
fragment descriptors. The mapping variable is advanced while filling the
descriptors and that advanced address is then saved as the page mapping.
Replacement also reads the old address after overwriting the descriptor
with the new mapping, so it unmaps the new buffer while leaving the old
mapping live.
The same reassignment leaves replacement metadata holding the new page
base while gmac_rx() still adds PAGE_SIZE before lookup. Once a page is
replaced under RX load, lookup therefore normally fails with "could not
find mapping" and drops the frame. If the resulting key matches another
page, RX can instead attach that page to the skb and drop the wrong page
reference.
Keep the page DMA base separate from the fragment iterator and remove the
invalid replacement unmap. The replaced mapping cannot be released until
its in-flight fragments have been claimed, which is handled with their
lifetime later in the series. Use the saved DMA base and PAGE_SIZE when
unwinding setup and tearing down the current queue.
Reject mappings that cannot fit in the 32-bit hardware descriptors and
derive fragment offsets from the saved DMA base instead of assuming
page-aligned DMA addresses. Snapshot the page, DMA base and offset under
the free queue lock so refill cannot replace them between lookup and use.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 112 ++++++++++++++++++----------------
1 file changed, 58 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index fa51f2b54073..71d7e6189531 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -726,32 +726,43 @@ static int gmac_setup_rxq(struct net_device *netdev)
return 0;
}
-static struct gmac_queue_page *
-gmac_get_queue_page(struct gemini_ethernet *geth,
- struct gemini_ethernet_port *port,
- dma_addr_t addr)
+static struct page *geth_freeq_lookup(struct gemini_ethernet *geth,
+ dma_addr_t mapping,
+ unsigned int *page_offs)
{
+ unsigned int frag_len = 1 << geth->freeq_frag_order;
struct gmac_queue_page *gpage;
- dma_addr_t mapping;
+ unsigned long flags;
+ dma_addr_t page_mapping;
+ struct page *page = NULL;
int i;
- /* Only look for even pages */
- mapping = addr & PAGE_MASK;
-
+ spin_lock_irqsave(&geth->freeq_lock, flags);
if (!geth->freeq_pages) {
dev_err_ratelimited(geth->dev,
"try to get page with no page list\n");
- return NULL;
+ goto unlock;
}
/* Look up a ring buffer page from virtual mapping */
for (i = 0; i < geth->num_freeq_pages; i++) {
gpage = &geth->freeq_pages[i];
- if (gpage->mapping == mapping)
- return gpage;
+ if (!gpage->page || mapping < gpage->mapping)
+ continue;
+
+ page_mapping = gpage->mapping;
+ if (mapping - page_mapping > PAGE_SIZE - frag_len ||
+ ((mapping - page_mapping) & (frag_len - 1)))
+ continue;
+
+ page = gpage->page;
+ *page_offs = mapping - page_mapping;
+ break;
}
- return NULL;
+unlock:
+ spin_unlock_irqrestore(&geth->freeq_lock, flags);
+ return page;
}
static void gmac_cleanup_rxq(struct net_device *netdev)
@@ -759,11 +770,12 @@ static void gmac_cleanup_rxq(struct net_device *netdev)
struct gemini_ethernet_port *port = netdev_priv(netdev);
struct gemini_ethernet *geth = port->geth;
struct gmac_rxdesc *rxd = port->rxq_ring;
- static struct gmac_queue_page *gpage;
struct nontoe_qhdr __iomem *qhdr;
void __iomem *dma_reg;
void __iomem *ptr_reg;
+ unsigned int page_offs;
dma_addr_t mapping;
+ struct page *page;
union dma_rwptr rw;
unsigned int r, w;
@@ -790,14 +802,13 @@ static void gmac_cleanup_rxq(struct net_device *netdev)
if (!mapping)
continue;
- /* Freeq pointers are one page off */
- gpage = gmac_get_queue_page(geth, port, mapping + PAGE_SIZE);
- if (!gpage) {
+ page = geth_freeq_lookup(geth, mapping, &page_offs);
+ if (!page) {
dev_err(geth->dev, "could not find page\n");
continue;
}
/* Release the RX queue reference to the page */
- put_page(gpage->page);
+ put_page(page);
}
dma_free_coherent(geth->dev, sizeof(*port->rxq_ring) << port->rxq_order,
@@ -811,6 +822,7 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
struct gmac_queue_page *gpage;
unsigned int fpp_order;
unsigned int frag_len;
+ dma_addr_t page_mapping;
dma_addr_t mapping;
struct page *page;
int i;
@@ -820,9 +832,17 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
if (!page)
return NULL;
- mapping = dma_map_single(geth->dev, page_address(page),
- PAGE_SIZE, DMA_FROM_DEVICE);
- if (dma_mapping_error(geth->dev, mapping)) {
+ page_mapping = dma_map_single(geth->dev, page_address(page),
+ PAGE_SIZE, DMA_FROM_DEVICE);
+ if (dma_mapping_error(geth->dev, page_mapping)) {
+ put_page(page);
+ return NULL;
+ }
+ if (page_mapping > U32_MAX - (PAGE_SIZE - 1)) {
+ dev_err_ratelimited(geth->dev,
+ "freeq DMA mapping exceeds 32 bits\n");
+ dma_unmap_single(geth->dev, page_mapping, PAGE_SIZE,
+ DMA_FROM_DEVICE);
put_page(page);
return NULL;
}
@@ -835,31 +855,25 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
*/
frag_len = 1 << geth->freeq_frag_order; /* Usually 2048 */
fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
+ gpage = &geth->freeq_pages[pn];
+ if (gpage->page)
+ put_page(gpage->page);
+
+ /* Then put our new mapping into the page table */
+ gpage->mapping = page_mapping;
+ gpage->page = page;
+
freeq_entry = geth->freeq_ring + (pn << fpp_order);
dev_dbg(geth->dev, "allocate page %d fragment length %d fragments per page %d, freeq entry %p\n",
- pn, frag_len, (1 << fpp_order), freeq_entry);
+ pn, frag_len, (1 << fpp_order), freeq_entry);
+ mapping = page_mapping;
for (i = (1 << fpp_order); i > 0; i--) {
freeq_entry->word2.buf_adr = mapping;
freeq_entry++;
mapping += frag_len;
}
-
- /* If the freeq entry already has a page mapped, then unmap it. */
- gpage = &geth->freeq_pages[pn];
- if (gpage->page) {
- mapping = geth->freeq_ring[pn << fpp_order].word2.buf_adr;
- dma_unmap_single(geth->dev, mapping, frag_len, DMA_FROM_DEVICE);
- /* This should be the last reference to the page so it gets
- * released
- */
- put_page(gpage->page);
- }
-
- /* Then put our new mapping into the page table */
- dev_dbg(geth->dev, "page %d, DMA addr: %08x, page %p\n",
- pn, (unsigned int)mapping, page);
- gpage->mapping = mapping;
- gpage->page = page;
+ dev_dbg(geth->dev, "page %d, DMA addr: %pad, page %p\n",
+ pn, &page_mapping, page);
return page;
}
@@ -930,7 +944,6 @@ static unsigned int geth_fill_freeq(struct gemini_ethernet *geth, bool refill)
static int geth_setup_freeq(struct gemini_ethernet *geth)
{
unsigned int fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
- unsigned int frag_len = 1 << geth->freeq_frag_order;
unsigned int len = 1 << geth->freeq_order;
unsigned int pages = len >> fpp_order;
union queue_threshold qt;
@@ -982,12 +995,11 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
err_freeq_alloc:
while (pn > 0) {
struct gmac_queue_page *gpage;
- dma_addr_t mapping;
--pn;
- mapping = geth->freeq_ring[pn << fpp_order].word2.buf_adr;
- dma_unmap_single(geth->dev, mapping, frag_len, DMA_FROM_DEVICE);
gpage = &geth->freeq_pages[pn];
+ dma_unmap_single(geth->dev, gpage->mapping, PAGE_SIZE,
+ DMA_FROM_DEVICE);
put_page(gpage->page);
}
@@ -1009,7 +1021,6 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
static void geth_cleanup_freeq(struct gemini_ethernet *geth)
{
unsigned int fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
- unsigned int frag_len = 1 << geth->freeq_frag_order;
unsigned int len = 1 << geth->freeq_order;
unsigned int pages = len >> fpp_order;
unsigned int pn;
@@ -1023,12 +1034,10 @@ static void geth_cleanup_freeq(struct gemini_ethernet *geth)
for (pn = 0; pn < pages; pn++) {
struct gmac_queue_page *gpage;
- dma_addr_t mapping;
-
- mapping = geth->freeq_ring[pn << fpp_order].word2.buf_adr;
- dma_unmap_single(geth->dev, mapping, frag_len, DMA_FROM_DEVICE);
gpage = &geth->freeq_pages[pn];
+ dma_unmap_single(geth->dev, gpage->mapping, PAGE_SIZE,
+ DMA_FROM_DEVICE);
while (page_ref_count(gpage->page) > 0)
put_page(gpage->page);
}
@@ -1486,7 +1495,6 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
unsigned int consumed = 0;
unsigned int frame_len, frag_len;
struct gmac_rxdesc *rx = NULL;
- struct gmac_queue_page *gpage;
unsigned int received = 0;
bool dropping = port->rx_dropping;
union gmac_rxdesc_0 word0;
@@ -1523,8 +1531,6 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
frag_len = word0.bits.buffer_size;
frame_len = word1.bits.byte_count;
- page_offs = mapping & ~PAGE_MASK;
-
if (word3.bits32 & SOF_BIT) {
if (skb) {
napi_free_frags(&port->napi);
@@ -1543,14 +1549,12 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
goto err_drop;
}
- /* Freeq pointers are one page off */
- gpage = gmac_get_queue_page(geth, port, mapping + PAGE_SIZE);
- if (!gpage) {
+ page = geth_freeq_lookup(geth, mapping, &page_offs);
+ if (!page) {
dev_err_ratelimited(geth->dev,
"could not find mapping\n");
goto err_drop;
}
- page = gpage->page;
if (word3.bits32 & SOF_BIT) {
skb = gmac_skb_if_good_frame(port, word0, frame_len);
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH net-next v7 05/18] net: ethernet: cortina: Correct free queue DMA mappings
2026-11-10 22:13 ` [PATCH net-next v7 05/18] net: ethernet: cortina: Correct free queue DMA mappings Linus Walleij
@ 2026-11-11 23:10 ` netdev-bot+sashiko
0 siblings, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-11-11 23:10 UTC (permalink / raw)
To: linusw
Cc: ulli.kroll, andrew+netdev, davem, kuba, pabeni, mirq-linux,
mhun512, edumazet, netdev
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 (5) · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] geth_freeq_lookup() now holds the global geth->freeq_lock with
spin_lock_irqsave() while it scans all num_freeq_pages entries…
Pre-existing issues:
- [Medium] After this patch, geth_freeq_alloc_map_page() still overwrites
geth->freeq_pages[pn].page and .mapping (gpage->mapping =…
- [Medium] When a free-queue page is replaced,
geth_freeq_alloc_map_page() overwrites the old page's PAGE_SIZE…
- [Medium] In this patch, gmac_rx() takes frag_len from the 16-bit
hardware fields word0.bits.buffer_size and word1.bits.byte_count.
- [Medium] geth_cleanup_freeq() drains references with 'while
(page_ref_count(gpage->page) > 0) put_page(gpage->page);'.
- [Medium] Free-queue pages use long-lived DMA_FROM_DEVICE streaming
mappings.
--
net: ethernet: cortina: Correct free queue DMA mappings
This patch keeps the free queue page DMA base separate from the fragment
iterator, drops the replacement path's unmap of the wrong buffer, and uses
the saved base and PAGE_SIZE for setup unwind and teardown. It replaces
gmac_get_queue_page() with geth_freeq_lookup(), which derives the fragment
offset from the saved base and snapshots page, base and offset under
freeq_lock. It also rejects mappings that cannot fit in the 32-bit
hardware descriptors.
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index fa51f2b54073f..71d7e61895312 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -726,32 +726,43 @@ static int gmac_setup_rxq(struct net_device *netdev)
[ ... ]
> + spin_lock_irqsave(&geth->freeq_lock, flags);
> if (!geth->freeq_pages) {
> dev_err_ratelimited(geth->dev,
> "try to get page with no page list\n");
> - return NULL;
> + goto unlock;
> }
>
> /* Look up a ring buffer page from virtual mapping */
> for (i = 0; i < geth->num_freeq_pages; i++) {
> gpage = &geth->freeq_pages[i];
> - if (gpage->mapping == mapping)
> - return gpage;
> + if (!gpage->page || mapping < gpage->mapping)
> + continue;
> +
> + page_mapping = gpage->mapping;
> + if (mapping - page_mapping > PAGE_SIZE - frag_len ||
> + ((mapping - page_mapping) & (frag_len - 1)))
> + continue;
> +
> + page = gpage->page;
> + *page_offs = mapping - page_mapping;
> + break;
> }
>
> - return NULL;
> +unlock:
> + spin_unlock_irqrestore(&geth->freeq_lock, flags);
> + return page;
> }
[Severity: Medium]
geth_freeq_lookup() now takes the shared geth->freeq_lock with
spin_lock_irqsave() and holds it while it scans all num_freeq_pages
entries one by one. gmac_rx() calls it once per RX descriptor. With
default sizing that is about 1024 iterations per descriptor with IRQs
off, and it contends with refill and with the other port.
Is this extra IRQ-off latency on the RX hot path acceptable for this
intermediate state?
The next patch in the series, "net: ethernet: cortina: Index free queue
fragments with XArray", replaces the scan with a single xa_load() under
the lock. The cost only exists between these two patches.
[ ... ]
> @@ -835,31 +855,25 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
> */
> frag_len = 1 << geth->freeq_frag_order; /* Usually 2048 */
> fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
> + gpage = &geth->freeq_pages[pn];
> + if (gpage->page)
> + put_page(gpage->page);
[Severity: Medium]
This isn't a bug introduced by this patch, but when a slot is replaced
here, the old page's PAGE_SIZE streaming mapping in gpage->mapping is
overwritten without being unmapped. geth_cleanup_freeq() and the
geth_setup_freeq() unwind path only unmap the slot's current
gpage->mapping.
Does the replaced DMA mapping leak? Can the page also go back to the
allocator while it is still mapped?
The old code never unmapped the old mapping either. It unmapped the first
fragment of the new buffer instead. The commit message also defers this
to later in the series.
At the end of the series this is handled: geth_freeq_recycle_page() calls
dma_unmap_single_attrs() before it drops a page that is still referenced,
geth_freeq_release_pages() unmaps every live slot, and slots are no longer
overwritten while their fragments are in flight. These changes come from
"net: ethernet: cortina: Preserve in-flight free queue pages" and
"Recycle claimed free queue pages".
> +
> + /* Then put our new mapping into the page table */
> + gpage->mapping = page_mapping;
> + gpage->page = page;
[Severity: Medium]
This is a pre-existing issue, but the slot metadata is still overwritten
here when geth_fill_freeq() replaces a page whose page_ref_count() > 1.
RX descriptors can still hold fragment addresses from the replaced page,
and geth_freeq_lookup() only matches each slot's current base:
geth_fill_freeq()
geth_freeq_alloc_map_page()
gpage->mapping = page_mapping; <- old base is forgotten
gmac_rx()
geth_freeq_lookup() <- NULL for an old-page fragment
goto err_drop; <- page == NULL, no put_page()
Could this drop the frame and leak that fragment's page reference?
gmac_cleanup_rxq() would hit the same NULL return and skip put_page().
The baseline behaved the same way, and the commit message defers the
lifetime of in-flight fragments to later in the series.
At the end of the series, "net: ethernet: cortina: Preserve in-flight
free queue pages" fixes this. Metadata slots are allocated independently
of the free queue position through geth_freeq_alloc_slot(), and each
fragment address stays in geth->freeq_mappings until geth_freeq_claim()
claims it.
[ ... ]
> @@ -1023,12 +1034,10 @@ static void geth_cleanup_freeq(struct gemini_ethernet *geth)
>
> for (pn = 0; pn < pages; pn++) {
> struct gmac_queue_page *gpage;
> - dma_addr_t mapping;
> -
> - mapping = geth->freeq_ring[pn << fpp_order].word2.buf_adr;
> - dma_unmap_single(geth->dev, mapping, frag_len, DMA_FROM_DEVICE);
>
> gpage = &geth->freeq_pages[pn];
> + dma_unmap_single(geth->dev, gpage->mapping, PAGE_SIZE,
> + DMA_FROM_DEVICE);
> while (page_ref_count(gpage->page) > 0)
> put_page(gpage->page);
[Severity: Medium]
This isn't a bug introduced by this patch, but this while loop drops
references the driver doesn't own, including references held by skbs
that were already handed to the stack. It also re-reads the refcount
after the page may already have been freed.
Can this over-put a page, or read a freed one, at queue teardown (ndo_stop
or a ring resize)?
At the end of the series, geth_cleanup_freeq() calls
geth_freeq_release_pages() instead. That function puts exactly
gpage->fragments outstanding hardware references plus the single base
reference ("net: ethernet: cortina: Preserve in-flight free queue pages").
[ ... ]
> @@ -1523,8 +1531,6 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
>
> frag_len = word0.bits.buffer_size;
> frame_len = word1.bits.byte_count;
> - page_offs = mapping & ~PAGE_MASK;
> -
[Severity: Medium]
This isn't a bug introduced by this patch, but frag_len and frame_len come
straight from the 16-bit hardware fields word0.bits.buffer_size and
word1.bits.byte_count. Later, frag_len gets unchecked unsigned
adjustments (frag_len -= NET_IP_ALIGN; frag_len = frame_len - skb->len)
and is then passed to skb_fill_page_desc(). geth_freeq_lookup() only
validates the offset.
Could a malformed descriptor attach a page fragment that runs past the
free queue fragment or past the end of the page?
This is fixed later in the series by "net: ethernet: cortina: Validate RX
fragment lengths". That patch rejects frag_len < NET_IP_ALIGN, frame_len
< ETH_ZLEN or frame_len < skb->len, zero-length descriptors, and frag_len
values larger than the space left in the fragment or the page.
> if (word3.bits32 & SOF_BIT) {
> if (skb) {
> napi_free_frags(&port->napi);
> @@ -1543,14 +1549,12 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
> goto err_drop;
> }
>
> - /* Freeq pointers are one page off */
> - gpage = gmac_get_queue_page(geth, port, mapping + PAGE_SIZE);
> - if (!gpage) {
> + page = geth_freeq_lookup(geth, mapping, &page_offs);
> + if (!page) {
> dev_err_ratelimited(geth->dev,
> "could not find mapping\n");
> goto err_drop;
> }
> - page = gpage->page;
[Severity: Medium]
This is a pre-existing issue, but the free queue pages use long-lived
DMA_FROM_DEVICE streaming mappings. gmac_rx() hands fragments written by
the device to the stack without any dma_sync_single_*_for_cpu() call.
geth_fill_freeq() also re-posts pages to the device without
dma_sync_single_for_device().
Doesn't this break the DMA API ownership rules for these buffers?
The CPU sync is a no-op on the Gemini FA526, so there should be no
visible effect on the target hardware. Other implementations could see
stale data.
At the end of the series this is handled:
- geth_freeq_claim() calls dma_sync_single_range_for_cpu() for each
claimed fragment.
- geth_freeq_post_page() calls dma_sync_single_for_device() when it
recycles a page.
- geth_freeq_release_pages() syncs fragments still owned by the
hardware before unmapping with DMA_ATTR_SKIP_CPU_SYNC.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-gemini-ethernet-fixes-3-v7-0-9c32a1a46aec%40kernel.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH net-next v7 06/18] net: ethernet: cortina: Index free queue fragments with XArray
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (4 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 05/18] net: ethernet: cortina: Correct free queue DMA mappings Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 07/18] net: ethernet: cortina: Preserve in-flight free queue pages Linus Walleij
` (11 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
RX descriptors return the DMA address of the free queue fragment used by
the hardware. The driver currently recovers its page by scanning every
free queue page and comparing address ranges.
This isn't very efficient...
Instead index each fragment in an XArray using its DMA address in
fragment units.
Populate and erase the fragment index under the free queue lock. The
lookup remains serialized with refill and validates the exact fragment
address before returning the page and offset snapshot.
The per-fragment index is a prerequisite for the following
"Preserve in-flight free queue pages" fix, which keeps each mapping until
its RX descriptor is claimed. Include this patch when backporting that
fix and its subsequent recycling changes.
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 105 ++++++++++++++++++++++++----------
1 file changed, 76 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 71d7e6189531..add1d3e563b6 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -38,6 +38,7 @@
#include <linux/ethtool.h>
#include <linux/tcp.h>
#include <linux/u64_stats_sync.h>
+#include <linux/xarray.h>
#include <linux/in.h>
#include <linux/ip.h>
@@ -167,6 +168,7 @@ struct gemini_ethernet {
struct gmac_rxdesc *freeq_ring;
dma_addr_t freeq_dma_base;
struct gmac_queue_page *freeq_pages;
+ struct xarray freeq_mappings;
unsigned int num_freeq_pages;
spinlock_t freeq_lock; /* Locks queue from reentrance */
};
@@ -726,43 +728,47 @@ static int gmac_setup_rxq(struct net_device *netdev)
return 0;
}
+static unsigned long
+geth_freeq_mapping_index(const struct gemini_ethernet *geth,
+ dma_addr_t mapping)
+{
+ return (unsigned long)(mapping >> geth->freeq_frag_order);
+}
+
static struct page *geth_freeq_lookup(struct gemini_ethernet *geth,
dma_addr_t mapping,
unsigned int *page_offs)
{
unsigned int frag_len = 1 << geth->freeq_frag_order;
struct gmac_queue_page *gpage;
+ unsigned long index;
unsigned long flags;
dma_addr_t page_mapping;
- struct page *page = NULL;
- int i;
-
- spin_lock_irqsave(&geth->freeq_lock, flags);
- if (!geth->freeq_pages) {
- dev_err_ratelimited(geth->dev,
- "try to get page with no page list\n");
- goto unlock;
- }
-
- /* Look up a ring buffer page from virtual mapping */
- for (i = 0; i < geth->num_freeq_pages; i++) {
- gpage = &geth->freeq_pages[i];
- if (!gpage->page || mapping < gpage->mapping)
- continue;
-
- page_mapping = gpage->mapping;
- if (mapping - page_mapping > PAGE_SIZE - frag_len ||
- ((mapping - page_mapping) & (frag_len - 1)))
- continue;
+ struct page *page;
+ bool valid;
- page = gpage->page;
- *page_offs = mapping - page_mapping;
- break;
- }
+ index = geth_freeq_mapping_index(geth, mapping);
-unlock:
+ spin_lock_irqsave(&geth->freeq_lock, flags);
+ gpage = xa_load(&geth->freeq_mappings, index);
+ if (!gpage || !gpage->page)
+ goto err_unlock;
+
+ page = gpage->page;
+ page_mapping = gpage->mapping;
+ valid = mapping >= page_mapping &&
+ mapping - page_mapping <= PAGE_SIZE - frag_len &&
+ !((mapping - page_mapping) & (frag_len - 1));
+ if (!valid)
+ goto err_unlock;
+
+ *page_offs = mapping - page_mapping;
spin_unlock_irqrestore(&geth->freeq_lock, flags);
return page;
+
+err_unlock:
+ spin_unlock_irqrestore(&geth->freeq_lock, flags);
+ return NULL;
}
static void gmac_cleanup_rxq(struct net_device *netdev)
@@ -821,12 +827,16 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
struct gmac_rxdesc *freeq_entry;
struct gmac_queue_page *gpage;
unsigned int fpp_order;
+ unsigned int fragments;
unsigned int frag_len;
dma_addr_t page_mapping;
dma_addr_t mapping;
struct page *page;
+ int ret;
int i;
+ lockdep_assert_held(&geth->freeq_lock);
+
/* First allocate and DMA map a single page */
page = alloc_page(GFP_ATOMIC);
if (!page)
@@ -855,9 +865,26 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
*/
frag_len = 1 << geth->freeq_frag_order; /* Usually 2048 */
fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
+ fragments = 1 << fpp_order;
+
gpage = &geth->freeq_pages[pn];
- if (gpage->page)
+ for (i = 0; i < fragments; i++) {
+ mapping = page_mapping + i * frag_len;
+ ret = xa_insert(&geth->freeq_mappings,
+ geth_freeq_mapping_index(geth, mapping),
+ gpage, GFP_ATOMIC);
+ if (ret)
+ goto err_mappings;
+ }
+
+ if (gpage->page) {
+ for (i = 0; i < fragments; i++) {
+ mapping = gpage->mapping + i * frag_len;
+ xa_erase(&geth->freeq_mappings,
+ geth_freeq_mapping_index(geth, mapping));
+ }
put_page(gpage->page);
+ }
/* Then put our new mapping into the page table */
gpage->mapping = page_mapping;
@@ -876,6 +903,17 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
pn, &page_mapping, page);
return page;
+
+err_mappings:
+ while (i--) {
+ mapping = page_mapping + i * frag_len;
+ xa_erase(&geth->freeq_mappings,
+ geth_freeq_mapping_index(geth, mapping));
+ }
+ dma_unmap_single(geth->dev, page_mapping, PAGE_SIZE,
+ DMA_FROM_DEVICE);
+ put_page(page);
+ return NULL;
}
/**
@@ -948,6 +986,8 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
unsigned int pages = len >> fpp_order;
union queue_threshold qt;
union dma_skb_size skbsz;
+ unsigned long flags;
+ struct page *page;
unsigned int filled;
unsigned int pn;
u16 rptr;
@@ -962,16 +1002,20 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
goto err_freeq;
}
- /* Allocate a mapping to page look-up index */
+ /* Allocate page metadata referenced by the mapping index */
geth->freeq_pages = kzalloc_objs(*geth->freeq_pages, pages);
if (!geth->freeq_pages)
goto err_freeq;
geth->num_freeq_pages = pages;
dev_info(geth->dev, "allocate %d pages for queue\n", pages);
- for (pn = 0; pn < pages; pn++)
- if (!geth_freeq_alloc_map_page(geth, pn))
+ for (pn = 0; pn < pages; pn++) {
+ spin_lock_irqsave(&geth->freeq_lock, flags);
+ page = geth_freeq_alloc_map_page(geth, pn);
+ spin_unlock_irqrestore(&geth->freeq_lock, flags);
+ if (!page)
goto err_freeq_alloc;
+ }
/* Match the software pointer to the retained hardware pointer. */
rptr = GET_RPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG);
@@ -1002,6 +1046,7 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
DMA_FROM_DEVICE);
put_page(gpage->page);
}
+ xa_destroy(&geth->freeq_mappings);
kfree(geth->freeq_pages);
geth->freeq_pages = NULL;
@@ -1041,6 +1086,7 @@ static void geth_cleanup_freeq(struct gemini_ethernet *geth)
while (page_ref_count(gpage->page) > 0)
put_page(gpage->page);
}
+ xa_destroy(&geth->freeq_mappings);
kfree(geth->freeq_pages);
geth->freeq_pages = NULL;
@@ -2787,6 +2833,7 @@ static int gemini_ethernet_probe(struct platform_device *pdev)
spin_lock_init(&geth->irq_lock);
spin_lock_init(&geth->freeq_lock);
+ xa_init(&geth->freeq_mappings);
/* Quiesce firmware state before child IRQ handlers are registered. */
gemini_ethernet_disable_irqs(geth);
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 07/18] net: ethernet: cortina: Preserve in-flight free queue pages
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (5 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 06/18] net: ethernet: cortina: Index free queue fragments with XArray Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-11 23:10 ` netdev-bot+sashiko
2026-11-10 22:13 ` [PATCH net-next v7 08/18] net: ethernet: cortina: Rotate free queue page allocation Linus Walleij
` (10 subsequent siblings)
17 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The software free queue and the two per-port RX queues have independent
lifetimes. A free queue ring position becomes reusable once hardware has
taken its buffer address, but the resulting RX descriptor can remain
unprocessed for longer.
With the default 2K fragments, the sequence is:
1. Free queue entries 2n and 2n + 1 contain the first and second halves
of page A.
2. Hardware consumes those entries and copies their DMA addresses into RX
descriptors, potentially in the RX queues of different ports.
3. The free queue read pointer advances, making entries 2n and 2n + 1
available for refill.
4. NAPI may not yet have processed the RX descriptors referring to page A,
so their DMA-to-page association must remain valid.
5. Refilling the same free queue positions with page B must therefore not
replace the metadata describing page A.
The driver currently indexes page metadata by free queue position. Step 5
therefore overwrites page A's association with page B. When NAPI later
processes an old descriptor containing page A's DMA address, the driver can
no longer find the mapping. This may explain long-standing reports of the
driver failing to find RX mappings.
This is not just caused by splitting a 4K page into two 2K
fragments: the metadata would also have to outlive its free queue
position with one full-page buffer per entry, but the 2K split makes
the lifetime more involved because the two halves of one page are
independently consumed and may be waiting in different port RX queues.
The page cannot be unmapped until both fragment references have been
claimed.
Keep free queue positions in descriptor units when posting pages. The
retained hardware pointer need not be aligned to the number of fragments
per page; rounding it down would place one fragment behind the hardware
read pointer and retain its mapping and reference until teardown. Wrap
individual descriptor entries instead.
Allocate page metadata slots independently of free queue positions and
track occupied slots with a bitmap. Keep each exact fragment DMA address in
the XArray until software claims its descriptor.
The pages use streaming DMA_FROM_DEVICE mappings. Synchronize each
fragment when claimed and unmap the page with DMA_ATTR_SKIP_CPU_SYNC after
both are claimed. This has no known visible effect on Gemini's FA526,
where CPU sync is currently a no-op, but follows the DMA API and avoids
resynchronizing a sibling on other implementations. During teardown,
synchronize only fragments still owned by hardware.
Give each posted fragment its own page reference. Queue and error cleanup
can then release precisely the outstanding hardware references without
dropping references held by delivered skbs.
Allocate and DMA-map each candidate page before taking the free queue
lock, then re-read the hardware pointers and publish one complete page at
a time. The old refill path reused pages by queue position and only
allocated replacements once the queued fill fell to 64 descriptors or
fewer. This change allocates replacements to fill the ring, but limits
each call to one usable ring's worth of fragments and a finite number of
iterations.
The iteration allowance covers the metadata slots and one ring of pages,
so later recycling retries remain bounded too, independently of hardware
progress.
The following "Recycle claimed free queue pages" patch restores recycling
independently of queue position. Backport both changes together with
their prerequisites, including "Index free queue fragments with XArray".
Size the metadata pool for the shared free queue and both registered RX
rings. Include that capacity in resize decisions and reuse excess ring or
metadata capacity only while the peer is running. Rebuild on same-order
metadata shrink when both ports are idle, and roll back rejected RX ring
changes. This prevents same-order ring growth from exhausting page slots
and lets downsizing release surplus pages when both ports are idle.
Reject queue geometries that cannot post one complete page before tearing
down the existing queue. Require the initial fill to populate every usable
queue entry. If mapping or metadata allocation stops the fill early,
rewind the hardware write pointer, release the partial population and fail
setup.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 382 +++++++++++++++++++++-------------
1 file changed, 240 insertions(+), 142 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index add1d3e563b6..bdbbc6d7844b 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -14,6 +14,7 @@
* Gary Chen & Ch Hsu Storlink Semiconductor
*/
#include <linux/kernel.h>
+#include <linux/bitmap.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/net.h>
@@ -21,6 +22,7 @@
#include <linux/spinlock.h>
#include <linux/slab.h>
#include <linux/dma-mapping.h>
+#include <linux/circ_buf.h>
#include <linux/cache.h>
#include <linux/interrupt.h>
#include <linux/reset.h>
@@ -89,11 +91,13 @@ MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
/**
* struct gmac_queue_page - page buffer per-page info
* @page: the page struct
- * @mapping: the dma address handle
+ * @mapping: DMA address of the first fragment
+ * @fragments: number of fragments not yet claimed from the hardware
*/
struct gmac_queue_page {
struct page *page;
dma_addr_t mapping;
+ unsigned int fragments;
};
struct gmac_txq {
@@ -170,6 +174,7 @@ struct gemini_ethernet {
struct gmac_queue_page *freeq_pages;
struct xarray freeq_mappings;
unsigned int num_freeq_pages;
+ unsigned long *freeq_page_bitmap;
spinlock_t freeq_lock; /* Locks queue from reentrance */
};
@@ -735,15 +740,32 @@ geth_freeq_mapping_index(const struct gemini_ethernet *geth,
return (unsigned long)(mapping >> geth->freeq_frag_order);
}
-static struct page *geth_freeq_lookup(struct gemini_ethernet *geth,
- dma_addr_t mapping,
- unsigned int *page_offs)
+static int geth_freeq_alloc_slot(struct gemini_ethernet *geth)
+{
+ unsigned int slot;
+
+ lockdep_assert_held(&geth->freeq_lock);
+
+ slot = find_first_zero_bit(geth->freeq_page_bitmap,
+ geth->num_freeq_pages);
+ if (slot == geth->num_freeq_pages)
+ return -ENOSPC;
+
+ __set_bit(slot, geth->freeq_page_bitmap);
+
+ return slot;
+}
+
+static struct page *geth_freeq_claim(struct gemini_ethernet *geth,
+ dma_addr_t mapping,
+ unsigned int *page_offs)
{
unsigned int frag_len = 1 << geth->freeq_frag_order;
struct gmac_queue_page *gpage;
unsigned long index;
unsigned long flags;
dma_addr_t page_mapping;
+ unsigned int slot;
struct page *page;
bool valid;
@@ -751,7 +773,7 @@ static struct page *geth_freeq_lookup(struct gemini_ethernet *geth,
spin_lock_irqsave(&geth->freeq_lock, flags);
gpage = xa_load(&geth->freeq_mappings, index);
- if (!gpage || !gpage->page)
+ if (!gpage || !gpage->page || !gpage->fragments)
goto err_unlock;
page = gpage->page;
@@ -762,6 +784,21 @@ static struct page *geth_freeq_lookup(struct gemini_ethernet *geth,
if (!valid)
goto err_unlock;
+ dma_sync_single_range_for_cpu(geth->dev, page_mapping,
+ mapping - page_mapping, frag_len,
+ DMA_FROM_DEVICE);
+ xa_erase(&geth->freeq_mappings, index);
+ if (!--gpage->fragments) {
+ slot = gpage - geth->freeq_pages;
+ dma_unmap_single_attrs(geth->dev, page_mapping, PAGE_SIZE,
+ DMA_FROM_DEVICE,
+ DMA_ATTR_SKIP_CPU_SYNC);
+ gpage->page = NULL;
+ gpage->mapping = 0;
+ __clear_bit(slot, geth->freeq_page_bitmap);
+ put_page(page);
+ }
+
*page_offs = mapping - page_mapping;
spin_unlock_irqrestore(&geth->freeq_lock, flags);
return page;
@@ -808,7 +845,7 @@ static void gmac_cleanup_rxq(struct net_device *netdev)
if (!mapping)
continue;
- page = geth_freeq_lookup(geth, mapping, &page_offs);
+ page = geth_freeq_claim(geth, mapping, &page_offs);
if (!page) {
dev_err(geth->dev, "could not find page\n");
continue;
@@ -821,32 +858,22 @@ static void gmac_cleanup_rxq(struct net_device *netdev)
port->rxq_ring, port->rxq_dma_base);
}
-static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
- int pn)
+static int geth_freeq_map_page(struct gemini_ethernet *geth,
+ struct page **pagep,
+ dma_addr_t *page_mappingp)
{
- struct gmac_rxdesc *freeq_entry;
- struct gmac_queue_page *gpage;
- unsigned int fpp_order;
- unsigned int fragments;
- unsigned int frag_len;
dma_addr_t page_mapping;
- dma_addr_t mapping;
struct page *page;
- int ret;
- int i;
- lockdep_assert_held(&geth->freeq_lock);
-
- /* First allocate and DMA map a single page */
page = alloc_page(GFP_ATOMIC);
if (!page)
- return NULL;
+ return -ENOMEM;
page_mapping = dma_map_single(geth->dev, page_address(page),
PAGE_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(geth->dev, page_mapping)) {
put_page(page);
- return NULL;
+ return -ENOMEM;
}
if (page_mapping > U32_MAX - (PAGE_SIZE - 1)) {
dev_err_ratelimited(geth->dev,
@@ -854,9 +881,35 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
dma_unmap_single(geth->dev, page_mapping, PAGE_SIZE,
DMA_FROM_DEVICE);
put_page(page);
- return NULL;
+ return -EOVERFLOW;
}
+ *pagep = page;
+ *page_mappingp = page_mapping;
+
+ return 0;
+}
+
+static int geth_freeq_add_page(struct gemini_ethernet *geth, unsigned int wptr,
+ struct page *page, dma_addr_t page_mapping)
+{
+ struct gmac_rxdesc *freeq_entry;
+ struct gmac_queue_page *gpage;
+ unsigned int ring_mask;
+ unsigned int fpp_order;
+ unsigned int fragments;
+ unsigned int frag_len;
+ dma_addr_t mapping;
+ int ret;
+ int slot;
+ int i;
+
+ lockdep_assert_held(&geth->freeq_lock);
+
+ slot = geth_freeq_alloc_slot(geth);
+ if (slot < 0)
+ return slot;
+
/* The assign the page mapping (physical address) to the buffer address
* in the hardware queue. PAGE_SHIFT on ARM is 12 (1 page is 4096 bytes,
* 4k), and the default RX frag order is 11 (fragments are up 20 2048
@@ -866,8 +919,11 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
frag_len = 1 << geth->freeq_frag_order; /* Usually 2048 */
fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
fragments = 1 << fpp_order;
+ ring_mask = (1 << geth->freeq_order) - 1;
- gpage = &geth->freeq_pages[pn];
+ gpage = &geth->freeq_pages[slot];
+ gpage->page = page;
+ gpage->mapping = page_mapping;
for (i = 0; i < fragments; i++) {
mapping = page_mapping + i * frag_len;
ret = xa_insert(&geth->freeq_mappings,
@@ -876,33 +932,22 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
if (ret)
goto err_mappings;
}
+ gpage->fragments = fragments;
+ page_ref_add(page, fragments);
- if (gpage->page) {
- for (i = 0; i < fragments; i++) {
- mapping = gpage->mapping + i * frag_len;
- xa_erase(&geth->freeq_mappings,
- geth_freeq_mapping_index(geth, mapping));
- }
- put_page(gpage->page);
- }
-
- /* Then put our new mapping into the page table */
- gpage->mapping = page_mapping;
- gpage->page = page;
-
- freeq_entry = geth->freeq_ring + (pn << fpp_order);
- dev_dbg(geth->dev, "allocate page %d fragment length %d fragments per page %d, freeq entry %p\n",
- pn, frag_len, (1 << fpp_order), freeq_entry);
+ dev_dbg(geth->dev,
+ "post page at entry %u fragment length %u fragments per page %u\n",
+ wptr, frag_len, fragments);
mapping = page_mapping;
- for (i = (1 << fpp_order); i > 0; i--) {
+ for (i = 0; i < fragments; i++) {
+ freeq_entry = geth->freeq_ring + ((wptr + i) & ring_mask);
freeq_entry->word2.buf_adr = mapping;
- freeq_entry++;
mapping += frag_len;
}
- dev_dbg(geth->dev, "page %d, DMA addr: %pad, page %p\n",
- pn, &page_mapping, page);
+ dev_dbg(geth->dev, "DMA addr: %pad, page %p\n",
+ &page_mapping, page);
- return page;
+ return 0;
err_mappings:
while (i--) {
@@ -910,88 +955,147 @@ static struct page *geth_freeq_alloc_map_page(struct gemini_ethernet *geth,
xa_erase(&geth->freeq_mappings,
geth_freeq_mapping_index(geth, mapping));
}
- dma_unmap_single(geth->dev, page_mapping, PAGE_SIZE,
- DMA_FROM_DEVICE);
- put_page(page);
- return NULL;
+ gpage->page = NULL;
+ gpage->mapping = 0;
+ __clear_bit(slot, geth->freeq_page_bitmap);
+ return ret;
}
/**
* geth_fill_freeq() - Fill the freeq with empty fragments to use
* @geth: the ethernet adapter
- * @refill: whether to reset the queue by filling in all freeq entries or
- * just refill it, usually the interrupt to refill the queue happens when
- * the queue is half empty.
+ *
+ * Usually the interrupt to refill the queue happens when the queue is half
+ * empty.
*/
-static unsigned int geth_fill_freeq(struct gemini_ethernet *geth, bool refill)
+static unsigned int geth_fill_freeq(struct gemini_ethernet *geth)
{
unsigned int fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
+ unsigned int fragments = 1 << fpp_order;
+ unsigned int len = 1 << geth->freeq_order;
+ unsigned int ring_mask = len - 1;
+ unsigned int remaining = geth->num_freeq_pages + (len >> fpp_order);
unsigned int count = 0;
- unsigned int pn, epn;
+ unsigned int rptr, wptr;
unsigned long flags;
union dma_rwptr rw;
- unsigned int m_pn;
+ bool space;
- /* Mask for page */
- m_pn = (1 << (geth->freeq_order - fpp_order)) - 1;
+ /* Bound retries and post at most one ring's worth of fragments. */
+ while (remaining-- && count < len - fragments) {
+ dma_addr_t page_mapping;
+ struct page *page;
+ int ret;
- spin_lock_irqsave(&geth->freeq_lock, flags);
+ spin_lock_irqsave(&geth->freeq_lock, flags);
+ rw.bits32 = readl(geth->base + GLOBAL_SWFQ_RWPTR_REG);
+ rptr = rw.bits.rptr & ring_mask;
+ wptr = rw.bits.wptr & ring_mask;
+ space = CIRC_SPACE(wptr, rptr, len) >= fragments;
+ spin_unlock_irqrestore(&geth->freeq_lock, flags);
+ if (!space)
+ break;
+
+ ret = geth_freeq_map_page(geth, &page, &page_mapping);
+ if (ret)
+ break;
+
+ spin_lock_irqsave(&geth->freeq_lock, flags);
+
+ rw.bits32 = readl(geth->base + GLOBAL_SWFQ_RWPTR_REG);
+ rptr = rw.bits.rptr & ring_mask;
+ wptr = rw.bits.wptr & ring_mask;
+ if (CIRC_SPACE(wptr, rptr, len) < fragments) {
+ ret = -ENOSPC;
+ } else {
+ ret = geth_freeq_add_page(geth, wptr, page,
+ page_mapping);
+ if (!ret) {
+ count += fragments;
+ wptr = (wptr + fragments) & ring_mask;
+ SET_WPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG, wptr);
+ }
+ }
+
+ spin_unlock_irqrestore(&geth->freeq_lock, flags);
+
+ if (ret) {
+ dma_unmap_single(geth->dev, page_mapping, PAGE_SIZE,
+ DMA_FROM_DEVICE);
+ put_page(page);
+ break;
+ }
+ }
- rw.bits32 = readl(geth->base + GLOBAL_SWFQ_RWPTR_REG);
- pn = (refill ? rw.bits.wptr : rw.bits.rptr) >> fpp_order;
- pn &= m_pn;
- epn = (rw.bits.rptr >> fpp_order) - 1;
- epn &= m_pn;
+ return count;
+}
- /* Loop over the freeq ring buffer entries */
- while (pn != epn) {
+static void geth_freeq_release_pages(struct gemini_ethernet *geth)
+{
+ unsigned int fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
+ unsigned int frag_len = 1 << geth->freeq_frag_order;
+ unsigned int pn;
+
+ for (pn = 0; pn < geth->num_freeq_pages; pn++) {
struct gmac_queue_page *gpage;
+ dma_addr_t mapping;
struct page *page;
+ unsigned int i;
gpage = &geth->freeq_pages[pn];
- page = gpage->page;
-
- dev_dbg(geth->dev, "fill entry %d page ref count %d add %d refs\n",
- pn, page_ref_count(page), 1 << fpp_order);
-
- if (page_ref_count(page) > 1) {
- unsigned int fl = (pn - epn) & m_pn;
+ if (!gpage->page)
+ continue;
- if (fl > 64 >> fpp_order)
- break;
+ page = gpage->page;
+ for (i = 0; i < (1 << fpp_order); i++) {
+ mapping = gpage->mapping + i * frag_len;
+ if (xa_load(&geth->freeq_mappings,
+ geth_freeq_mapping_index(geth, mapping)) != gpage)
+ continue;
- page = geth_freeq_alloc_map_page(geth, pn);
- if (!page)
- break;
+ dma_sync_single_range_for_cpu(geth->dev, gpage->mapping,
+ i * frag_len, frag_len,
+ DMA_FROM_DEVICE);
}
-
- /* Add one reference per fragment in the page */
- page_ref_add(page, 1 << fpp_order);
- count += 1 << fpp_order;
- pn++;
- pn &= m_pn;
+ dma_unmap_single_attrs(geth->dev, gpage->mapping, PAGE_SIZE,
+ DMA_FROM_DEVICE,
+ DMA_ATTR_SKIP_CPU_SYNC);
+ while (gpage->fragments) {
+ put_page(page);
+ gpage->fragments--;
+ }
+ put_page(page);
}
+ xa_destroy(&geth->freeq_mappings);
+}
- writew(pn << fpp_order, geth->base + GLOBAL_SWFQ_RWPTR_REG + 2);
+static unsigned int
+geth_freeq_page_slots(struct gemini_ethernet *geth, unsigned int order)
+{
+ unsigned int fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
+ unsigned int slots = 1 << (order - fpp_order);
- spin_unlock_irqrestore(&geth->freeq_lock, flags);
+ if (geth->port0 && geth->port0->netdev)
+ slots += 1 << geth->port0->rxq_order;
+ if (geth->port1 && geth->port1->netdev)
+ slots += 1 << geth->port1->rxq_order;
- return count;
+ return slots;
}
static int geth_setup_freeq(struct gemini_ethernet *geth)
{
unsigned int fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
unsigned int len = 1 << geth->freeq_order;
- unsigned int pages = len >> fpp_order;
+ unsigned int page_slots;
+ unsigned int expected;
union queue_threshold qt;
union dma_skb_size skbsz;
- unsigned long flags;
- struct page *page;
unsigned int filled;
- unsigned int pn;
u16 rptr;
+ page_slots = geth_freeq_page_slots(geth, geth->freeq_order);
+
geth->freeq_ring = dma_alloc_coherent(geth->dev,
sizeof(*geth->freeq_ring) << geth->freeq_order,
&geth->freeq_dma_base, GFP_KERNEL);
@@ -1003,27 +1107,22 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
}
/* Allocate page metadata referenced by the mapping index */
- geth->freeq_pages = kzalloc_objs(*geth->freeq_pages, pages);
+ geth->freeq_pages = kzalloc_objs(*geth->freeq_pages, page_slots);
if (!geth->freeq_pages)
goto err_freeq;
- geth->num_freeq_pages = pages;
-
- dev_info(geth->dev, "allocate %d pages for queue\n", pages);
- for (pn = 0; pn < pages; pn++) {
- spin_lock_irqsave(&geth->freeq_lock, flags);
- page = geth_freeq_alloc_map_page(geth, pn);
- spin_unlock_irqrestore(&geth->freeq_lock, flags);
- if (!page)
- goto err_freeq_alloc;
- }
+ geth->freeq_page_bitmap = bitmap_zalloc(page_slots, GFP_KERNEL);
+ if (!geth->freeq_page_bitmap)
+ goto err_freeq_pages;
+ geth->num_freeq_pages = page_slots;
/* Match the software pointer to the retained hardware pointer. */
rptr = GET_RPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG);
SET_WPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG, rptr & (len - 1));
- filled = geth_fill_freeq(geth, false);
- if (!filled)
- goto err_freeq_alloc;
+ expected = len - (1 << fpp_order);
+ filled = geth_fill_freeq(geth);
+ if (filled != expected)
+ goto err_freeq_bitmap;
qt.bits32 = readl(geth->base + GLOBAL_QUEUE_THRESHOLD_REG);
qt.bits.swfq_empty = 32;
@@ -1036,18 +1135,13 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
return 0;
-err_freeq_alloc:
- while (pn > 0) {
- struct gmac_queue_page *gpage;
-
- --pn;
- gpage = &geth->freeq_pages[pn];
- dma_unmap_single(geth->dev, gpage->mapping, PAGE_SIZE,
- DMA_FROM_DEVICE);
- put_page(gpage->page);
- }
- xa_destroy(&geth->freeq_mappings);
-
+err_freeq_bitmap:
+ writew(readw(geth->base + GLOBAL_SWFQ_RWPTR_REG),
+ geth->base + GLOBAL_SWFQ_RWPTR_REG + 2);
+ geth_freeq_release_pages(geth);
+ bitmap_free(geth->freeq_page_bitmap);
+ geth->freeq_page_bitmap = NULL;
+err_freeq_pages:
kfree(geth->freeq_pages);
geth->freeq_pages = NULL;
geth->num_freeq_pages = 0;
@@ -1065,11 +1159,6 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
*/
static void geth_cleanup_freeq(struct gemini_ethernet *geth)
{
- unsigned int fpp_order = PAGE_SHIFT - geth->freeq_frag_order;
- unsigned int len = 1 << geth->freeq_order;
- unsigned int pages = len >> fpp_order;
- unsigned int pn;
-
if (!geth->freeq_ring)
return;
@@ -1077,17 +1166,10 @@ static void geth_cleanup_freeq(struct gemini_ethernet *geth)
geth->base + GLOBAL_SWFQ_RWPTR_REG + 2);
writel(0, geth->base + GLOBAL_SW_FREEQ_BASE_SIZE_REG);
- for (pn = 0; pn < pages; pn++) {
- struct gmac_queue_page *gpage;
-
- gpage = &geth->freeq_pages[pn];
- dma_unmap_single(geth->dev, gpage->mapping, PAGE_SIZE,
- DMA_FROM_DEVICE);
- while (page_ref_count(gpage->page) > 0)
- put_page(gpage->page);
- }
- xa_destroy(&geth->freeq_mappings);
+ geth_freeq_release_pages(geth);
+ bitmap_free(geth->freeq_page_bitmap);
+ geth->freeq_page_bitmap = NULL;
kfree(geth->freeq_pages);
geth->freeq_pages = NULL;
geth->num_freeq_pages = 0;
@@ -1127,7 +1209,9 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
struct net_device *netdev = port->netdev;
struct gemini_ethernet_port *other_port;
struct net_device *other_netdev;
+ bool other_running;
unsigned int new_size = 0;
+ unsigned int page_slots;
unsigned int new_order;
u16 rptr;
int ret;
@@ -1143,9 +1227,7 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
else
other_port = geth->port0;
other_netdev = other_port ? other_port->netdev : NULL;
-
- if (other_netdev && netif_running(other_netdev))
- return -EBUSY;
+ other_running = other_netdev && netif_running(other_netdev);
new_size = 1 << (port->rxq_order + 1);
netdev_dbg(netdev, "port %d size: %d order %d\n",
@@ -1167,8 +1249,21 @@ static int geth_resize_freeq(struct gemini_ethernet_port *port)
new_order = max_t(unsigned int, new_order, fls(rptr));
dev_dbg(geth->dev, "set shared queue to size %d order %d\n",
new_size, new_order);
- if (geth->freeq_ring && geth->freeq_order == new_order)
- return 0;
+ if (new_order <= PAGE_SHIFT - geth->freeq_frag_order)
+ return -EINVAL;
+
+ if (geth->freeq_ring) {
+ page_slots = geth_freeq_page_slots(geth, geth->freeq_order);
+ if (geth->freeq_order == new_order &&
+ geth->num_freeq_pages == page_slots)
+ return 0;
+ if (other_running && geth->freeq_order >= new_order &&
+ geth->num_freeq_pages >= page_slots)
+ return 0;
+ }
+
+ if (other_running)
+ return -EBUSY;
disable_irq(geth->port1->irq);
geth_set_freeq_irq(geth, false);
@@ -1595,7 +1690,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
goto err_drop;
}
- page = geth_freeq_lookup(geth, mapping, &page_offs);
+ page = geth_freeq_claim(geth, mapping, &page_offs);
if (!page) {
dev_err_ratelimited(geth->dev,
"could not find mapping\n");
@@ -1692,7 +1787,7 @@ static int gmac_napi_poll(struct napi_struct *napi, int budget)
port->freeq_refill += freeq_consumed;
if (port->freeq_refill > freeq_threshold) {
port->freeq_refill -= freeq_threshold;
- geth_fill_freeq(geth, true);
+ geth_fill_freeq(geth);
}
if (!reschedule && received < budget &&
@@ -1947,10 +2042,7 @@ static int gmac_open(struct net_device *netdev)
phy_start(netdev->phydev);
err = geth_resize_freeq(port);
- /* It's fine if it's just busy, the other port has set up
- * the freeq in that case.
- */
- if (err && (err != -EBUSY)) {
+ if (err) {
netdev_err(netdev, "could not resize freeq\n");
goto err_stop_phy;
}
@@ -2309,14 +2401,20 @@ static int gmac_set_ringparam(struct net_device *netdev,
struct netlink_ext_ack *extack)
{
struct gemini_ethernet_port *port = netdev_priv(netdev);
+ unsigned int old_rxq_order;
int err = 0;
if (netif_running(netdev))
return -EBUSY;
if (rp->rx_pending) {
+ old_rxq_order = port->rxq_order;
port->rxq_order = min(15, ilog2(rp->rx_pending - 1) + 1);
err = geth_resize_freeq(port);
+ if (err) {
+ port->rxq_order = old_rxq_order;
+ return err;
+ }
}
if (rp->tx_pending) {
port->txq_order = min(15, ilog2(rp->tx_pending - 1) + 1);
@@ -2422,7 +2520,7 @@ static irqreturn_t gemini_port_irq_thread(int irq, void *data)
geth = port->geth;
/* The queue is half empty so refill it */
- geth_fill_freeq(geth, true);
+ geth_fill_freeq(geth);
spin_lock_irqsave(&geth->irq_lock, flags);
/* ACK queue interrupt */
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH net-next v7 07/18] net: ethernet: cortina: Preserve in-flight free queue pages
2026-11-10 22:13 ` [PATCH net-next v7 07/18] net: ethernet: cortina: Preserve in-flight free queue pages Linus Walleij
@ 2026-11-11 23:10 ` netdev-bot+sashiko
0 siblings, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-11-11 23:10 UTC (permalink / raw)
To: linusw
Cc: ulli.kroll, andrew+netdev, davem, kuba, pabeni, mirq-linux,
mhun512, edumazet, netdev
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The page metadata pool is now a fixed set of slots tracked by
freeq_page_bitmap, and nothing reclaims a slot while the queue is…
--
net: ethernet: cortina: Preserve in-flight free queue pages
This patch stops tying Gemini free queue page metadata to free queue
ring positions. Metadata slots come from a fixed pool tracked by a
bitmap, each posted fragment holds its own page reference, and a slot
is released only once all of its fragments have been claimed from RX
descriptors.
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index add1d3e563b62..bdbbc6d7844b9 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
[ ... ]
> @@ -735,15 +740,32 @@ geth_freeq_mapping_index(const struct gemini_ethernet *geth,
> return (unsigned long)(mapping >> geth->freeq_frag_order);
> }
>
> -static struct page *geth_freeq_lookup(struct gemini_ethernet *geth,
> - dma_addr_t mapping,
> - unsigned int *page_offs)
> +static int geth_freeq_alloc_slot(struct gemini_ethernet *geth)
> +{
> + unsigned int slot;
> +
> + lockdep_assert_held(&geth->freeq_lock);
> +
> + slot = find_first_zero_bit(geth->freeq_page_bitmap,
> + geth->num_freeq_pages);
> + if (slot == geth->num_freeq_pages)
> + return -ENOSPC;
> +
> + __set_bit(slot, geth->freeq_page_bitmap);
> +
> + return slot;
> +}
> +
> +static struct page *geth_freeq_claim(struct gemini_ethernet *geth,
> + dma_addr_t mapping,
> + unsigned int *page_offs)
> {
[ ... ]
> @@ -762,6 +784,21 @@ static struct page *geth_freeq_lookup(struct gemini_ethernet *geth,
> if (!valid)
> goto err_unlock;
>
> + dma_sync_single_range_for_cpu(geth->dev, page_mapping,
> + mapping - page_mapping, frag_len,
> + DMA_FROM_DEVICE);
> + xa_erase(&geth->freeq_mappings, index);
> + if (!--gpage->fragments) {
> + slot = gpage - geth->freeq_pages;
> + dma_unmap_single_attrs(geth->dev, page_mapping, PAGE_SIZE,
> + DMA_FROM_DEVICE,
> + DMA_ATTR_SKIP_CPU_SYNC);
> + gpage->page = NULL;
> + gpage->mapping = 0;
> + __clear_bit(slot, geth->freeq_page_bitmap);
> + put_page(page);
> + }
[Severity: Medium]
Can the fixed slot pool run out while the queue is running?
At runtime, geth_freeq_claim() is the only place that clears a bit in
freeq_page_bitmap and drops the page reference and DMA mapping for that
slot. The only other place is teardown in geth_freeq_release_pages().
So every fragment of a posted page has to come back from hardware as an
RX descriptor address that resolves.
gmac_rx() has two paths that drop a descriptor without claiming it:
if (!mapping) {
if (net_ratelimit())
netdev_err(netdev,
"rxq[%u]: HW BUG: zero DMA desc\n",
r);
goto err_drop;
}
page = geth_freeq_claim(geth, mapping, &page_offs);
if (!page) {
dev_err_ratelimited(geth->dev,
"could not find mapping\n");
goto err_drop;
}
The "could not find page" path in gmac_cleanup_rxq() behaves the same
way. Buffers that RX DMA has already pulled for a frame when
gmac_stop()->gmac_stop_dma() stops it mid-frame are also never claimed,
because gmac_cleanup_rxq() only walks the descriptors between r and w.
If the hardware has consumed a free queue fragment in any of these
cases, its slot stays set in freeq_page_bitmap permanently. The page
reference and streaming DMA mapping stay pinned with it.
geth_freeq_page_slots() sizes the pool at roughly the worst-case number
of live pages, with little slack:
unsigned int slots = 1 << (order - fpp_order);
if (geth->port0 && geth->port0->netdev)
slots += 1 << geth->port0->rxq_order;
if (geth->port1 && geth->port1->netdev)
slots += 1 << geth->port1->rxq_order;
An ifdown/ifup cycle does not recover leaked slots either. If the peer
port is running, the existing queue is kept. If both ports are idle and
the geometry has not changed, geth_resize_freeq() returns early:
if (geth->freeq_order == new_order &&
geth->num_freeq_pages == page_slots)
return 0;
Once enough slots have leaked, geth_freeq_alloc_slot() returns -ENOSPC
on every call. geth_fill_freeq() then maps a candidate page, unmaps and
frees it, and breaks without posting anything.
Would the shared SWFQ then drain and stop RX on both GMAC ports? It
looks like the only recovery is an ethtool geometry change with both
ports down, or a driver unbind.
Before this patch, metadata was indexed by queue position. A lost
fragment leaked page memory, but a later refill reused that position,
so refill never stalled.
The follow-up "Recycle claimed free queue pages" patch only recycles
slots whose fragments have all been claimed. The zero-descriptor and
lookup-failure paths still claim nothing, so this looks like it remains
at the end of the series.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-gemini-ethernet-fixes-3-v7-0-9c32a1a46aec%40kernel.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH net-next v7 08/18] net: ethernet: cortina: Rotate free queue page allocation
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (6 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 07/18] net: ethernet: cortina: Preserve in-flight free queue pages Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 09/18] net: ethernet: cortina: Recycle claimed free queue pages Linus Walleij
` (9 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The free queue metadata bitmap currently searches for a free slot from
zero for every page allocation. Under sustained traffic this repeatedly
scans occupied low-numbered slots.
Remember the position after the most recently allocated slot and wrap the
bitmap search at its end. Reset the cursor when the bitmap is released so
the two remain consistent across failed setup and teardown. This keeps
allocation cost distributed across the metadata table.
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index bdbbc6d7844b..b9d6fffd19b8 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -175,6 +175,7 @@ struct gemini_ethernet {
struct xarray freeq_mappings;
unsigned int num_freeq_pages;
unsigned long *freeq_page_bitmap;
+ unsigned int freeq_page_cursor;
spinlock_t freeq_lock; /* Locks queue from reentrance */
};
@@ -746,12 +747,20 @@ static int geth_freeq_alloc_slot(struct gemini_ethernet *geth)
lockdep_assert_held(&geth->freeq_lock);
- slot = find_first_zero_bit(geth->freeq_page_bitmap,
- geth->num_freeq_pages);
- if (slot == geth->num_freeq_pages)
- return -ENOSPC;
+ slot = find_next_zero_bit(geth->freeq_page_bitmap,
+ geth->num_freeq_pages,
+ geth->freeq_page_cursor);
+ if (slot == geth->num_freeq_pages) {
+ slot = find_first_zero_bit(geth->freeq_page_bitmap,
+ geth->freeq_page_cursor);
+ if (slot == geth->freeq_page_cursor)
+ return -ENOSPC;
+ }
__set_bit(slot, geth->freeq_page_bitmap);
+ geth->freeq_page_cursor = slot + 1;
+ if (geth->freeq_page_cursor == geth->num_freeq_pages)
+ geth->freeq_page_cursor = 0;
return slot;
}
@@ -1114,6 +1123,7 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
if (!geth->freeq_page_bitmap)
goto err_freeq_pages;
geth->num_freeq_pages = page_slots;
+ geth->freeq_page_cursor = 0;
/* Match the software pointer to the retained hardware pointer. */
rptr = GET_RPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG);
@@ -1145,6 +1155,7 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
kfree(geth->freeq_pages);
geth->freeq_pages = NULL;
geth->num_freeq_pages = 0;
+ geth->freeq_page_cursor = 0;
err_freeq:
dma_free_coherent(geth->dev,
sizeof(*geth->freeq_ring) << geth->freeq_order,
@@ -1173,6 +1184,7 @@ static void geth_cleanup_freeq(struct gemini_ethernet *geth)
kfree(geth->freeq_pages);
geth->freeq_pages = NULL;
geth->num_freeq_pages = 0;
+ geth->freeq_page_cursor = 0;
dma_free_coherent(geth->dev,
sizeof(*geth->freeq_ring) << geth->freeq_order,
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 09/18] net: ethernet: cortina: Recycle claimed free queue pages
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (7 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 08/18] net: ethernet: cortina: Rotate free queue page allocation Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 10/18] net: ethernet: cortina: Validate RX fragment lengths Linus Walleij
` (8 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The preceding "Preserve in-flight free queue pages" fix releases each
mapping after its last descriptor is claimed. Refilling therefore
allocates and maps a fresh page at line rate even after the network stack
has released all references to an old page.
Keep the free queue base reference and streaming mapping after the
fragments are claimed. Track fully claimed slots in a separate bitmap and
inspect one at a time during refill. Recycle a page once its reference
count returns to one. If the stack still owns fragments, unmap the page
and relinquish the base reference before allocating its replacement.
Reject pfmemalloc pages when recycling so emergency-reserve pages return
to the allocator instead of remaining pinned in the free queue.
This prevents retained pages from exhausting the finite metadata table
and bounds the IRQ-disabled search to a bitmap lookup and one reference
count check. It retains the existing free queue allocator without
introducing a page pool.
Recheck the recycle bitmap on each refill iteration and again after mapping
a candidate page. If another RX path retires a page concurrently, release
the candidate and return to the recycler before consuming another metadata
slot. Retry recycling immediately when a retained page is discarded so
other pending entries are examined before allocating a replacement. This
avoids stopping refill while recyclable slots are pending.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 153 +++++++++++++++++++++++++++++-----
1 file changed, 131 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index b9d6fffd19b8..4dfc8325eb72 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -175,7 +175,9 @@ struct gemini_ethernet {
struct xarray freeq_mappings;
unsigned int num_freeq_pages;
unsigned long *freeq_page_bitmap;
+ unsigned long *freeq_recycle_bitmap;
unsigned int freeq_page_cursor;
+ unsigned int freeq_recycle_pending;
spinlock_t freeq_lock; /* Locks queue from reentrance */
};
@@ -765,6 +767,34 @@ static int geth_freeq_alloc_slot(struct gemini_ethernet *geth)
return slot;
}
+static int geth_freeq_recycle_slot(struct gemini_ethernet *geth)
+{
+ unsigned int slot = geth->freeq_page_cursor;
+
+ lockdep_assert_held(&geth->freeq_lock);
+
+ if (!geth->freeq_recycle_pending)
+ return -ENOSPC;
+
+ slot = find_next_bit(geth->freeq_recycle_bitmap,
+ geth->num_freeq_pages,
+ geth->freeq_page_cursor);
+ if (slot == geth->num_freeq_pages) {
+ slot = find_first_bit(geth->freeq_recycle_bitmap,
+ geth->freeq_page_cursor);
+ if (slot == geth->freeq_page_cursor)
+ return -ENOSPC;
+ }
+
+ __clear_bit(slot, geth->freeq_recycle_bitmap);
+ geth->freeq_page_cursor = slot + 1;
+ if (geth->freeq_page_cursor == geth->num_freeq_pages)
+ geth->freeq_page_cursor = 0;
+ geth->freeq_recycle_pending--;
+
+ return slot;
+}
+
static struct page *geth_freeq_claim(struct gemini_ethernet *geth,
dma_addr_t mapping,
unsigned int *page_offs)
@@ -774,8 +804,8 @@ static struct page *geth_freeq_claim(struct gemini_ethernet *geth,
unsigned long index;
unsigned long flags;
dma_addr_t page_mapping;
- unsigned int slot;
struct page *page;
+ unsigned int slot;
bool valid;
index = geth_freeq_mapping_index(geth, mapping);
@@ -799,13 +829,8 @@ static struct page *geth_freeq_claim(struct gemini_ethernet *geth,
xa_erase(&geth->freeq_mappings, index);
if (!--gpage->fragments) {
slot = gpage - geth->freeq_pages;
- dma_unmap_single_attrs(geth->dev, page_mapping, PAGE_SIZE,
- DMA_FROM_DEVICE,
- DMA_ATTR_SKIP_CPU_SYNC);
- gpage->page = NULL;
- gpage->mapping = 0;
- __clear_bit(slot, geth->freeq_page_bitmap);
- put_page(page);
+ __set_bit(slot, geth->freeq_recycle_bitmap);
+ geth->freeq_recycle_pending++;
}
*page_offs = mapping - page_mapping;
@@ -899,26 +924,23 @@ static int geth_freeq_map_page(struct gemini_ethernet *geth,
return 0;
}
-static int geth_freeq_add_page(struct gemini_ethernet *geth, unsigned int wptr,
- struct page *page, dma_addr_t page_mapping)
+static int geth_freeq_post_page(struct gemini_ethernet *geth,
+ unsigned int wptr,
+ struct gmac_queue_page *gpage, bool recycle)
{
struct gmac_rxdesc *freeq_entry;
- struct gmac_queue_page *gpage;
unsigned int ring_mask;
unsigned int fpp_order;
unsigned int fragments;
unsigned int frag_len;
+ dma_addr_t page_mapping = gpage->mapping;
dma_addr_t mapping;
+ struct page *page = gpage->page;
int ret;
- int slot;
int i;
lockdep_assert_held(&geth->freeq_lock);
- slot = geth_freeq_alloc_slot(geth);
- if (slot < 0)
- return slot;
-
/* The assign the page mapping (physical address) to the buffer address
* in the hardware queue. PAGE_SHIFT on ARM is 12 (1 page is 4096 bytes,
* 4k), and the default RX frag order is 11 (fragments are up 20 2048
@@ -930,9 +952,6 @@ static int geth_freeq_add_page(struct gemini_ethernet *geth, unsigned int wptr,
fragments = 1 << fpp_order;
ring_mask = (1 << geth->freeq_order) - 1;
- gpage = &geth->freeq_pages[slot];
- gpage->page = page;
- gpage->mapping = page_mapping;
for (i = 0; i < fragments; i++) {
mapping = page_mapping + i * frag_len;
ret = xa_insert(&geth->freeq_mappings,
@@ -941,6 +960,9 @@ static int geth_freeq_add_page(struct gemini_ethernet *geth, unsigned int wptr,
if (ret)
goto err_mappings;
}
+ if (recycle)
+ dma_sync_single_for_device(geth->dev, page_mapping, PAGE_SIZE,
+ DMA_FROM_DEVICE);
gpage->fragments = fragments;
page_ref_add(page, fragments);
@@ -964,9 +986,70 @@ static int geth_freeq_add_page(struct gemini_ethernet *geth, unsigned int wptr,
xa_erase(&geth->freeq_mappings,
geth_freeq_mapping_index(geth, mapping));
}
- gpage->page = NULL;
- gpage->mapping = 0;
- __clear_bit(slot, geth->freeq_page_bitmap);
+ return ret;
+}
+
+static int geth_freeq_add_page(struct gemini_ethernet *geth,
+ unsigned int wptr,
+ struct page *page, dma_addr_t page_mapping)
+{
+ struct gmac_queue_page *gpage;
+ int ret;
+ int slot;
+
+ lockdep_assert_held(&geth->freeq_lock);
+
+ slot = geth_freeq_alloc_slot(geth);
+ if (slot < 0)
+ return slot;
+
+ gpage = &geth->freeq_pages[slot];
+ gpage->page = page;
+ gpage->mapping = page_mapping;
+ ret = geth_freeq_post_page(geth, wptr, gpage, false);
+ if (ret) {
+ gpage->page = NULL;
+ gpage->mapping = 0;
+ __clear_bit(slot, geth->freeq_page_bitmap);
+ }
+
+ return ret;
+}
+
+static int geth_freeq_recycle_page(struct gemini_ethernet *geth,
+ unsigned int wptr)
+{
+ struct gmac_queue_page *gpage;
+ struct page *page;
+ int ret;
+ int slot;
+
+ lockdep_assert_held(&geth->freeq_lock);
+
+ slot = geth_freeq_recycle_slot(geth);
+ if (slot < 0)
+ return slot;
+
+ gpage = &geth->freeq_pages[slot];
+ if (page_ref_count(gpage->page) > 1 ||
+ !dev_page_is_reusable(gpage->page)) {
+ page = gpage->page;
+ dma_unmap_single_attrs(geth->dev, gpage->mapping, PAGE_SIZE,
+ DMA_FROM_DEVICE,
+ DMA_ATTR_SKIP_CPU_SYNC);
+ gpage->page = NULL;
+ gpage->mapping = 0;
+ __clear_bit(slot, geth->freeq_page_bitmap);
+ put_page(page);
+ return -EAGAIN;
+ }
+
+ ret = geth_freeq_post_page(geth, wptr, gpage, true);
+ if (ret) {
+ __set_bit(slot, geth->freeq_recycle_bitmap);
+ geth->freeq_recycle_pending++;
+ }
+
return ret;
}
@@ -1001,9 +1084,22 @@ static unsigned int geth_fill_freeq(struct gemini_ethernet *geth)
rptr = rw.bits.rptr & ring_mask;
wptr = rw.bits.wptr & ring_mask;
space = CIRC_SPACE(wptr, rptr, len) >= fragments;
+ ret = -ENOSPC;
+ if (space && geth->freeq_recycle_pending) {
+ ret = geth_freeq_recycle_page(geth, wptr);
+ if (!ret) {
+ count += fragments;
+ wptr = (wptr + fragments) & ring_mask;
+ SET_WPTR(geth->base + GLOBAL_SWFQ_RWPTR_REG, wptr);
+ }
+ }
spin_unlock_irqrestore(&geth->freeq_lock, flags);
if (!space)
break;
+ if (!ret || ret == -EAGAIN)
+ continue;
+ if (ret != -ENOSPC)
+ break;
ret = geth_freeq_map_page(geth, &page, &page_mapping);
if (ret)
@@ -1016,6 +1112,8 @@ static unsigned int geth_fill_freeq(struct gemini_ethernet *geth)
wptr = rw.bits.wptr & ring_mask;
if (CIRC_SPACE(wptr, rptr, len) < fragments) {
ret = -ENOSPC;
+ } else if (geth->freeq_recycle_pending) {
+ ret = -EAGAIN;
} else {
ret = geth_freeq_add_page(geth, wptr, page,
page_mapping);
@@ -1032,6 +1130,8 @@ static unsigned int geth_fill_freeq(struct gemini_ethernet *geth)
dma_unmap_single(geth->dev, page_mapping, PAGE_SIZE,
DMA_FROM_DEVICE);
put_page(page);
+ if (ret == -EAGAIN)
+ continue;
break;
}
}
@@ -1076,6 +1176,7 @@ static void geth_freeq_release_pages(struct gemini_ethernet *geth)
put_page(page);
}
xa_destroy(&geth->freeq_mappings);
+ geth->freeq_recycle_pending = 0;
}
static unsigned int
@@ -1122,6 +1223,9 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
geth->freeq_page_bitmap = bitmap_zalloc(page_slots, GFP_KERNEL);
if (!geth->freeq_page_bitmap)
goto err_freeq_pages;
+ geth->freeq_recycle_bitmap = bitmap_zalloc(page_slots, GFP_KERNEL);
+ if (!geth->freeq_recycle_bitmap)
+ goto err_freeq_page_bitmap;
geth->num_freeq_pages = page_slots;
geth->freeq_page_cursor = 0;
@@ -1149,6 +1253,9 @@ static int geth_setup_freeq(struct gemini_ethernet *geth)
writew(readw(geth->base + GLOBAL_SWFQ_RWPTR_REG),
geth->base + GLOBAL_SWFQ_RWPTR_REG + 2);
geth_freeq_release_pages(geth);
+ bitmap_free(geth->freeq_recycle_bitmap);
+ geth->freeq_recycle_bitmap = NULL;
+err_freeq_page_bitmap:
bitmap_free(geth->freeq_page_bitmap);
geth->freeq_page_bitmap = NULL;
err_freeq_pages:
@@ -1179,6 +1286,8 @@ static void geth_cleanup_freeq(struct gemini_ethernet *geth)
geth_freeq_release_pages(geth);
+ bitmap_free(geth->freeq_recycle_bitmap);
+ geth->freeq_recycle_bitmap = NULL;
bitmap_free(geth->freeq_page_bitmap);
geth->freeq_page_bitmap = NULL;
kfree(geth->freeq_pages);
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 10/18] net: ethernet: cortina: Validate RX fragment lengths
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (8 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 09/18] net: ethernet: cortina: Recycle claimed free queue pages Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 11/18] net: ethernet: cortina: Account RX buffer size in truesize Linus Walleij
` (7 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
RX descriptor lengths are used to adjust the page offset and populate skb
fragments without checking that the resulting range remains inside the
posted free queue fragment. A zero-length descriptor is logged but is still
appended.
The backing page is larger than the default 2 KiB DMA fragment, so checking
only the page boundary would allow a malformed descriptor for the first
half of a page to consume data from the sibling fragment. That fragment may
still be owned by the device.
Reject a short initial fragment before applying NET_IP_ALIGN. Validate the
final fragment length derived from the total frame byte count, rejecting
underflow and ranges extending beyond either the DMA fragment or the page.
Drop zero-length non-final descriptors. If an EOF descriptor carries no new
data after earlier descriptors completed the frame, release its unused page
reference and pass the assembled skb to GRO. Check this case before the
fragment limit so an empty EOF can terminate a full fragment array.
Correct the copied buffer_size comment to match the Gemini manual, which
defines it as the transfer buffer size associated with the descriptor.
Require the final frame length to cover an Ethernet frame so a malformed
chain cannot pass a short skb to GRO. Account descriptor inconsistencies as
drops rather than wire length errors.
The issue was found by inspecting the descriptor arithmetic and validated
against the Gemini receive descriptor definitions. The series was tested on
a D-Link DIR-685 with media playback and ksmbd transfers under ping load.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 42 +++++++++++++++++++++++++++--------
drivers/net/ethernet/cortina/gemini.h | 2 +-
2 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 4dfc8325eb72..ddda4a9c236b 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1749,6 +1749,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
struct gemini_ethernet_port *port = netdev_priv(netdev);
unsigned short m = (1 << port->rxq_order) - 1;
struct gemini_ethernet *geth = port->geth;
+ unsigned int freeq_frag_len = 1 << geth->freeq_frag_order;
void __iomem *ptr_reg = port->rxq_rwptr;
unsigned int frag_nr = port->rx_frag_nr;
struct sk_buff *skb = port->rx_skb;
@@ -1823,6 +1824,9 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
if (!skb)
goto err_drop;
+ if (frag_len < NET_IP_ALIGN)
+ goto err_drop;
+
page_offs += NET_IP_ALIGN;
frag_len -= NET_IP_ALIGN;
frag_nr = 0;
@@ -1831,15 +1835,33 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
goto err_drop;
}
- if (word3.bits32 & EOF_BIT)
+ if (word3.bits32 & EOF_BIT) {
+ if (frame_len < ETH_ZLEN ||
+ frame_len < skb->len)
+ goto err_drop;
frag_len = frame_len - skb->len;
+ }
+
+ if (!frag_len) {
+ if (word3.bits32 & EOF_BIT) {
+ put_page(page);
+ page = NULL;
+ goto complete_frame;
+ }
+
+ if (net_ratelimit())
+ netdev_err(netdev,
+ "Received fragment with len = 0\n");
+ goto err_drop;
+ }
/* append page frag to skb */
if (frag_nr == MAX_SKB_FRAGS)
goto err_drop;
-
- if (frag_len == 0 && net_ratelimit())
- netdev_err(netdev, "Received fragment with len = 0\n");
+ if (frag_len > freeq_frag_len -
+ (page_offs & (freeq_frag_len - 1)) ||
+ frag_len > PAGE_SIZE - page_offs)
+ goto err_drop;
skb_fill_page_desc(skb, frag_nr, page, page_offs, frag_len);
skb->len += frag_len;
@@ -1847,11 +1869,13 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
skb->truesize += frag_len;
frag_nr++;
- if (word3.bits32 & EOF_BIT) {
- napi_gro_frags(&port->napi);
- skb = NULL;
- frag_nr = 0;
- }
+ if (!(word3.bits32 & EOF_BIT))
+ goto next_desc;
+
+complete_frame:
+ napi_gro_frags(&port->napi);
+ skb = NULL;
+ frag_nr = 0;
goto next_desc;
err_drop:
diff --git a/drivers/net/ethernet/cortina/gemini.h b/drivers/net/ethernet/cortina/gemini.h
index d359c633c5de..43006fe44c9f 100644
--- a/drivers/net/ethernet/cortina/gemini.h
+++ b/drivers/net/ethernet/cortina/gemini.h
@@ -519,7 +519,7 @@ struct gmac_txdesc {
union gmac_rxdesc_0 {
unsigned int bits32;
struct bit_8060 {
- /* bit 15:0 number of descriptors used for the current frame */
+ /* bit 15:0 transfer buffer size for this descriptor */
unsigned int buffer_size:16;
/* bit 21:16 number of descriptors used for the current frame */
unsigned int desc_count:6;
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 11/18] net: ethernet: cortina: Account RX buffer size in truesize
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (9 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 10/18] net: ethernet: cortina: Validate RX fragment lengths Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 12/18] net: ethernet: cortina: Release partial RX frames on stop Linus Walleij
` (6 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
gmac_rx() charges only the received payload length for each page fragment
attached to an skb. Each fragment pins a complete free queue DMA buffer, so
short packets understate their memory use to GRO and socket accounting.
Charge the full posted fragment size for every attached fragment. The skb
already accounts for its own head separately.
The issue was found by reviewing the RX fragment accounting. The series was
tested on a D-Link DIR-685 with media playback and ksmbd transfers under
ping load.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index ddda4a9c236b..73ba008a9c7f 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1866,7 +1866,7 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
skb_fill_page_desc(skb, frag_nr, page, page_offs, frag_len);
skb->len += frag_len;
skb->data_len += frag_len;
- skb->truesize += frag_len;
+ skb->truesize += freeq_frag_len;
frag_nr++;
if (!(word3.bits32 & EOF_BIT))
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 12/18] net: ethernet: cortina: Release partial RX frames on stop
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (10 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 11/18] net: ethernet: cortina: Account RX buffer size in truesize Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 13/18] net: ethernet: cortina: Scale Gemini RX queues to system memory Linus Walleij
` (5 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
A NAPI poll can retain a partially assembled frame across invocations. The
stop path clears port->rx_skb without freeing the skb retained in
napi->skb. If the interface is reopened, napi_get_frags() returns that
stale skb. The next frame overwrites its fragment array from index zero
while len and data_len still include the partial frame.
This can lose page references belonging to the old fragments or pass an
skb with inconsistent fragment accounting to GRO. If the interface is
removed without reopening, NAPI teardown eventually frees the stale skb.
Free a pending fragment skb after disabling NAPI and before clearing the
saved RX state. Count the discarded frame as dropped under the RX
statistics sequence counter. Use the irqsave update helpers because
ndo_stop runs in preemptible process context while 32-bit statistics
writers must not be preempted.
The issue was found by inspecting the partial frame lifetime and statistics
writer contexts. The series was tested on a D-Link DIR-685 with media
playback and ksmbd transfers under ping load.
Fixes: 06937db21ee3 ("net: ethernet: cortina: Make RX SKB per-port")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 73ba008a9c7f..d08dbb0aabcf 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -2228,12 +2228,19 @@ static int gmac_open(struct net_device *netdev)
static int gmac_stop(struct net_device *netdev)
{
struct gemini_ethernet_port *port = netdev_priv(netdev);
+ unsigned long flags;
hrtimer_cancel(&port->rx_coalesce_timer);
netif_tx_stop_all_queues(netdev);
gmac_disable_tx_rx(netdev);
gmac_stop_dma(port);
napi_disable(&port->napi);
+ if (port->rx_skb) {
+ napi_free_frags(&port->napi);
+ flags = u64_stats_update_begin_irqsave(&port->rx_stats_syncp);
+ port->stats.rx_dropped++;
+ u64_stats_update_end_irqrestore(&port->rx_stats_syncp, flags);
+ }
port->rx_skb = NULL;
port->rx_frag_nr = 0;
port->rx_dropping = false;
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 13/18] net: ethernet: cortina: Scale Gemini RX queues to system memory
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (11 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 12/18] net: ethernet: cortina: Release partial RX frames on stop Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 14/18] net: ethernet: cortina: Serialize TX completion statistics Linus Walleij
` (4 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The default RX queues contain 512 descriptors per port. The shared
software free queue is sized at twice their combined capacity, requiring
4 MiB of page fragments even on Gemini systems with only 32 or 64 MiB of
RAM.
Choose smaller default RX queues on memory-constrained systems. Use 256
descriptors per port and a 2 MiB free queue with at most 64 MiB of RAM,
and 128 descriptors per port with a 1 MiB free queue with at most 32 MiB.
Keep the existing defaults on systems with more memory.
This preserves the existing relationship between RX descriptor capacity
and the free queue. Users can still override the defaults with ethtool.
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index d08dbb0aabcf..a96b6292820a 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -16,6 +16,7 @@
#include <linux/kernel.h>
#include <linux/bitmap.h>
#include <linux/init.h>
+#include <linux/mm.h>
#include <linux/module.h>
#include <linux/net.h>
#include <linux/platform_device.h>
@@ -37,6 +38,7 @@
#include <linux/skbuff.h>
#include <linux/phy.h>
#include <linux/crc32.h>
+#include <linux/sizes.h>
#include <linux/ethtool.h>
#include <linux/tcp.h>
#include <linux/u64_stats_sync.h>
@@ -475,6 +477,17 @@ static int gmac_pick_rx_max_len(unsigned int max_l3_len)
return -1;
}
+static unsigned int gmac_pick_rxq_order(void)
+{
+ if (totalram_pages() <= (SZ_32M >> PAGE_SHIFT))
+ return DEFAULT_GMAC_RXQ_ORDER - 2;
+
+ if (totalram_pages() <= (SZ_64M >> PAGE_SHIFT))
+ return DEFAULT_GMAC_RXQ_ORDER - 1;
+
+ return DEFAULT_GMAC_RXQ_ORDER;
+}
+
static int gmac_init(struct net_device *netdev)
{
struct gemini_ethernet_port *port = netdev_priv(netdev);
@@ -543,7 +556,7 @@ static int gmac_init(struct net_device *netdev)
writel(sw_weigh.bits32,
port->dma_base + GMAC_TX_WEIGHTING_CTRL_1_REG);
- port->rxq_order = DEFAULT_GMAC_RXQ_ORDER;
+ port->rxq_order = gmac_pick_rxq_order();
port->txq_order = DEFAULT_GMAC_TXQ_ORDER;
port->rx_coalesce_nsecs = DEFAULT_RX_COALESCE_NSECS;
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 14/18] net: ethernet: cortina: Serialize TX completion statistics
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (12 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 13/18] net: ethernet: cortina: Scale Gemini RX queues to system memory Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 15/18] net: ethernet: cortina: Scope RX statistics updates Linus Walleij
` (3 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
gmac_clean_txq() updates ir_stats_syncp with hard interrupts enabled,
while the MIB and RX overrun interrupt paths update the same seqcount.
Initializing its lockdep key exposes the incompatible IRQ contexts on
32-bit debug kernels. The writer sections also lack mutual exclusion.
Take the existing IRQ-safe irq_lock around the TX completion statistics
update, matching the other ir_stats_syncp writers. Acquire it before
opening the seqcount writer section so PREEMPT_RT does not acquire a
potentially sleeping spinlock with preemption disabled.
Found during Sashiko review and confirmed by tracing all three writer
paths. Cross-built gemini.o for ARM with W=1. No hardware lockdep or
PREEMPT_RT runtime testing was performed for this change.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index a96b6292820a..04a125d6d413 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -637,6 +637,7 @@ static void gmac_clean_txq(struct net_device *netdev, struct gmac_txq *txq,
union gmac_txdesc_1 word1;
unsigned int hwchksum = 0;
unsigned long bytes = 0;
+ unsigned long flags;
struct gmac_txdesc *txd;
unsigned short nfrags;
unsigned int errs = 0;
@@ -688,12 +689,14 @@ static void gmac_clean_txq(struct net_device *netdev, struct gmac_txq *txq,
}
}
+ spin_lock_irqsave(&geth->irq_lock, flags);
u64_stats_update_begin(&port->ir_stats_syncp);
port->stats.tx_errors += errs;
port->stats.tx_packets += pkts;
port->stats.tx_bytes += bytes;
port->tx_hw_csummed += hwchksum;
u64_stats_update_end(&port->ir_stats_syncp);
+ spin_unlock_irqrestore(&geth->irq_lock, flags);
txq->cptr = c;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 15/18] net: ethernet: cortina: Scope RX statistics updates
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (13 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 14/18] net: ethernet: cortina: Serialize TX completion statistics Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-11 23:10 ` netdev-bot+sashiko
2026-11-10 22:13 ` [PATCH net-next v7 16/18] net: ethernet: cortina: Drain RX coalescing timer on stop Linus Walleij
` (2 subsequent siblings)
17 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
On 32-bit PREEMPT_RT systems u64_stats_update_begin() disables
preemption. The RX poll currently holds that writer section across
gmac_rx(), which takes spinlock_t locks and calls into GRO and fragment
handling. spinlock_t can sleep on PREEMPT_RT and must not be acquired
with preemption disabled.
Keep the writer sections around the counter updates only. NAPI ownership
continues to serialize normal RX writers, while the stop path updates its
counter after napi_disable(). This preserves the existing accounting
without covering potentially sleeping operations.
Initialize the dynamically allocated statistics synchronization objects
so their seqcount lockdep keys are valid on 32-bit debug kernels.
The issue was found by inspecting the u64_stats_sync writer scope. The
series was tested on a D-Link DIR-685 with media playback and ksmbd
transfers under ping load. PREEMPT_RT was not runtime-tested.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 04a125d6d413..59e6aeb07e0b 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1727,12 +1727,12 @@ static struct sk_buff *gmac_skb_if_good_frame(struct gemini_ethernet_port *port,
unsigned int rx_status = word0.bits.status;
struct sk_buff *skb = NULL;
- port->rx_stats[rx_status]++;
- port->rx_csum_stats[rx_csum]++;
-
if (word0.bits.derr || word0.bits.perr ||
rx_status || frame_len < ETH_ZLEN ||
rx_csum >= RX_CHKSUM_IP_ERR_UNKNOWN) {
+ u64_stats_update_begin(&port->rx_stats_syncp);
+ port->rx_stats[rx_status]++;
+ port->rx_csum_stats[rx_csum]++;
port->stats.rx_errors++;
if (frame_len < ETH_ZLEN || RX_ERROR_LENGTH(rx_status))
@@ -1743,6 +1743,7 @@ static struct sk_buff *gmac_skb_if_good_frame(struct gemini_ethernet_port *port,
port->stats.rx_crc_errors++;
if (RX_ERROR_FRAME(rx_status))
port->stats.rx_frame_errors++;
+ u64_stats_update_end(&port->rx_stats_syncp);
return NULL;
}
@@ -1754,8 +1755,12 @@ static struct sk_buff *gmac_skb_if_good_frame(struct gemini_ethernet_port *port,
skb->ip_summed = CHECKSUM_UNNECESSARY;
update_exit:
+ u64_stats_update_begin(&port->rx_stats_syncp);
+ port->rx_stats[rx_status]++;
+ port->rx_csum_stats[rx_csum]++;
port->stats.rx_bytes += frame_len;
port->stats.rx_packets++;
+ u64_stats_update_end(&port->rx_stats_syncp);
return skb;
}
@@ -1813,7 +1818,9 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
if (word3.bits32 & SOF_BIT) {
if (skb) {
napi_free_frags(&port->napi);
+ u64_stats_update_begin(&port->rx_stats_syncp);
port->stats.rx_dropped++;
+ u64_stats_update_end(&port->rx_stats_syncp);
skb = NULL;
frag_nr = 0;
}
@@ -1905,7 +1912,9 @@ static unsigned int gmac_rx(struct net_device *netdev, unsigned int budget,
put_page(page);
if (!dropping) {
+ u64_stats_update_begin(&port->rx_stats_syncp);
port->stats.rx_dropped++;
+ u64_stats_update_end(&port->rx_stats_syncp);
dropping = true;
}
@@ -1937,13 +1946,13 @@ static int gmac_napi_poll(struct napi_struct *napi, int budget)
bool reschedule;
freeq_threshold = 1 << (geth->freeq_order - 1);
- u64_stats_update_begin(&port->rx_stats_syncp);
received = gmac_rx(napi->dev, budget, &freeq_consumed, &reschedule);
- if (!reschedule && received < budget)
+ if (!reschedule && received < budget) {
+ u64_stats_update_begin(&port->rx_stats_syncp);
++port->rx_napi_exits;
-
- u64_stats_update_end(&port->rx_stats_syncp);
+ u64_stats_update_end(&port->rx_stats_syncp);
+ }
port->freeq_refill += freeq_consumed;
if (port->freeq_refill > freeq_threshold) {
@@ -2870,6 +2879,9 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev)
}
port = netdev_priv(netdev);
+ u64_stats_init(&port->tx_stats_syncp);
+ u64_stats_init(&port->rx_stats_syncp);
+ u64_stats_init(&port->ir_stats_syncp);
SET_NETDEV_DEV(netdev, dev);
port->netdev = netdev;
port->id = id;
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH net-next v7 15/18] net: ethernet: cortina: Scope RX statistics updates
2026-11-10 22:13 ` [PATCH net-next v7 15/18] net: ethernet: cortina: Scope RX statistics updates Linus Walleij
@ 2026-11-11 23:10 ` netdev-bot+sashiko
0 siblings, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-11-11 23:10 UTC (permalink / raw)
To: linusw
Cc: ulli.kroll, andrew+netdev, davem, kuba, pabeni, mirq-linux,
mhun512, edumazet, netdev
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] net: cortina: softirq-unsafe tx_stats_syncp update in
gmac_clean_txq()
--
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -2870,6 +2879,9 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev)
> }
>
> port = netdev_priv(netdev);
> + u64_stats_init(&port->tx_stats_syncp);
> + u64_stats_init(&port->rx_stats_syncp);
> + u64_stats_init(&port->ir_stats_syncp);
> SET_NETDEV_DEV(netdev, dev);
> port->netdev = netdev;
> port->id = id;
>
[Severity: Medium]
Initializing tx_stats_syncp here is correct. On 32-bit lockdep
kernels, though, it also exposes an existing tx_stats_syncp writer
that runs from the wrong context.
gmac_clean_txq() bumps port->tx_frag_stats[nfrags] under a plain
u64_stats_update_begin(&port->tx_stats_syncp). When called from
gmac_start_xmit(), this runs with BH disabled, so lockdep records the
seqcount as used in softirq.
gmac_stop() also reaches it, via gmac_cleanup_txqs() and
gmac_clean_txq(), but from process context with BH and preemption
enabled. The last frames queued before the interface goes down are
normally still in the ring at that point. If one of them was a
multi-fragment frame (nfrags > 0), the tx_stats_syncp update runs
preemptible and with softirqs enabled.
On 32-bit non-RT, that hits the lockdep_assert_preemption_disabled()
check in __u64_stats_update_begin(). With the lockdep map now
initialized, it should also produce an inconsistent
IN-SOFTIRQ-W -> SOFTIRQ-ON-W report for tx_stats_syncp.
There is a real hazard behind the splat too. While the sequence count
is odd, nothing stops a softirq on the same CPU from reading TX stats
through gmac_get_stats64(). That reader would spin forever in
u64_stats_fetch_begin().
The other counters in these paths are already protected:
- The rx_stats_syncp update in gmac_stop() uses
u64_stats_update_begin_irqsave().
- The ir_stats_syncp update in gmac_clean_txq() is done under
geth->irq_lock.
Should the tx_stats_syncp update get the same treatment? Or would you
rather disable BH around gmac_cleanup_txqs() in gmac_stop()? Either
way, I think the fix belongs in this patch or just before it, since
the u64_stats_init() added here is what lets lockdep start tracking
this seqcount.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-gemini-ethernet-fixes-3-v7-0-9c32a1a46aec%40kernel.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH net-next v7 16/18] net: ethernet: cortina: Drain RX coalescing timer on stop
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (14 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 15/18] net: ethernet: cortina: Scope RX statistics updates Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-11 23:10 ` netdev-bot+sashiko
2026-11-10 22:13 ` [PATCH net-next v7 17/18] net: ethernet: cortina: Reject TX rings smaller than retained pointers Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 18/18] net: ethernet: cortina: Fix ethtool statistics reads Linus Walleij
17 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
gmac_stop() cancels the RX coalescing timer before disabling and
freeing the IRQ. An RX interrupt racing with that cancellation can
rearm the timer, leaving it queued after the netdev and its private
port state have been freed.
Initialize the timer before requesting the IRQ, and cancel it again
after free_irq() has drained every handler that can rearm it. Keep the
first cancellation to prevent the timer callback from scheduling NAPI
while the poll instance is being disabled.
This was found by code inspection and tested on the D-Link DIR-685.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 59e6aeb07e0b..58d6ed81a59c 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -2201,6 +2201,10 @@ static int gmac_open(struct net_device *netdev)
struct gemini_ethernet_port *port = netdev_priv(netdev);
int err;
+ hrtimer_setup(&port->rx_coalesce_timer,
+ &gmac_coalesce_delay_expired,
+ CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+
err = request_irq(netdev->irq, gmac_irq,
IRQF_SHARED, netdev->name, netdev);
if (err) {
@@ -2237,9 +2241,6 @@ static int gmac_open(struct net_device *netdev)
gmac_enable_tx_rx(netdev);
netif_tx_start_all_queues(netdev);
- hrtimer_setup(&port->rx_coalesce_timer, &gmac_coalesce_delay_expired, CLOCK_MONOTONIC,
- HRTIMER_MODE_REL);
-
netdev_dbg(netdev, "opened\n");
return 0;
@@ -2276,6 +2277,7 @@ static int gmac_stop(struct net_device *netdev)
phy_stop(netdev->phydev);
free_irq(netdev->irq, netdev);
+ hrtimer_cancel(&port->rx_coalesce_timer);
gmac_update_hw_stats(netdev);
return 0;
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH net-next v7 16/18] net: ethernet: cortina: Drain RX coalescing timer on stop
2026-11-10 22:13 ` [PATCH net-next v7 16/18] net: ethernet: cortina: Drain RX coalescing timer on stop Linus Walleij
@ 2026-11-11 23:10 ` netdev-bot+sashiko
0 siblings, 0 replies; 25+ messages in thread
From: netdev-bot+sashiko @ 2026-11-11 23:10 UTC (permalink / raw)
To: linusw
Cc: ulli.kroll, andrew+netdev, davem, kuba, pabeni, mirq-linux,
mhun512, edumazet, netdev
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The commit message does not explain that moving hrtimer_setup()
in gmac_open() (drivers/net/ethernet/cortina/gemini.c) fixes a…
- [Low] The commit message's reason for keeping the first
hrtimer_cancel() in gmac_stop()…
--
net: ethernet: cortina: Drain RX coalescing timer on stop
This patch moves hrtimer_setup() for the RX coalescing timer in
gmac_open() so it runs before request_irq(). It also adds a second
hrtimer_cancel() in gmac_stop() after free_irq(). The aim is that no RX
interrupt can leave the timer queued after the port state has been freed.
> Initialize the timer before requesting the IRQ, and cancel it again
> after free_irq() has drained every handler that can rearm it. Keep the
> first cancellation to prevent the timer callback from scheduling NAPI
> while the poll instance is being disabled.
[Severity: Low]
Is this description of the first cancellation accurate? The paragraph
before it says an RX interrupt racing with that cancellation can rearm
the timer. That still seems possible while napi_disable() runs.
In gmac_stop(), the RX interrupt is only masked after napi_disable():
gmac_stop() {
hrtimer_cancel(&port->rx_coalesce_timer);
netif_tx_stop_all_queues(netdev);
gmac_disable_tx_rx(netdev);
gmac_stop_dma(port);
napi_disable(&port->napi);
...
gmac_enable_irq(netdev, 0);
...
}
A NAPI poll that finishes at the same time as the first cancel turns
the RX interrupt back on:
gmac_napi_poll() {
...
if (!reschedule && received < budget &&
napi_complete_done(napi, received))
gmac_enable_rx_irq(napi->dev, 1);
...
}
After that, gmac_irq() can call hrtimer_start() again. Then
gmac_coalesce_delay_expired()->napi_schedule() can run while
napi_disable() is still in progress. When rx_coalesce_nsecs is 0,
gmac_irq() calls napi_schedule() directly in the same window.
This does not cause a functional problem. A napi_schedule() on a NAPI
instance that is being disabled is either a no-op or gets waited out.
Still, the first cancel only handles a timer that is already pending. It
does not handle the rearm case.
Could the commit message say the early cancel is only best-effort?
Another option is to mask the RX interrupt before napi_disable().
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index 59e6aeb07e0b4..58d6ed81a59c8 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -2201,6 +2201,10 @@ static int gmac_open(struct net_device *netdev)
> struct gemini_ethernet_port *port = netdev_priv(netdev);
> int err;
>
> + hrtimer_setup(&port->rx_coalesce_timer,
> + &gmac_coalesce_delay_expired,
> + CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +
> err = request_irq(netdev->irq, gmac_irq,
> IRQF_SHARED, netdev->name, netdev);
> if (err) {
> @@ -2237,9 +2241,6 @@ static int gmac_open(struct net_device *netdev)
> gmac_enable_tx_rx(netdev);
> netif_tx_start_all_queues(netdev);
>
> - hrtimer_setup(&port->rx_coalesce_timer, &gmac_coalesce_delay_expired, CLOCK_MONOTONIC,
> - HRTIMER_MODE_REL);
> -
> netdev_dbg(netdev, "opened\n");
>
> return 0;
[Severity: Low]
This move also seems to fix a separate race on the open side, and the
commit message does not describe it. Should the commit message mention
it, or should it go in its own patch?
Before this patch, gmac_open() called gmac_enable_irq(netdev, 1) before
hrtimer_setup(). gmac_irq() arms the timer once the RX interrupt is
unmasked, if rx_coalesce_nsecs is non-zero:
if (!port->rx_coalesce_nsecs) {
napi_schedule(&port->napi);
} else {
...
hrtimer_start(&port->rx_coalesce_timer, ktime,
HRTIMER_MODE_REL);
}
The default is 0, but gmac_set_coalesce() lets rx-usecs be set while
the interface is down. In that case, an RX interrupt can arrive between
gmac_enable_irq(netdev, 1) and the old hrtimer_setup() call:
- On the first open, it would start a zeroed hrtimer that was never
initialized.
- On a reopen, it would start a timer that hrtimer_setup() then
re-initializes while it is still queued.
The stop-side fix does not depend on this move, because ndo_stop only
runs after a successful open. If the commit message does not explain the
move, backporters might treat this hunk as cosmetic.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-gemini-ethernet-fixes-3-v7-0-9c32a1a46aec%40kernel.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH net-next v7 17/18] net: ethernet: cortina: Reject TX rings smaller than retained pointers
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (15 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 16/18] net: ethernet: cortina: Drain RX coalescing timer on stop Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
2026-11-10 22:13 ` [PATCH net-next v7 18/18] net: ethernet: cortina: Fix ethtool statistics reads Linus Walleij
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The hardware-owned TX read pointers can retain their positions when the
queues are rebuilt. After shrinking the TX ring with ethtool -G while
the interface is down, gmac_setup_txqs() copies those pointers into the
write and clean pointers without checking the new ring size. A retained
pointer outside the smaller ring can then index past both the skb table
and the coherent descriptor allocation.
Reject a TX ring size that cannot contain every retained read pointer,
before applying any RX ring changes in the same ethtool request. Check
again before allocating and programming the TX rings on open, including
for pointer state inherited from firmware. DMA is stopped at both checks.
Do not just mask the software pointers, which would leave the hardware
read pointer outside the new ring.
Found during Sashiko review and confirmed with a pointer arithmetic
model: a retained pointer of 200 with 16 entries reports 15 free entries
and permits the first TX write at index 200. Checked all 16-bit pointer
values against queue orders 0 through 15 and cross-built the driver for
ARM.
Tested on the D-Link DIR-685 with intense transfers and ping storms.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 58d6ed81a59c..1007fd54cb05 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -568,6 +568,20 @@ static int gmac_init(struct net_device *netdev)
return 0;
}
+static bool gmac_txq_size_valid(struct net_device *netdev, unsigned int entries)
+{
+ struct gemini_ethernet_port *port = netdev_priv(netdev);
+ unsigned int i;
+
+ /* DMA is stopped, so the hardware read pointers cannot advance. */
+ for (i = 0; i < netdev->num_tx_queues; i++)
+ if (GET_RPTR(port->dma_base + GMAC_SW_TX_QUEUE_PTR_REG(i)) >=
+ entries)
+ return false;
+
+ return true;
+}
+
static int gmac_setup_txqs(struct net_device *netdev)
{
struct gemini_ethernet_port *port = netdev_priv(netdev);
@@ -582,6 +596,12 @@ static int gmac_setup_txqs(struct net_device *netdev)
unsigned int r;
int i;
+ if (!gmac_txq_size_valid(netdev, entries)) {
+ netdev_err(netdev,
+ "TX ring is too small for the retained read pointer\n");
+ return -EINVAL;
+ }
+
rwptr_reg = port->dma_base + GMAC_SW_TX_QUEUE0_PTR_REG;
skb_tab = kzalloc_objs(*skb_tab, len);
@@ -2581,11 +2601,21 @@ static int gmac_set_ringparam(struct net_device *netdev,
{
struct gemini_ethernet_port *port = netdev_priv(netdev);
unsigned int old_rxq_order;
+ unsigned int txq_order = port->txq_order;
int err = 0;
if (netif_running(netdev))
return -EBUSY;
+ if (rp->tx_pending) {
+ txq_order = min(15, ilog2(rp->tx_pending - 1) + 1);
+ if (!gmac_txq_size_valid(netdev, BIT(txq_order))) {
+ NL_SET_ERR_MSG(extack,
+ "TX ring is too small for the retained read pointer");
+ return -EINVAL;
+ }
+ }
+
if (rp->rx_pending) {
old_rxq_order = port->rxq_order;
port->rxq_order = min(15, ilog2(rp->rx_pending - 1) + 1);
@@ -2596,7 +2626,7 @@ static int gmac_set_ringparam(struct net_device *netdev,
}
}
if (rp->tx_pending) {
- port->txq_order = min(15, ilog2(rp->tx_pending - 1) + 1);
+ port->txq_order = txq_order;
port->irq_every_tx_packets = 1 << (port->txq_order - 2);
}
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH net-next v7 18/18] net: ethernet: cortina: Fix ethtool statistics reads
2026-11-10 22:13 [PATCH net-next v7 00/18] net: ethernet: cortina: Fix Gemini RX/TX buffer management Linus Walleij
` (16 preceding siblings ...)
2026-11-10 22:13 ` [PATCH net-next v7 17/18] net: ethernet: cortina: Reject TX rings smaller than retained pointers Linus Walleij
@ 2026-11-10 22:13 ` Linus Walleij
17 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-11-10 22:13 UTC (permalink / raw)
To: Hans Ulli Kroll, Andrew Lunn, David S. Miller, Jakub Kicinski,
Paolo Abeni, Michał Mirosław, Myeonghun Pak,
Eric Dumazet
Cc: netdev, Linus Walleij
The TX statistics reader resets p on a seqcount retry but writes through
values instead. A retry on 32-bit systems therefore writes another 18
counters past the end of the ethtool statistics array. ETHTOOL_GSTATS
does not require privileges.
Write through p so each retry overwrites the same output slots. Stop
clearing the fragment counters from the reader, which races with TX
updates and resets the counters on each read. Snapshot tx_hw_csummed
under ir_stats_syncp, matching its writer, and retain its output position.
Found during Sashiko review and checked with an extracted-reader host
harness that forces seqcount retries. The old reader overwrote guard
entries and cleared counters; the fixed reader stays within the output
array and preserves counters.
Tested on the D-Link DIR-685 with intense transfers and ping storms,
reading out stats with ethtool at the same time.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
drivers/net/ethernet/cortina/gemini.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 1007fd54cb05..b586205a7e20 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -2478,6 +2478,7 @@ static void gmac_get_ethtool_stats(struct net_device *netdev,
{
struct gemini_ethernet_port *port = netdev_priv(netdev);
unsigned int start;
+ u64 tx_hw_csummed;
u64 *p;
int i;
@@ -2490,6 +2491,7 @@ static void gmac_get_ethtool_stats(struct net_device *netdev,
for (i = 0; i < RX_STATS_NUM; i++)
*p++ = port->hw_stats[i];
+ tx_hw_csummed = port->tx_hw_csummed;
} while (u64_stats_fetch_retry(&port->ir_stats_syncp, start));
values = p;
@@ -2513,12 +2515,10 @@ static void gmac_get_ethtool_stats(struct net_device *netdev,
p = values;
start = u64_stats_fetch_begin(&port->tx_stats_syncp);
- for (i = 0; i < TX_MAX_FRAGS; i++) {
- *values++ = port->tx_frag_stats[i];
- port->tx_frag_stats[i] = 0;
- }
- *values++ = port->tx_frags_linearized;
- *values++ = port->tx_hw_csummed;
+ for (i = 0; i < TX_MAX_FRAGS; i++)
+ *p++ = port->tx_frag_stats[i];
+ *p++ = port->tx_frags_linearized;
+ *p++ = tx_hw_csummed;
} while (u64_stats_fetch_retry(&port->tx_stats_syncp, start));
}
--
2.56.0
^ permalink raw reply related [flat|nested] 25+ messages in thread