net: ibm: emac: Clear MAL descriptors without memset

[PATCH net-next] net: ibm: emac: Clear MAL descriptors without memset

Posted by Rosen Penev 1 week, 1 day ago

Clear MAL descriptor rings with explicit field stores instead of
memset().  The descriptor rings are carved from MAL coherent DMA memory,
which may be mapped uncached on 32-bit powerpc.  The optimized memset()
path can use dcbz there and trigger an alignment warning.

The skb tracking arrays remain ordinary CPU memory and still use memset().

Assisted-by: Codex:GPT-5.5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/ibm/emac/core.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index 1fd7cb50c568..7ab5c944f5de 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -1161,6 +1161,17 @@ static void emac_clean_rx_ring(struct emac_instance *dev)
 	}
 }
 
+static void emac_clear_mal_desc(struct mal_descriptor *desc, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		desc[i].ctrl = 0;
+		desc[i].data_len = 0;
+		desc[i].data_ptr = 0;
+	}
+}
+
 static int
 __emac_prepare_rx_skb(struct sk_buff *skb, struct emac_instance *dev, int slot)
 {
@@ -3121,8 +3132,8 @@ static int emac_probe(struct platform_device *ofdev)
 	DBG(dev, "rx_desc %p" NL, dev->rx_desc);
 
 	/* Clean rings */
-	memset(dev->tx_desc, 0, NUM_TX_BUFF * sizeof(struct mal_descriptor));
-	memset(dev->rx_desc, 0, NUM_RX_BUFF * sizeof(struct mal_descriptor));
+	emac_clear_mal_desc(dev->tx_desc, NUM_TX_BUFF);
+	emac_clear_mal_desc(dev->rx_desc, NUM_RX_BUFF);
 	memset(dev->tx_skb, 0, NUM_TX_BUFF * sizeof(struct sk_buff *));
 	memset(dev->rx_skb, 0, NUM_RX_BUFF * sizeof(struct sk_buff *));
 
-- 
2.54.0

Re: [PATCH net-next] net: ibm: emac: Clear MAL descriptors without memset

Posted by Paolo Abeni 3 days, 20 hours ago

On 5/17/26 5:36 AM, Rosen Penev wrote:
> Clear MAL descriptor rings with explicit field stores instead of
> memset().  The descriptor rings are carved from MAL coherent DMA memory,
> which may be mapped uncached on 32-bit powerpc.  The optimized memset()
> path can use dcbz there and trigger an alignment warning.
> 
> The skb tracking arrays remain ordinary CPU memory and still use memset().
> 
> Assisted-by: Codex:GPT-5.5
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/net/ethernet/ibm/emac/core.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
> index 1fd7cb50c568..7ab5c944f5de 100644
> --- a/drivers/net/ethernet/ibm/emac/core.c
> +++ b/drivers/net/ethernet/ibm/emac/core.c
> @@ -1161,6 +1161,17 @@ static void emac_clean_rx_ring(struct emac_instance *dev)
>  	}
>  }
>  
> +static void emac_clear_mal_desc(struct mal_descriptor *desc, int count)
> +{
> +	int i;
> +
> +	for (i = 0; i < count; i++) {
> +		desc[i].ctrl = 0;
> +		desc[i].data_len = 0;
> +		desc[i].data_ptr = 0;

Sashiko noted that the compiler could still optimize the above to a
memset(). Perhaps a barrier() is needed here?

/P

Re: [PATCH net-next] net: ibm: emac: Clear MAL descriptors without memset

Posted by Rosen Penev 3 days, 10 hours ago

On Thu, May 21, 2026 at 2:45 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 5/17/26 5:36 AM, Rosen Penev wrote:
> > Clear MAL descriptor rings with explicit field stores instead of
> > memset().  The descriptor rings are carved from MAL coherent DMA memory,
> > which may be mapped uncached on 32-bit powerpc.  The optimized memset()
> > path can use dcbz there and trigger an alignment warning.
> >
> > The skb tracking arrays remain ordinary CPU memory and still use memset().
> >
> > Assisted-by: Codex:GPT-5.5
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  drivers/net/ethernet/ibm/emac/core.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
> > index 1fd7cb50c568..7ab5c944f5de 100644
> > --- a/drivers/net/ethernet/ibm/emac/core.c
> > +++ b/drivers/net/ethernet/ibm/emac/core.c
> > @@ -1161,6 +1161,17 @@ static void emac_clean_rx_ring(struct emac_instance *dev)
> >       }
> >  }
> >
> > +static void emac_clear_mal_desc(struct mal_descriptor *desc, int count)
> > +{
> > +     int i;
> > +
> > +     for (i = 0; i < count; i++) {
> > +             desc[i].ctrl = 0;
> > +             desc[i].data_len = 0;
> > +             desc[i].data_ptr = 0;
>
> Sashiko noted that the compiler could still optimize the above to a
> memset(). Perhaps a barrier() is needed here?
The canonical way to do this is WRITE_ONCE() AFAIK.

In practice it seems it's not an issue. That is

https://lore.kernel.org/all/2e3acfe63d289c6fba366e16973c9ab8369e8b75.1631803922.git.christophe.leroy@csgroup.eu/

no longer triggers.
>
> /P
>

Re: [PATCH net-next] net: ibm: emac: Clear MAL descriptors without memset

Posted by Andrew Lunn 3 days, 17 hours ago

On Thu, May 21, 2026 at 11:45:25AM +0200, Paolo Abeni wrote:
> On 5/17/26 5:36 AM, Rosen Penev wrote:
> > Clear MAL descriptor rings with explicit field stores instead of
> > memset().  The descriptor rings are carved from MAL coherent DMA memory,
> > which may be mapped uncached on 32-bit powerpc.  The optimized memset()
> > path can use dcbz there and trigger an alignment warning.
> 
> Sashiko noted that the compiler could still optimize the above to a
> memset(). Perhaps a barrier() is needed here?

Maybe my understanding is wrong, but the compiler should have the
knowledge about if the addresses are aligned or not, and so if it can
call the optimised memset, or needs to keep with individual byte
accesses?

Has this patch been in user for a while, a port from downstream
OpenWRT or something?

	Andrew

Re: [PATCH net-next] net: ibm: emac: Clear MAL descriptors without memset

Posted by Rosen Penev 3 days, 10 hours ago

On Thu, May 21, 2026 at 5:06 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, May 21, 2026 at 11:45:25AM +0200, Paolo Abeni wrote:
> > On 5/17/26 5:36 AM, Rosen Penev wrote:
> > > Clear MAL descriptor rings with explicit field stores instead of
> > > memset().  The descriptor rings are carved from MAL coherent DMA memory,
> > > which may be mapped uncached on 32-bit powerpc.  The optimized memset()
> > > path can use dcbz there and trigger an alignment warning.
> >
> > Sashiko noted that the compiler could still optimize the above to a
> > memset(). Perhaps a barrier() is needed here?
>
> Maybe my understanding is wrong, but the compiler should have the
> knowledge about if the addresses are aligned or not, and so if it can
> call the optimised memset, or needs to keep with individual byte
> accesses?
>
> Has this patch been in user for a while, a port from downstream
> OpenWRT or something?
Yes and no. These devices are currently on kernel 6.12, where the
warnings are present. In the transition to 6.18, I'm fixing them.
>
>         Andrew