* [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing
2020-06-29 18:47 [PATCH RESEND net-next 0/2] " Ioana Ciornei
@ 2020-06-29 18:47 ` Ioana Ciornei
0 siblings, 0 replies; 8+ messages in thread
From: Ioana Ciornei @ 2020-06-29 18:47 UTC (permalink / raw)
To: davem, netdev; +Cc: Ioana Ciornei
Instead of realloc-ing the skb on the Tx path when the provided headroom
is smaller than the HW requirements, create a Scatter/Gather frame
descriptor with only one entry.
Remove the '[drv] tx realloc frames' counter exposed previously through
ethtool since it is no longer used.
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
.../freescale/dpaa2/dpaa2-eth-debugfs.c | 7 +-
.../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 177 +++++++++++++++---
.../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 9 +-
.../ethernet/freescale/dpaa2/dpaa2-ethtool.c | 1 -
4 files changed, 160 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
index 2880ca02d7e7..5cb357c74dec 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
@@ -19,14 +19,14 @@ static int dpaa2_dbg_cpu_show(struct seq_file *file, void *offset)
int i;
seq_printf(file, "Per-CPU stats for %s\n", priv->net_dev->name);
- seq_printf(file, "%s%16s%16s%16s%16s%16s%16s%16s%16s%16s\n",
+ seq_printf(file, "%s%16s%16s%16s%16s%16s%16s%16s%16s\n",
"CPU", "Rx", "Rx Err", "Rx SG", "Tx", "Tx Err", "Tx conf",
- "Tx SG", "Tx realloc", "Enq busy");
+ "Tx SG", "Enq busy");
for_each_online_cpu(i) {
stats = per_cpu_ptr(priv->percpu_stats, i);
extras = per_cpu_ptr(priv->percpu_extras, i);
- seq_printf(file, "%3d%16llu%16llu%16llu%16llu%16llu%16llu%16llu%16llu%16llu\n",
+ seq_printf(file, "%3d%16llu%16llu%16llu%16llu%16llu%16llu%16llu%16llu\n",
i,
stats->rx_packets,
stats->rx_errors,
@@ -35,7 +35,6 @@ static int dpaa2_dbg_cpu_show(struct seq_file *file, void *offset)
stats->tx_errors,
extras->tx_conf_frames,
extras->tx_sg_frames,
- extras->tx_reallocs,
extras->tx_portal_busy);
}
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 712bbfdbe7d7..4a264b75c035 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -685,6 +685,86 @@ static int build_sg_fd(struct dpaa2_eth_priv *priv,
return err;
}
+/* Create a SG frame descriptor based on a linear skb.
+ *
+ * This function is used on the Tx path when the skb headroom is not large
+ * enough for the HW requirements, thus instead of realloc-ing the skb we
+ * create a SG frame descriptor with only one entry.
+ */
+static int build_sg_fd_single_buf(struct dpaa2_eth_priv *priv,
+ struct sk_buff *skb,
+ struct dpaa2_fd *fd)
+{
+ struct device *dev = priv->net_dev->dev.parent;
+ struct dpaa2_eth_sgt_cache *sgt_cache;
+ struct dpaa2_sg_entry *sgt;
+ struct dpaa2_eth_swa *swa;
+ dma_addr_t addr, sgt_addr;
+ void *sgt_buf = NULL;
+ int sgt_buf_size;
+ int err;
+
+ /* Prepare the HW SGT structure */
+ sgt_cache = this_cpu_ptr(priv->sgt_cache);
+ sgt_buf_size = priv->tx_data_offset + sizeof(struct dpaa2_sg_entry);
+
+ if (sgt_cache->count == 0)
+ sgt_buf = kzalloc(sgt_buf_size + DPAA2_ETH_TX_BUF_ALIGN,
+ GFP_ATOMIC);
+ else
+ sgt_buf = sgt_cache->buf[--sgt_cache->count];
+ if (unlikely(!sgt_buf))
+ return -ENOMEM;
+
+ sgt_buf = PTR_ALIGN(sgt_buf, DPAA2_ETH_TX_BUF_ALIGN);
+ sgt = (struct dpaa2_sg_entry *)(sgt_buf + priv->tx_data_offset);
+
+ addr = dma_map_single(dev, skb->data, skb->len, DMA_BIDIRECTIONAL);
+ if (unlikely(dma_mapping_error(dev, addr))) {
+ err = -ENOMEM;
+ goto data_map_failed;
+ }
+
+ /* Fill in the HW SGT structure */
+ dpaa2_sg_set_addr(sgt, addr);
+ dpaa2_sg_set_len(sgt, skb->len);
+ dpaa2_sg_set_final(sgt, true);
+
+ /* Store the skb backpointer in the SGT buffer */
+ swa = (struct dpaa2_eth_swa *)sgt_buf;
+ swa->type = DPAA2_ETH_SWA_SINGLE;
+ swa->single.skb = skb;
+ swa->sg.sgt_size = sgt_buf_size;
+
+ /* Separately map the SGT buffer */
+ sgt_addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_BIDIRECTIONAL);
+ if (unlikely(dma_mapping_error(dev, sgt_addr))) {
+ err = -ENOMEM;
+ goto sgt_map_failed;
+ }
+
+ dpaa2_fd_set_offset(fd, priv->tx_data_offset);
+ dpaa2_fd_set_format(fd, dpaa2_fd_sg);
+ dpaa2_fd_set_addr(fd, sgt_addr);
+ dpaa2_fd_set_len(fd, skb->len);
+ dpaa2_fd_set_ctrl(fd, FD_CTRL_PTA);
+
+ if (priv->tx_tstamp && skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
+ enable_tx_tstamp(fd, sgt_buf);
+
+ return 0;
+
+sgt_map_failed:
+ dma_unmap_single(dev, addr, skb->len, DMA_BIDIRECTIONAL);
+data_map_failed:
+ if (sgt_cache->count >= DPAA2_ETH_SGT_CACHE_SIZE)
+ kfree(sgt_buf);
+ else
+ sgt_cache->buf[sgt_cache->count++] = sgt_buf;
+
+ return err;
+}
+
/* Create a frame descriptor based on a linear skb */
static int build_single_fd(struct dpaa2_eth_priv *priv,
struct sk_buff *skb,
@@ -743,13 +823,16 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
const struct dpaa2_fd *fd, bool in_napi)
{
struct device *dev = priv->net_dev->dev.parent;
- dma_addr_t fd_addr;
+ dma_addr_t fd_addr, sg_addr;
struct sk_buff *skb = NULL;
unsigned char *buffer_start;
struct dpaa2_eth_swa *swa;
u8 fd_format = dpaa2_fd_get_format(fd);
u32 fd_len = dpaa2_fd_get_len(fd);
+ struct dpaa2_eth_sgt_cache *sgt_cache;
+ struct dpaa2_sg_entry *sgt;
+
fd_addr = dpaa2_fd_get_addr(fd);
buffer_start = dpaa2_iova_to_virt(priv->iommu_domain, fd_addr);
swa = (struct dpaa2_eth_swa *)buffer_start;
@@ -769,16 +852,29 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
DMA_BIDIRECTIONAL);
}
} else if (fd_format == dpaa2_fd_sg) {
- skb = swa->sg.skb;
+ if (swa->type == DPAA2_ETH_SWA_SG) {
+ skb = swa->sg.skb;
+
+ /* Unmap the scatterlist */
+ dma_unmap_sg(dev, swa->sg.scl, swa->sg.num_sg,
+ DMA_BIDIRECTIONAL);
+ kfree(swa->sg.scl);
- /* Unmap the scatterlist */
- dma_unmap_sg(dev, swa->sg.scl, swa->sg.num_sg,
- DMA_BIDIRECTIONAL);
- kfree(swa->sg.scl);
+ /* Unmap the SGT buffer */
+ dma_unmap_single(dev, fd_addr, swa->sg.sgt_size,
+ DMA_BIDIRECTIONAL);
+ } else {
+ skb = swa->single.skb;
- /* Unmap the SGT buffer */
- dma_unmap_single(dev, fd_addr, swa->sg.sgt_size,
- DMA_BIDIRECTIONAL);
+ /* Unmap the SGT Buffer */
+ dma_unmap_single(dev, fd_addr, swa->single.sgt_size,
+ DMA_BIDIRECTIONAL);
+
+ sgt = (struct dpaa2_sg_entry *)(buffer_start +
+ priv->tx_data_offset);
+ sg_addr = dpaa2_sg_get_addr(sgt);
+ dma_unmap_single(dev, sg_addr, skb->len, DMA_BIDIRECTIONAL);
+ }
} else {
netdev_dbg(priv->net_dev, "Invalid FD format\n");
return;
@@ -808,8 +904,17 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
}
/* Free SGT buffer allocated on tx */
- if (fd_format != dpaa2_fd_single)
- skb_free_frag(buffer_start);
+ if (fd_format != dpaa2_fd_single) {
+ sgt_cache = this_cpu_ptr(priv->sgt_cache);
+ if (swa->type == DPAA2_ETH_SWA_SG) {
+ skb_free_frag(buffer_start);
+ } else {
+ if (sgt_cache->count >= DPAA2_ETH_SGT_CACHE_SIZE)
+ kfree(buffer_start);
+ else
+ sgt_cache->buf[sgt_cache->count++] = buffer_start;
+ }
+ }
/* Move on with skb release */
napi_consume_skb(skb, in_napi);
@@ -833,22 +938,6 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
percpu_extras = this_cpu_ptr(priv->percpu_extras);
needed_headroom = dpaa2_eth_needed_headroom(priv, skb);
- if (skb_headroom(skb) < needed_headroom) {
- struct sk_buff *ns;
-
- ns = skb_realloc_headroom(skb, needed_headroom);
- if (unlikely(!ns)) {
- percpu_stats->tx_dropped++;
- goto err_alloc_headroom;
- }
- percpu_extras->tx_reallocs++;
-
- if (skb->sk)
- skb_set_owner_w(ns, skb->sk);
-
- dev_kfree_skb(skb);
- skb = ns;
- }
/* We'll be holding a back-reference to the skb until Tx Confirmation;
* we don't want that overwritten by a concurrent Tx with a cloned skb.
@@ -867,6 +956,10 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
err = build_sg_fd(priv, skb, &fd);
percpu_extras->tx_sg_frames++;
percpu_extras->tx_sg_bytes += skb->len;
+ } else if (skb_headroom(skb) < needed_headroom) {
+ err = build_sg_fd_single_buf(priv, skb, &fd);
+ percpu_extras->tx_sg_frames++;
+ percpu_extras->tx_sg_bytes += skb->len;
} else {
err = build_single_fd(priv, skb, &fd);
}
@@ -924,7 +1017,6 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
return NETDEV_TX_OK;
err_build_fd:
-err_alloc_headroom:
dev_kfree_skb(skb);
return NETDEV_TX_OK;
@@ -1161,6 +1253,22 @@ static int refill_pool(struct dpaa2_eth_priv *priv,
return 0;
}
+static void dpaa2_eth_sgt_cache_drain(struct dpaa2_eth_priv *priv)
+{
+ struct dpaa2_eth_sgt_cache *sgt_cache;
+ u16 count;
+ int k, i;
+
+ for_each_online_cpu(k) {
+ sgt_cache = per_cpu_ptr(priv->sgt_cache, k);
+ count = sgt_cache->count;
+
+ for (i = 0; i < count; i++)
+ kfree(sgt_cache->buf[i]);
+ sgt_cache->count = 0;
+ }
+}
+
static int pull_channel(struct dpaa2_eth_channel *ch)
{
int err;
@@ -1562,6 +1670,9 @@ static int dpaa2_eth_stop(struct net_device *net_dev)
/* Empty the buffer pool */
drain_pool(priv);
+ /* Empty the Scatter-Gather Buffer cache */
+ dpaa2_eth_sgt_cache_drain(priv);
+
return 0;
}
@@ -3846,6 +3957,13 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
goto err_alloc_percpu_extras;
}
+ priv->sgt_cache = alloc_percpu(*priv->sgt_cache);
+ if (!priv->sgt_cache) {
+ dev_err(dev, "alloc_percpu(sgt_cache) failed\n");
+ err = -ENOMEM;
+ goto err_alloc_sgt_cache;
+ }
+
err = netdev_init(net_dev);
if (err)
goto err_netdev_init;
@@ -3914,6 +4032,8 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
err_alloc_rings:
err_csum:
err_netdev_init:
+ free_percpu(priv->sgt_cache);
+err_alloc_sgt_cache:
free_percpu(priv->percpu_extras);
err_alloc_percpu_extras:
free_percpu(priv->percpu_stats);
@@ -3959,6 +4079,7 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
fsl_mc_free_irqs(ls_dev);
free_rings(priv);
+ free_percpu(priv->sgt_cache);
free_percpu(priv->percpu_stats);
free_percpu(priv->percpu_extras);
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
index 2d7ada0f0dbd..9e4ceb92f240 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
@@ -125,6 +125,7 @@ struct dpaa2_eth_swa {
union {
struct {
struct sk_buff *skb;
+ int sgt_size;
} single;
struct {
struct sk_buff *skb;
@@ -282,7 +283,6 @@ struct dpaa2_eth_drv_stats {
__u64 tx_conf_bytes;
__u64 tx_sg_frames;
__u64 tx_sg_bytes;
- __u64 tx_reallocs;
__u64 rx_sg_frames;
__u64 rx_sg_bytes;
/* Enqueues retried due to portal busy */
@@ -395,6 +395,12 @@ struct dpaa2_eth_cls_rule {
u8 in_use;
};
+#define DPAA2_ETH_SGT_CACHE_SIZE 256
+struct dpaa2_eth_sgt_cache {
+ void *buf[DPAA2_ETH_SGT_CACHE_SIZE];
+ u16 count;
+};
+
/* Driver private data */
struct dpaa2_eth_priv {
struct net_device *net_dev;
@@ -409,6 +415,7 @@ struct dpaa2_eth_priv {
u8 num_channels;
struct dpaa2_eth_channel *channel[DPAA2_ETH_MAX_DPCONS];
+ struct dpaa2_eth_sgt_cache __percpu *sgt_cache;
struct dpni_attr dpni_attrs;
u16 dpni_ver_major;
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
index e88269fe3de7..c4cbbcaa9a3f 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
@@ -43,7 +43,6 @@ static char dpaa2_ethtool_extras[][ETH_GSTRING_LEN] = {
"[drv] tx conf bytes",
"[drv] tx sg frames",
"[drv] tx sg bytes",
- "[drv] tx realloc frames",
"[drv] rx sg frames",
"[drv] rx sg bytes",
"[drv] enqueue portal busy",
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing
[not found] ` <20201210173156.mbizovo6rxvkda73@holly.lan>
@ 2020-12-10 18:06 ` Ioana Ciornei
2020-12-11 7:21 ` Jon Nettleton
2020-12-11 14:01 ` Ioana Ciornei
0 siblings, 2 replies; 8+ messages in thread
From: Ioana Ciornei @ 2020-12-10 18:06 UTC (permalink / raw)
To: Daniel Thompson
Cc: linux-netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
[Added also the netdev mailing list, I haven't heard of linux-netdev
before but kept it]
On Thu, Dec 10, 2020 at 05:31:56PM +0000, Daniel Thompson wrote:
> Hi Ioana
Hi Daniel,
>
> On Mon, Jun 29, 2020 at 06:47:11PM +0000, Ioana Ciornei wrote:
> > Instead of realloc-ing the skb on the Tx path when the provided headroom
> > is smaller than the HW requirements, create a Scatter/Gather frame
> > descriptor with only one entry.
> >
> > Remove the '[drv] tx realloc frames' counter exposed previously through
> > ethtool since it is no longer used.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
>
> I've been chasing down a networking regression on my LX2160A board
> (Honeycomb LX2K based on CEx7 LX2160A COM) that first appeared in v5.9.
>
> It makes the board unreliable opening outbound connections meaning
> things like `apt update` or `git fetch` often can't open the connection.
> It does not happen all the time but is sufficient to make the boards
> built-in networking useless for workstation use.
>
> The problem is strongly linked to warnings in the logs so I used the
> warnings to bisect down to locate the cause of the regression and it
> pinpointed this patch. I have confirmed that in both v5.9 and v5.10-rc7
> that reverting this patch (and fixing up the merge issues) fixes the
> regression and the warnings stop appearing.
>
> A typical example of the warning is below (io-pgtable-arm.c:281 is an
> error path that I guess would cause dma_map_page_attrs() to return
> an error):
>
> [ 714.464927] WARNING: CPU: 13 PID: 0 at
> drivers/iommu/io-pgtable-arm.c:281 __arm_lpae_map+0x2d4/0x30c
> [ 714.464930] Modules linked in: snd_seq_dummy(E) snd_hrtimer(E)
> snd_seq(E) snd_seq_device(E) snd_timer(E) snd(E) soundcore(E) bridge(E)
> stp(E) llc(E) rfkill(E) caam_jr(E) crypto_engine(E) rng_core(E)
> joydev(E) evdev(E) dpaa2_caam(E) caamhash_desc(E) caamalg_desc(E)
> authenc(E) libdes(E) dpaa2_console(E) ofpart(E) caam(E) sg(E) error(E)
> lm90(E) at24(E) spi_nor(E) mtd(E) sbsa_gwdt(E) qoriq_thermal(E)
> layerscape_edac_mod(E) qoriq_cpufreq(E) drm(E) fuse(E) configfs(E)
> ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E)
> mbcache(E) jbd2(E) hid_generic(E) usbhid(E) hid(E) dm_crypt(E) dm_mod(E)
> sd_mod(E) fsl_dpaa2_ptp(E) ptp_qoriq(E) fsl_dpaa2_eth(E)
> xhci_plat_hcd(E) xhci_hcd(E) usbcore(E) aes_ce_blk(E) crypto_simd(E)
> cryptd(E) aes_ce_cipher(E) ghash_ce(E) gf128mul(E) at803x(E) libaes(E)
> fsl_mc_dpio(E) pcs_lynx(E) rtc_pcf2127(E) sha2_ce(E) phylink(E)
> xgmac_mdio(E) regmap_spi(E) of_mdio(E) sha256_arm64(E)
> i2c_mux_pca954x(E) fixed_phy(E) i2c_mux(E) sha1_ce(E) ptp(E) libphy(E)
> [ 714.465131] pps_core(E) ahci_qoriq(E) libahci_platform(E) nvme(E)
> libahci(E) nvme_core(E) t10_pi(E) libata(E) crc_t10dif(E)
> crct10dif_generic(E) crct10dif_common(E) dwc3(E) scsi_mod(E) udc_core(E)
> roles(E) ulpi(E) sdhci_of_esdhc(E) sdhci_pltfm(E) sdhci(E)
> spi_nxp_fspi(E) i2c_imx(E) fixed(E)
> [ 714.465192] CPU: 13 PID: 0 Comm: swapper/13 Tainted: G W E
> 5.10.0-rc7-00001-gba98d13279ca #52
> [ 714.465196] Hardware name: SolidRun LX2160A Honeycomb (DT)
> [ 714.465202] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> [ 714.465207] pc : __arm_lpae_map+0x2d4/0x30c
> [ 714.465211] lr : __arm_lpae_map+0x114/0x30c
> [ 714.465215] sp : ffff80001006b340
> [ 714.465219] x29: ffff80001006b340 x28: 0000002086538003
> [ 714.465227] x27: 0000000000000a20 x26: 0000000000001000
> [ 714.465236] x25: 0000000000000f44 x24: 00000020adf8d000
> [ 714.465245] x23: 0000000000000001 x22: 0000fffffaeca000
> [ 714.465253] x21: 0000000000000003 x20: ffff19b60d64d200
> [ 714.465261] x19: 00000000000000ca x18: 0000000000000000
> [ 714.465270] x17: 0000000000000000 x16: ffffcccb7cf3ca20
> [ 714.465278] x15: 0000000000000000 x14: 0000000000000000
> [ 714.465286] x13: 0000000000000003 x12: 0000000000000010
> [ 714.465294] x11: 0000000000000000 x10: 0000000000000002
> [ 714.465302] x9 : ffffcccb7d5b6e78 x8 : 00000000000001ff
> [ 714.465311] x7 : ffff19b606538650 x6 : ffff19b606538000
> [ 714.465319] x5 : 0000000000000009 x4 : 0000000000000f44
> [ 714.465327] x3 : 0000000000001000 x2 : 00000020adf8d000
> [ 714.465335] x1 : 0000000000000002 x0 : 0000000000000003
> [ 714.465343] Call trace:
> [ 714.465348] __arm_lpae_map+0x2d4/0x30c
> [ 714.465353] __arm_lpae_map+0x114/0x30c
> [ 714.465357] __arm_lpae_map+0x114/0x30c
> [ 714.465362] __arm_lpae_map+0x114/0x30c
> [ 714.465366] arm_lpae_map+0xf4/0x180
> [ 714.465373] arm_smmu_map+0x4c/0xc0
> [ 714.465379] __iommu_map+0x100/0x2bc
> [ 714.465385] iommu_map_atomic+0x20/0x30
> [ 714.465391] __iommu_dma_map+0xb0/0x110
> [ 714.465397] iommu_dma_map_page+0xb8/0x120
> [ 714.465404] dma_map_page_attrs+0x1a8/0x210
> [ 714.465413] __dpaa2_eth_tx+0x384/0xbd0 [fsl_dpaa2_eth]
> [ 714.465421] dpaa2_eth_tx+0x84/0x134 [fsl_dpaa2_eth]
> [ 714.465427] dev_hard_start_xmit+0x10c/0x2b0
> [ 714.465433] sch_direct_xmit+0x1a0/0x550
> [ 714.465438] __qdisc_run+0x140/0x670
> [ 714.465443] __dev_queue_xmit+0x6c4/0xa74
> [ 714.465449] dev_queue_xmit+0x20/0x2c
> [ 714.465463] br_dev_queue_push_xmit+0xc4/0x1a0 [bridge]
> [ 714.465476] br_forward_finish+0xdc/0xf0 [bridge]
> [ 714.465489] __br_forward+0x160/0x1c0 [bridge]
> [ 714.465502] br_forward+0x13c/0x160 [bridge]
> [ 714.465514] br_dev_xmit+0x228/0x3b0 [bridge]
> [ 714.465520] dev_hard_start_xmit+0x10c/0x2b0
> [ 714.465526] __dev_queue_xmit+0x8f0/0xa74
> [ 714.465531] dev_queue_xmit+0x20/0x2c
> [ 714.465538] arp_xmit+0xc0/0xd0
> [ 714.465544] arp_send_dst+0x78/0xa0
> [ 714.465550] arp_solicit+0xf4/0x260
> [ 714.465554] neigh_probe+0x64/0xb0
> [ 714.465560] neigh_timer_handler+0x2f4/0x400
> [ 714.465566] call_timer_fn+0x3c/0x184
> [ 714.465572] __run_timers.part.0+0x2bc/0x370
> [ 714.465578] run_timer_softirq+0x48/0x80
> [ 714.465583] __do_softirq+0x120/0x36c
> [ 714.465589] irq_exit+0xac/0x100
> [ 714.465596] __handle_domain_irq+0x8c/0xf0
> [ 714.465600] gic_handle_irq+0xcc/0x14c
> [ 714.465605] el1_irq+0xc4/0x180
> [ 714.465610] arch_cpu_idle+0x18/0x30
> [ 714.465617] default_idle_call+0x4c/0x180
> [ 714.465623] do_idle+0x238/0x2b0
> [ 714.465629] cpu_startup_entry+0x30/0xa0
> [ 714.465636] secondary_start_kernel+0x134/0x180
> [ 714.465640] ---[ end trace a84a7f61b559005f ]---
>
>
> Given it is the iommu code that is provoking the warning I should
> probably mention that the board I have requires
> arm-smmu.disable_bypass=0 on the kernel command line in order to boot.
> Also if it matters I am running the latest firmware from Solidrun
> which is based on LSDK-20.04.
>
Hmmm, from what I remember I think I tested this with the smmu bypassed
so that is why I didn't catch it.
> Is there any reason for this code not to be working for LX2160A?
I wouldn't expect this to be LX2160A specific but rather a bug in the
implementation.. sorry.
Let me reproduce it and see if I can get to the bottom of it and I will
get back with some more info.
Ioana
>
> Daniel.
>
>
> PS A few months have gone by so I decided not to trim the patch out
> of this reply so you don't have to go digging!
>
>
>
> > .../freescale/dpaa2/dpaa2-eth-debugfs.c | 7 +-
> > .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 177 +++++++++++++++---
> > .../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 9 +-
> > .../ethernet/freescale/dpaa2/dpaa2-ethtool.c | 1 -
> > 4 files changed, 160 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
> > index 2880ca02d7e7..5cb357c74dec 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
> > @@ -19,14 +19,14 @@ static int dpaa2_dbg_cpu_show(struct seq_file *file, void *offset)
> > int i;
> >
> > seq_printf(file, "Per-CPU stats for %s\n", priv->net_dev->name);
> > - seq_printf(file, "%s%16s%16s%16s%16s%16s%16s%16s%16s%16s\n",
> > + seq_printf(file, "%s%16s%16s%16s%16s%16s%16s%16s%16s\n",
> > "CPU", "Rx", "Rx Err", "Rx SG", "Tx", "Tx Err", "Tx conf",
> > - "Tx SG", "Tx realloc", "Enq busy");
> > + "Tx SG", "Enq busy");
> >
> > for_each_online_cpu(i) {
> > stats = per_cpu_ptr(priv->percpu_stats, i);
> > extras = per_cpu_ptr(priv->percpu_extras, i);
> > - seq_printf(file, "%3d%16llu%16llu%16llu%16llu%16llu%16llu%16llu%16llu%16llu\n",
> > + seq_printf(file, "%3d%16llu%16llu%16llu%16llu%16llu%16llu%16llu%16llu\n",
> > i,
> > stats->rx_packets,
> > stats->rx_errors,
> > @@ -35,7 +35,6 @@ static int dpaa2_dbg_cpu_show(struct seq_file *file, void *offset)
> > stats->tx_errors,
> > extras->tx_conf_frames,
> > extras->tx_sg_frames,
> > - extras->tx_reallocs,
> > extras->tx_portal_busy);
> > }
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > index 712bbfdbe7d7..4a264b75c035 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > @@ -685,6 +685,86 @@ static int build_sg_fd(struct dpaa2_eth_priv *priv,
> > return err;
> > }
> >
> > +/* Create a SG frame descriptor based on a linear skb.
> > + *
> > + * This function is used on the Tx path when the skb headroom is not large
> > + * enough for the HW requirements, thus instead of realloc-ing the skb we
> > + * create a SG frame descriptor with only one entry.
> > + */
> > +static int build_sg_fd_single_buf(struct dpaa2_eth_priv *priv,
> > + struct sk_buff *skb,
> > + struct dpaa2_fd *fd)
> > +{
> > + struct device *dev = priv->net_dev->dev.parent;
> > + struct dpaa2_eth_sgt_cache *sgt_cache;
> > + struct dpaa2_sg_entry *sgt;
> > + struct dpaa2_eth_swa *swa;
> > + dma_addr_t addr, sgt_addr;
> > + void *sgt_buf = NULL;
> > + int sgt_buf_size;
> > + int err;
> > +
> > + /* Prepare the HW SGT structure */
> > + sgt_cache = this_cpu_ptr(priv->sgt_cache);
> > + sgt_buf_size = priv->tx_data_offset + sizeof(struct dpaa2_sg_entry);
> > +
> > + if (sgt_cache->count == 0)
> > + sgt_buf = kzalloc(sgt_buf_size + DPAA2_ETH_TX_BUF_ALIGN,
> > + GFP_ATOMIC);
> > + else
> > + sgt_buf = sgt_cache->buf[--sgt_cache->count];
> > + if (unlikely(!sgt_buf))
> > + return -ENOMEM;
> > +
> > + sgt_buf = PTR_ALIGN(sgt_buf, DPAA2_ETH_TX_BUF_ALIGN);
> > + sgt = (struct dpaa2_sg_entry *)(sgt_buf + priv->tx_data_offset);
> > +
> > + addr = dma_map_single(dev, skb->data, skb->len, DMA_BIDIRECTIONAL);
> > + if (unlikely(dma_mapping_error(dev, addr))) {
> > + err = -ENOMEM;
> > + goto data_map_failed;
> > + }
> > +
> > + /* Fill in the HW SGT structure */
> > + dpaa2_sg_set_addr(sgt, addr);
> > + dpaa2_sg_set_len(sgt, skb->len);
> > + dpaa2_sg_set_final(sgt, true);
> > +
> > + /* Store the skb backpointer in the SGT buffer */
> > + swa = (struct dpaa2_eth_swa *)sgt_buf;
> > + swa->type = DPAA2_ETH_SWA_SINGLE;
> > + swa->single.skb = skb;
> > + swa->sg.sgt_size = sgt_buf_size;
> > +
> > + /* Separately map the SGT buffer */
> > + sgt_addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_BIDIRECTIONAL);
> > + if (unlikely(dma_mapping_error(dev, sgt_addr))) {
> > + err = -ENOMEM;
> > + goto sgt_map_failed;
> > + }
> > +
> > + dpaa2_fd_set_offset(fd, priv->tx_data_offset);
> > + dpaa2_fd_set_format(fd, dpaa2_fd_sg);
> > + dpaa2_fd_set_addr(fd, sgt_addr);
> > + dpaa2_fd_set_len(fd, skb->len);
> > + dpaa2_fd_set_ctrl(fd, FD_CTRL_PTA);
> > +
> > + if (priv->tx_tstamp && skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
> > + enable_tx_tstamp(fd, sgt_buf);
> > +
> > + return 0;
> > +
> > +sgt_map_failed:
> > + dma_unmap_single(dev, addr, skb->len, DMA_BIDIRECTIONAL);
> > +data_map_failed:
> > + if (sgt_cache->count >= DPAA2_ETH_SGT_CACHE_SIZE)
> > + kfree(sgt_buf);
> > + else
> > + sgt_cache->buf[sgt_cache->count++] = sgt_buf;
> > +
> > + return err;
> > +}
> > +
> > /* Create a frame descriptor based on a linear skb */
> > static int build_single_fd(struct dpaa2_eth_priv *priv,
> > struct sk_buff *skb,
> > @@ -743,13 +823,16 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
> > const struct dpaa2_fd *fd, bool in_napi)
> > {
> > struct device *dev = priv->net_dev->dev.parent;
> > - dma_addr_t fd_addr;
> > + dma_addr_t fd_addr, sg_addr;
> > struct sk_buff *skb = NULL;
> > unsigned char *buffer_start;
> > struct dpaa2_eth_swa *swa;
> > u8 fd_format = dpaa2_fd_get_format(fd);
> > u32 fd_len = dpaa2_fd_get_len(fd);
> >
> > + struct dpaa2_eth_sgt_cache *sgt_cache;
> > + struct dpaa2_sg_entry *sgt;
> > +
> > fd_addr = dpaa2_fd_get_addr(fd);
> > buffer_start = dpaa2_iova_to_virt(priv->iommu_domain, fd_addr);
> > swa = (struct dpaa2_eth_swa *)buffer_start;
> > @@ -769,16 +852,29 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
> > DMA_BIDIRECTIONAL);
> > }
> > } else if (fd_format == dpaa2_fd_sg) {
> > - skb = swa->sg.skb;
> > + if (swa->type == DPAA2_ETH_SWA_SG) {
> > + skb = swa->sg.skb;
> > +
> > + /* Unmap the scatterlist */
> > + dma_unmap_sg(dev, swa->sg.scl, swa->sg.num_sg,
> > + DMA_BIDIRECTIONAL);
> > + kfree(swa->sg.scl);
> >
> > - /* Unmap the scatterlist */
> > - dma_unmap_sg(dev, swa->sg.scl, swa->sg.num_sg,
> > - DMA_BIDIRECTIONAL);
> > - kfree(swa->sg.scl);
> > + /* Unmap the SGT buffer */
> > + dma_unmap_single(dev, fd_addr, swa->sg.sgt_size,
> > + DMA_BIDIRECTIONAL);
> > + } else {
> > + skb = swa->single.skb;
> >
> > - /* Unmap the SGT buffer */
> > - dma_unmap_single(dev, fd_addr, swa->sg.sgt_size,
> > - DMA_BIDIRECTIONAL);
> > + /* Unmap the SGT Buffer */
> > + dma_unmap_single(dev, fd_addr, swa->single.sgt_size,
> > + DMA_BIDIRECTIONAL);
> > +
> > + sgt = (struct dpaa2_sg_entry *)(buffer_start +
> > + priv->tx_data_offset);
> > + sg_addr = dpaa2_sg_get_addr(sgt);
> > + dma_unmap_single(dev, sg_addr, skb->len, DMA_BIDIRECTIONAL);
> > + }
> > } else {
> > netdev_dbg(priv->net_dev, "Invalid FD format\n");
> > return;
> > @@ -808,8 +904,17 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
> > }
> >
> > /* Free SGT buffer allocated on tx */
> > - if (fd_format != dpaa2_fd_single)
> > - skb_free_frag(buffer_start);
> > + if (fd_format != dpaa2_fd_single) {
> > + sgt_cache = this_cpu_ptr(priv->sgt_cache);
> > + if (swa->type == DPAA2_ETH_SWA_SG) {
> > + skb_free_frag(buffer_start);
> > + } else {
> > + if (sgt_cache->count >= DPAA2_ETH_SGT_CACHE_SIZE)
> > + kfree(buffer_start);
> > + else
> > + sgt_cache->buf[sgt_cache->count++] = buffer_start;
> > + }
> > + }
> >
> > /* Move on with skb release */
> > napi_consume_skb(skb, in_napi);
> > @@ -833,22 +938,6 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
> > percpu_extras = this_cpu_ptr(priv->percpu_extras);
> >
> > needed_headroom = dpaa2_eth_needed_headroom(priv, skb);
> > - if (skb_headroom(skb) < needed_headroom) {
> > - struct sk_buff *ns;
> > -
> > - ns = skb_realloc_headroom(skb, needed_headroom);
> > - if (unlikely(!ns)) {
> > - percpu_stats->tx_dropped++;
> > - goto err_alloc_headroom;
> > - }
> > - percpu_extras->tx_reallocs++;
> > -
> > - if (skb->sk)
> > - skb_set_owner_w(ns, skb->sk);
> > -
> > - dev_kfree_skb(skb);
> > - skb = ns;
> > - }
> >
> > /* We'll be holding a back-reference to the skb until Tx Confirmation;
> > * we don't want that overwritten by a concurrent Tx with a cloned skb.
> > @@ -867,6 +956,10 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
> > err = build_sg_fd(priv, skb, &fd);
> > percpu_extras->tx_sg_frames++;
> > percpu_extras->tx_sg_bytes += skb->len;
> > + } else if (skb_headroom(skb) < needed_headroom) {
> > + err = build_sg_fd_single_buf(priv, skb, &fd);
> > + percpu_extras->tx_sg_frames++;
> > + percpu_extras->tx_sg_bytes += skb->len;
> > } else {
> > err = build_single_fd(priv, skb, &fd);
> > }
> > @@ -924,7 +1017,6 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
> > return NETDEV_TX_OK;
> >
> > err_build_fd:
> > -err_alloc_headroom:
> > dev_kfree_skb(skb);
> >
> > return NETDEV_TX_OK;
> > @@ -1161,6 +1253,22 @@ static int refill_pool(struct dpaa2_eth_priv *priv,
> > return 0;
> > }
> >
> > +static void dpaa2_eth_sgt_cache_drain(struct dpaa2_eth_priv *priv)
> > +{
> > + struct dpaa2_eth_sgt_cache *sgt_cache;
> > + u16 count;
> > + int k, i;
> > +
> > + for_each_online_cpu(k) {
> > + sgt_cache = per_cpu_ptr(priv->sgt_cache, k);
> > + count = sgt_cache->count;
> > +
> > + for (i = 0; i < count; i++)
> > + kfree(sgt_cache->buf[i]);
> > + sgt_cache->count = 0;
> > + }
> > +}
> > +
> > static int pull_channel(struct dpaa2_eth_channel *ch)
> > {
> > int err;
> > @@ -1562,6 +1670,9 @@ static int dpaa2_eth_stop(struct net_device *net_dev)
> > /* Empty the buffer pool */
> > drain_pool(priv);
> >
> > + /* Empty the Scatter-Gather Buffer cache */
> > + dpaa2_eth_sgt_cache_drain(priv);
> > +
> > return 0;
> > }
> >
> > @@ -3846,6 +3957,13 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
> > goto err_alloc_percpu_extras;
> > }
> >
> > + priv->sgt_cache = alloc_percpu(*priv->sgt_cache);
> > + if (!priv->sgt_cache) {
> > + dev_err(dev, "alloc_percpu(sgt_cache) failed\n");
> > + err = -ENOMEM;
> > + goto err_alloc_sgt_cache;
> > + }
> > +
> > err = netdev_init(net_dev);
> > if (err)
> > goto err_netdev_init;
> > @@ -3914,6 +4032,8 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
> > err_alloc_rings:
> > err_csum:
> > err_netdev_init:
> > + free_percpu(priv->sgt_cache);
> > +err_alloc_sgt_cache:
> > free_percpu(priv->percpu_extras);
> > err_alloc_percpu_extras:
> > free_percpu(priv->percpu_stats);
> > @@ -3959,6 +4079,7 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
> > fsl_mc_free_irqs(ls_dev);
> >
> > free_rings(priv);
> > + free_percpu(priv->sgt_cache);
> > free_percpu(priv->percpu_stats);
> > free_percpu(priv->percpu_extras);
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
> > index 2d7ada0f0dbd..9e4ceb92f240 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
> > @@ -125,6 +125,7 @@ struct dpaa2_eth_swa {
> > union {
> > struct {
> > struct sk_buff *skb;
> > + int sgt_size;
> > } single;
> > struct {
> > struct sk_buff *skb;
> > @@ -282,7 +283,6 @@ struct dpaa2_eth_drv_stats {
> > __u64 tx_conf_bytes;
> > __u64 tx_sg_frames;
> > __u64 tx_sg_bytes;
> > - __u64 tx_reallocs;
> > __u64 rx_sg_frames;
> > __u64 rx_sg_bytes;
> > /* Enqueues retried due to portal busy */
> > @@ -395,6 +395,12 @@ struct dpaa2_eth_cls_rule {
> > u8 in_use;
> > };
> >
> > +#define DPAA2_ETH_SGT_CACHE_SIZE 256
> > +struct dpaa2_eth_sgt_cache {
> > + void *buf[DPAA2_ETH_SGT_CACHE_SIZE];
> > + u16 count;
> > +};
> > +
> > /* Driver private data */
> > struct dpaa2_eth_priv {
> > struct net_device *net_dev;
> > @@ -409,6 +415,7 @@ struct dpaa2_eth_priv {
> >
> > u8 num_channels;
> > struct dpaa2_eth_channel *channel[DPAA2_ETH_MAX_DPCONS];
> > + struct dpaa2_eth_sgt_cache __percpu *sgt_cache;
> >
> > struct dpni_attr dpni_attrs;
> > u16 dpni_ver_major;
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
> > index e88269fe3de7..c4cbbcaa9a3f 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
> > @@ -43,7 +43,6 @@ static char dpaa2_ethtool_extras[][ETH_GSTRING_LEN] = {
> > "[drv] tx conf bytes",
> > "[drv] tx sg frames",
> > "[drv] tx sg bytes",
> > - "[drv] tx realloc frames",
> > "[drv] rx sg frames",
> > "[drv] rx sg bytes",
> > "[drv] enqueue portal busy",
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing
2020-12-10 18:06 ` [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing Ioana Ciornei
@ 2020-12-11 7:21 ` Jon Nettleton
2020-12-11 14:01 ` Ioana Ciornei
1 sibling, 0 replies; 8+ messages in thread
From: Jon Nettleton @ 2020-12-11 7:21 UTC (permalink / raw)
To: Ioana Ciornei
Cc: Daniel Thompson, linux-netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
On Thu, Dec 10, 2020 at 7:08 PM Ioana Ciornei <ioana.ciornei@nxp.com> wrote:
>
> [Added also the netdev mailing list, I haven't heard of linux-netdev
> before but kept it]
>
> On Thu, Dec 10, 2020 at 05:31:56PM +0000, Daniel Thompson wrote:
> > Hi Ioana
>
> Hi Daniel,
>
> >
> > On Mon, Jun 29, 2020 at 06:47:11PM +0000, Ioana Ciornei wrote:
> > > Instead of realloc-ing the skb on the Tx path when the provided headroom
> > > is smaller than the HW requirements, create a Scatter/Gather frame
> > > descriptor with only one entry.
> > >
> > > Remove the '[drv] tx realloc frames' counter exposed previously through
> > > ethtool since it is no longer used.
> > >
> > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > > ---
> >
> > I've been chasing down a networking regression on my LX2160A board
> > (Honeycomb LX2K based on CEx7 LX2160A COM) that first appeared in v5.9.
> >
> > It makes the board unreliable opening outbound connections meaning
> > things like `apt update` or `git fetch` often can't open the connection.
> > It does not happen all the time but is sufficient to make the boards
> > built-in networking useless for workstation use.
> >
> > The problem is strongly linked to warnings in the logs so I used the
> > warnings to bisect down to locate the cause of the regression and it
> > pinpointed this patch. I have confirmed that in both v5.9 and v5.10-rc7
> > that reverting this patch (and fixing up the merge issues) fixes the
> > regression and the warnings stop appearing.
> >
> > A typical example of the warning is below (io-pgtable-arm.c:281 is an
> > error path that I guess would cause dma_map_page_attrs() to return
> > an error):
> >
> > [ 714.464927] WARNING: CPU: 13 PID: 0 at
> > drivers/iommu/io-pgtable-arm.c:281 __arm_lpae_map+0x2d4/0x30c
> > [ 714.464930] Modules linked in: snd_seq_dummy(E) snd_hrtimer(E)
> > snd_seq(E) snd_seq_device(E) snd_timer(E) snd(E) soundcore(E) bridge(E)
> > stp(E) llc(E) rfkill(E) caam_jr(E) crypto_engine(E) rng_core(E)
> > joydev(E) evdev(E) dpaa2_caam(E) caamhash_desc(E) caamalg_desc(E)
> > authenc(E) libdes(E) dpaa2_console(E) ofpart(E) caam(E) sg(E) error(E)
> > lm90(E) at24(E) spi_nor(E) mtd(E) sbsa_gwdt(E) qoriq_thermal(E)
> > layerscape_edac_mod(E) qoriq_cpufreq(E) drm(E) fuse(E) configfs(E)
> > ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E)
> > mbcache(E) jbd2(E) hid_generic(E) usbhid(E) hid(E) dm_crypt(E) dm_mod(E)
> > sd_mod(E) fsl_dpaa2_ptp(E) ptp_qoriq(E) fsl_dpaa2_eth(E)
> > xhci_plat_hcd(E) xhci_hcd(E) usbcore(E) aes_ce_blk(E) crypto_simd(E)
> > cryptd(E) aes_ce_cipher(E) ghash_ce(E) gf128mul(E) at803x(E) libaes(E)
> > fsl_mc_dpio(E) pcs_lynx(E) rtc_pcf2127(E) sha2_ce(E) phylink(E)
> > xgmac_mdio(E) regmap_spi(E) of_mdio(E) sha256_arm64(E)
> > i2c_mux_pca954x(E) fixed_phy(E) i2c_mux(E) sha1_ce(E) ptp(E) libphy(E)
> > [ 714.465131] pps_core(E) ahci_qoriq(E) libahci_platform(E) nvme(E)
> > libahci(E) nvme_core(E) t10_pi(E) libata(E) crc_t10dif(E)
> > crct10dif_generic(E) crct10dif_common(E) dwc3(E) scsi_mod(E) udc_core(E)
> > roles(E) ulpi(E) sdhci_of_esdhc(E) sdhci_pltfm(E) sdhci(E)
> > spi_nxp_fspi(E) i2c_imx(E) fixed(E)
> > [ 714.465192] CPU: 13 PID: 0 Comm: swapper/13 Tainted: G W E
> > 5.10.0-rc7-00001-gba98d13279ca #52
> > [ 714.465196] Hardware name: SolidRun LX2160A Honeycomb (DT)
> > [ 714.465202] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> > [ 714.465207] pc : __arm_lpae_map+0x2d4/0x30c
> > [ 714.465211] lr : __arm_lpae_map+0x114/0x30c
> > [ 714.465215] sp : ffff80001006b340
> > [ 714.465219] x29: ffff80001006b340 x28: 0000002086538003
> > [ 714.465227] x27: 0000000000000a20 x26: 0000000000001000
> > [ 714.465236] x25: 0000000000000f44 x24: 00000020adf8d000
> > [ 714.465245] x23: 0000000000000001 x22: 0000fffffaeca000
> > [ 714.465253] x21: 0000000000000003 x20: ffff19b60d64d200
> > [ 714.465261] x19: 00000000000000ca x18: 0000000000000000
> > [ 714.465270] x17: 0000000000000000 x16: ffffcccb7cf3ca20
> > [ 714.465278] x15: 0000000000000000 x14: 0000000000000000
> > [ 714.465286] x13: 0000000000000003 x12: 0000000000000010
> > [ 714.465294] x11: 0000000000000000 x10: 0000000000000002
> > [ 714.465302] x9 : ffffcccb7d5b6e78 x8 : 00000000000001ff
> > [ 714.465311] x7 : ffff19b606538650 x6 : ffff19b606538000
> > [ 714.465319] x5 : 0000000000000009 x4 : 0000000000000f44
> > [ 714.465327] x3 : 0000000000001000 x2 : 00000020adf8d000
> > [ 714.465335] x1 : 0000000000000002 x0 : 0000000000000003
> > [ 714.465343] Call trace:
> > [ 714.465348] __arm_lpae_map+0x2d4/0x30c
> > [ 714.465353] __arm_lpae_map+0x114/0x30c
> > [ 714.465357] __arm_lpae_map+0x114/0x30c
> > [ 714.465362] __arm_lpae_map+0x114/0x30c
> > [ 714.465366] arm_lpae_map+0xf4/0x180
> > [ 714.465373] arm_smmu_map+0x4c/0xc0
> > [ 714.465379] __iommu_map+0x100/0x2bc
> > [ 714.465385] iommu_map_atomic+0x20/0x30
> > [ 714.465391] __iommu_dma_map+0xb0/0x110
> > [ 714.465397] iommu_dma_map_page+0xb8/0x120
> > [ 714.465404] dma_map_page_attrs+0x1a8/0x210
> > [ 714.465413] __dpaa2_eth_tx+0x384/0xbd0 [fsl_dpaa2_eth]
> > [ 714.465421] dpaa2_eth_tx+0x84/0x134 [fsl_dpaa2_eth]
> > [ 714.465427] dev_hard_start_xmit+0x10c/0x2b0
> > [ 714.465433] sch_direct_xmit+0x1a0/0x550
> > [ 714.465438] __qdisc_run+0x140/0x670
> > [ 714.465443] __dev_queue_xmit+0x6c4/0xa74
> > [ 714.465449] dev_queue_xmit+0x20/0x2c
> > [ 714.465463] br_dev_queue_push_xmit+0xc4/0x1a0 [bridge]
> > [ 714.465476] br_forward_finish+0xdc/0xf0 [bridge]
> > [ 714.465489] __br_forward+0x160/0x1c0 [bridge]
> > [ 714.465502] br_forward+0x13c/0x160 [bridge]
> > [ 714.465514] br_dev_xmit+0x228/0x3b0 [bridge]
> > [ 714.465520] dev_hard_start_xmit+0x10c/0x2b0
> > [ 714.465526] __dev_queue_xmit+0x8f0/0xa74
> > [ 714.465531] dev_queue_xmit+0x20/0x2c
> > [ 714.465538] arp_xmit+0xc0/0xd0
> > [ 714.465544] arp_send_dst+0x78/0xa0
> > [ 714.465550] arp_solicit+0xf4/0x260
> > [ 714.465554] neigh_probe+0x64/0xb0
> > [ 714.465560] neigh_timer_handler+0x2f4/0x400
> > [ 714.465566] call_timer_fn+0x3c/0x184
> > [ 714.465572] __run_timers.part.0+0x2bc/0x370
> > [ 714.465578] run_timer_softirq+0x48/0x80
> > [ 714.465583] __do_softirq+0x120/0x36c
> > [ 714.465589] irq_exit+0xac/0x100
> > [ 714.465596] __handle_domain_irq+0x8c/0xf0
> > [ 714.465600] gic_handle_irq+0xcc/0x14c
> > [ 714.465605] el1_irq+0xc4/0x180
> > [ 714.465610] arch_cpu_idle+0x18/0x30
> > [ 714.465617] default_idle_call+0x4c/0x180
> > [ 714.465623] do_idle+0x238/0x2b0
> > [ 714.465629] cpu_startup_entry+0x30/0xa0
> > [ 714.465636] secondary_start_kernel+0x134/0x180
> > [ 714.465640] ---[ end trace a84a7f61b559005f ]---
> >
> >
> > Given it is the iommu code that is provoking the warning I should
> > probably mention that the board I have requires
> > arm-smmu.disable_bypass=0 on the kernel command line in order to boot.
> > Also if it matters I am running the latest firmware from Solidrun
> > which is based on LSDK-20.04.
> >
>
> Hmmm, from what I remember I think I tested this with the smmu bypassed
> so that is why I didn't catch it.
>
> > Is there any reason for this code not to be working for LX2160A?
>
> I wouldn't expect this to be LX2160A specific but rather a bug in the
> implementation.. sorry.
>
> Let me reproduce it and see if I can get to the bottom of it and I will
> get back with some more info.
>
> Ioana
Ioana,
I reported this issue to Calvin last week. I can verify that reverting that
change also fixes the issue for me.
-Jon
>
> >
> > Daniel.
> >
> >
> > PS A few months have gone by so I decided not to trim the patch out
> > of this reply so you don't have to go digging!
> >
> >
> >
> > > .../freescale/dpaa2/dpaa2-eth-debugfs.c | 7 +-
> > > .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 177 +++++++++++++++---
> > > .../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 9 +-
> > > .../ethernet/freescale/dpaa2/dpaa2-ethtool.c | 1 -
> > > 4 files changed, 160 insertions(+), 34 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
> > > index 2880ca02d7e7..5cb357c74dec 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
> > > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
> > > @@ -19,14 +19,14 @@ static int dpaa2_dbg_cpu_show(struct seq_file *file, void *offset)
> > > int i;
> > >
> > > seq_printf(file, "Per-CPU stats for %s\n", priv->net_dev->name);
> > > - seq_printf(file, "%s%16s%16s%16s%16s%16s%16s%16s%16s%16s\n",
> > > + seq_printf(file, "%s%16s%16s%16s%16s%16s%16s%16s%16s\n",
> > > "CPU", "Rx", "Rx Err", "Rx SG", "Tx", "Tx Err", "Tx conf",
> > > - "Tx SG", "Tx realloc", "Enq busy");
> > > + "Tx SG", "Enq busy");
> > >
> > > for_each_online_cpu(i) {
> > > stats = per_cpu_ptr(priv->percpu_stats, i);
> > > extras = per_cpu_ptr(priv->percpu_extras, i);
> > > - seq_printf(file, "%3d%16llu%16llu%16llu%16llu%16llu%16llu%16llu%16llu%16llu\n",
> > > + seq_printf(file, "%3d%16llu%16llu%16llu%16llu%16llu%16llu%16llu%16llu\n",
> > > i,
> > > stats->rx_packets,
> > > stats->rx_errors,
> > > @@ -35,7 +35,6 @@ static int dpaa2_dbg_cpu_show(struct seq_file *file, void *offset)
> > > stats->tx_errors,
> > > extras->tx_conf_frames,
> > > extras->tx_sg_frames,
> > > - extras->tx_reallocs,
> > > extras->tx_portal_busy);
> > > }
> > >
> > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > > index 712bbfdbe7d7..4a264b75c035 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > > @@ -685,6 +685,86 @@ static int build_sg_fd(struct dpaa2_eth_priv *priv,
> > > return err;
> > > }
> > >
> > > +/* Create a SG frame descriptor based on a linear skb.
> > > + *
> > > + * This function is used on the Tx path when the skb headroom is not large
> > > + * enough for the HW requirements, thus instead of realloc-ing the skb we
> > > + * create a SG frame descriptor with only one entry.
> > > + */
> > > +static int build_sg_fd_single_buf(struct dpaa2_eth_priv *priv,
> > > + struct sk_buff *skb,
> > > + struct dpaa2_fd *fd)
> > > +{
> > > + struct device *dev = priv->net_dev->dev.parent;
> > > + struct dpaa2_eth_sgt_cache *sgt_cache;
> > > + struct dpaa2_sg_entry *sgt;
> > > + struct dpaa2_eth_swa *swa;
> > > + dma_addr_t addr, sgt_addr;
> > > + void *sgt_buf = NULL;
> > > + int sgt_buf_size;
> > > + int err;
> > > +
> > > + /* Prepare the HW SGT structure */
> > > + sgt_cache = this_cpu_ptr(priv->sgt_cache);
> > > + sgt_buf_size = priv->tx_data_offset + sizeof(struct dpaa2_sg_entry);
> > > +
> > > + if (sgt_cache->count == 0)
> > > + sgt_buf = kzalloc(sgt_buf_size + DPAA2_ETH_TX_BUF_ALIGN,
> > > + GFP_ATOMIC);
> > > + else
> > > + sgt_buf = sgt_cache->buf[--sgt_cache->count];
> > > + if (unlikely(!sgt_buf))
> > > + return -ENOMEM;
> > > +
> > > + sgt_buf = PTR_ALIGN(sgt_buf, DPAA2_ETH_TX_BUF_ALIGN);
> > > + sgt = (struct dpaa2_sg_entry *)(sgt_buf + priv->tx_data_offset);
> > > +
> > > + addr = dma_map_single(dev, skb->data, skb->len, DMA_BIDIRECTIONAL);
> > > + if (unlikely(dma_mapping_error(dev, addr))) {
> > > + err = -ENOMEM;
> > > + goto data_map_failed;
> > > + }
> > > +
> > > + /* Fill in the HW SGT structure */
> > > + dpaa2_sg_set_addr(sgt, addr);
> > > + dpaa2_sg_set_len(sgt, skb->len);
> > > + dpaa2_sg_set_final(sgt, true);
> > > +
> > > + /* Store the skb backpointer in the SGT buffer */
> > > + swa = (struct dpaa2_eth_swa *)sgt_buf;
> > > + swa->type = DPAA2_ETH_SWA_SINGLE;
> > > + swa->single.skb = skb;
> > > + swa->sg.sgt_size = sgt_buf_size;
> > > +
> > > + /* Separately map the SGT buffer */
> > > + sgt_addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_BIDIRECTIONAL);
> > > + if (unlikely(dma_mapping_error(dev, sgt_addr))) {
> > > + err = -ENOMEM;
> > > + goto sgt_map_failed;
> > > + }
> > > +
> > > + dpaa2_fd_set_offset(fd, priv->tx_data_offset);
> > > + dpaa2_fd_set_format(fd, dpaa2_fd_sg);
> > > + dpaa2_fd_set_addr(fd, sgt_addr);
> > > + dpaa2_fd_set_len(fd, skb->len);
> > > + dpaa2_fd_set_ctrl(fd, FD_CTRL_PTA);
> > > +
> > > + if (priv->tx_tstamp && skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
> > > + enable_tx_tstamp(fd, sgt_buf);
> > > +
> > > + return 0;
> > > +
> > > +sgt_map_failed:
> > > + dma_unmap_single(dev, addr, skb->len, DMA_BIDIRECTIONAL);
> > > +data_map_failed:
> > > + if (sgt_cache->count >= DPAA2_ETH_SGT_CACHE_SIZE)
> > > + kfree(sgt_buf);
> > > + else
> > > + sgt_cache->buf[sgt_cache->count++] = sgt_buf;
> > > +
> > > + return err;
> > > +}
> > > +
> > > /* Create a frame descriptor based on a linear skb */
> > > static int build_single_fd(struct dpaa2_eth_priv *priv,
> > > struct sk_buff *skb,
> > > @@ -743,13 +823,16 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
> > > const struct dpaa2_fd *fd, bool in_napi)
> > > {
> > > struct device *dev = priv->net_dev->dev.parent;
> > > - dma_addr_t fd_addr;
> > > + dma_addr_t fd_addr, sg_addr;
> > > struct sk_buff *skb = NULL;
> > > unsigned char *buffer_start;
> > > struct dpaa2_eth_swa *swa;
> > > u8 fd_format = dpaa2_fd_get_format(fd);
> > > u32 fd_len = dpaa2_fd_get_len(fd);
> > >
> > > + struct dpaa2_eth_sgt_cache *sgt_cache;
> > > + struct dpaa2_sg_entry *sgt;
> > > +
> > > fd_addr = dpaa2_fd_get_addr(fd);
> > > buffer_start = dpaa2_iova_to_virt(priv->iommu_domain, fd_addr);
> > > swa = (struct dpaa2_eth_swa *)buffer_start;
> > > @@ -769,16 +852,29 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
> > > DMA_BIDIRECTIONAL);
> > > }
> > > } else if (fd_format == dpaa2_fd_sg) {
> > > - skb = swa->sg.skb;
> > > + if (swa->type == DPAA2_ETH_SWA_SG) {
> > > + skb = swa->sg.skb;
> > > +
> > > + /* Unmap the scatterlist */
> > > + dma_unmap_sg(dev, swa->sg.scl, swa->sg.num_sg,
> > > + DMA_BIDIRECTIONAL);
> > > + kfree(swa->sg.scl);
> > >
> > > - /* Unmap the scatterlist */
> > > - dma_unmap_sg(dev, swa->sg.scl, swa->sg.num_sg,
> > > - DMA_BIDIRECTIONAL);
> > > - kfree(swa->sg.scl);
> > > + /* Unmap the SGT buffer */
> > > + dma_unmap_single(dev, fd_addr, swa->sg.sgt_size,
> > > + DMA_BIDIRECTIONAL);
> > > + } else {
> > > + skb = swa->single.skb;
> > >
> > > - /* Unmap the SGT buffer */
> > > - dma_unmap_single(dev, fd_addr, swa->sg.sgt_size,
> > > - DMA_BIDIRECTIONAL);
> > > + /* Unmap the SGT Buffer */
> > > + dma_unmap_single(dev, fd_addr, swa->single.sgt_size,
> > > + DMA_BIDIRECTIONAL);
> > > +
> > > + sgt = (struct dpaa2_sg_entry *)(buffer_start +
> > > + priv->tx_data_offset);
> > > + sg_addr = dpaa2_sg_get_addr(sgt);
> > > + dma_unmap_single(dev, sg_addr, skb->len, DMA_BIDIRECTIONAL);
> > > + }
> > > } else {
> > > netdev_dbg(priv->net_dev, "Invalid FD format\n");
> > > return;
> > > @@ -808,8 +904,17 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
> > > }
> > >
> > > /* Free SGT buffer allocated on tx */
> > > - if (fd_format != dpaa2_fd_single)
> > > - skb_free_frag(buffer_start);
> > > + if (fd_format != dpaa2_fd_single) {
> > > + sgt_cache = this_cpu_ptr(priv->sgt_cache);
> > > + if (swa->type == DPAA2_ETH_SWA_SG) {
> > > + skb_free_frag(buffer_start);
> > > + } else {
> > > + if (sgt_cache->count >= DPAA2_ETH_SGT_CACHE_SIZE)
> > > + kfree(buffer_start);
> > > + else
> > > + sgt_cache->buf[sgt_cache->count++] = buffer_start;
> > > + }
> > > + }
> > >
> > > /* Move on with skb release */
> > > napi_consume_skb(skb, in_napi);
> > > @@ -833,22 +938,6 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
> > > percpu_extras = this_cpu_ptr(priv->percpu_extras);
> > >
> > > needed_headroom = dpaa2_eth_needed_headroom(priv, skb);
> > > - if (skb_headroom(skb) < needed_headroom) {
> > > - struct sk_buff *ns;
> > > -
> > > - ns = skb_realloc_headroom(skb, needed_headroom);
> > > - if (unlikely(!ns)) {
> > > - percpu_stats->tx_dropped++;
> > > - goto err_alloc_headroom;
> > > - }
> > > - percpu_extras->tx_reallocs++;
> > > -
> > > - if (skb->sk)
> > > - skb_set_owner_w(ns, skb->sk);
> > > -
> > > - dev_kfree_skb(skb);
> > > - skb = ns;
> > > - }
> > >
> > > /* We'll be holding a back-reference to the skb until Tx Confirmation;
> > > * we don't want that overwritten by a concurrent Tx with a cloned skb.
> > > @@ -867,6 +956,10 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
> > > err = build_sg_fd(priv, skb, &fd);
> > > percpu_extras->tx_sg_frames++;
> > > percpu_extras->tx_sg_bytes += skb->len;
> > > + } else if (skb_headroom(skb) < needed_headroom) {
> > > + err = build_sg_fd_single_buf(priv, skb, &fd);
> > > + percpu_extras->tx_sg_frames++;
> > > + percpu_extras->tx_sg_bytes += skb->len;
> > > } else {
> > > err = build_single_fd(priv, skb, &fd);
> > > }
> > > @@ -924,7 +1017,6 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
> > > return NETDEV_TX_OK;
> > >
> > > err_build_fd:
> > > -err_alloc_headroom:
> > > dev_kfree_skb(skb);
> > >
> > > return NETDEV_TX_OK;
> > > @@ -1161,6 +1253,22 @@ static int refill_pool(struct dpaa2_eth_priv *priv,
> > > return 0;
> > > }
> > >
> > > +static void dpaa2_eth_sgt_cache_drain(struct dpaa2_eth_priv *priv)
> > > +{
> > > + struct dpaa2_eth_sgt_cache *sgt_cache;
> > > + u16 count;
> > > + int k, i;
> > > +
> > > + for_each_online_cpu(k) {
> > > + sgt_cache = per_cpu_ptr(priv->sgt_cache, k);
> > > + count = sgt_cache->count;
> > > +
> > > + for (i = 0; i < count; i++)
> > > + kfree(sgt_cache->buf[i]);
> > > + sgt_cache->count = 0;
> > > + }
> > > +}
> > > +
> > > static int pull_channel(struct dpaa2_eth_channel *ch)
> > > {
> > > int err;
> > > @@ -1562,6 +1670,9 @@ static int dpaa2_eth_stop(struct net_device *net_dev)
> > > /* Empty the buffer pool */
> > > drain_pool(priv);
> > >
> > > + /* Empty the Scatter-Gather Buffer cache */
> > > + dpaa2_eth_sgt_cache_drain(priv);
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -3846,6 +3957,13 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
> > > goto err_alloc_percpu_extras;
> > > }
> > >
> > > + priv->sgt_cache = alloc_percpu(*priv->sgt_cache);
> > > + if (!priv->sgt_cache) {
> > > + dev_err(dev, "alloc_percpu(sgt_cache) failed\n");
> > > + err = -ENOMEM;
> > > + goto err_alloc_sgt_cache;
> > > + }
> > > +
> > > err = netdev_init(net_dev);
> > > if (err)
> > > goto err_netdev_init;
> > > @@ -3914,6 +4032,8 @@ static int dpaa2_eth_probe(struct fsl_mc_device *dpni_dev)
> > > err_alloc_rings:
> > > err_csum:
> > > err_netdev_init:
> > > + free_percpu(priv->sgt_cache);
> > > +err_alloc_sgt_cache:
> > > free_percpu(priv->percpu_extras);
> > > err_alloc_percpu_extras:
> > > free_percpu(priv->percpu_stats);
> > > @@ -3959,6 +4079,7 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
> > > fsl_mc_free_irqs(ls_dev);
> > >
> > > free_rings(priv);
> > > + free_percpu(priv->sgt_cache);
> > > free_percpu(priv->percpu_stats);
> > > free_percpu(priv->percpu_extras);
> > >
> > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
> > > index 2d7ada0f0dbd..9e4ceb92f240 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
> > > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h
> > > @@ -125,6 +125,7 @@ struct dpaa2_eth_swa {
> > > union {
> > > struct {
> > > struct sk_buff *skb;
> > > + int sgt_size;
> > > } single;
> > > struct {
> > > struct sk_buff *skb;
> > > @@ -282,7 +283,6 @@ struct dpaa2_eth_drv_stats {
> > > __u64 tx_conf_bytes;
> > > __u64 tx_sg_frames;
> > > __u64 tx_sg_bytes;
> > > - __u64 tx_reallocs;
> > > __u64 rx_sg_frames;
> > > __u64 rx_sg_bytes;
> > > /* Enqueues retried due to portal busy */
> > > @@ -395,6 +395,12 @@ struct dpaa2_eth_cls_rule {
> > > u8 in_use;
> > > };
> > >
> > > +#define DPAA2_ETH_SGT_CACHE_SIZE 256
> > > +struct dpaa2_eth_sgt_cache {
> > > + void *buf[DPAA2_ETH_SGT_CACHE_SIZE];
> > > + u16 count;
> > > +};
> > > +
> > > /* Driver private data */
> > > struct dpaa2_eth_priv {
> > > struct net_device *net_dev;
> > > @@ -409,6 +415,7 @@ struct dpaa2_eth_priv {
> > >
> > > u8 num_channels;
> > > struct dpaa2_eth_channel *channel[DPAA2_ETH_MAX_DPCONS];
> > > + struct dpaa2_eth_sgt_cache __percpu *sgt_cache;
> > >
> > > struct dpni_attr dpni_attrs;
> > > u16 dpni_ver_major;
> > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
> > > index e88269fe3de7..c4cbbcaa9a3f 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
> > > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c
> > > @@ -43,7 +43,6 @@ static char dpaa2_ethtool_extras[][ETH_GSTRING_LEN] = {
> > > "[drv] tx conf bytes",
> > > "[drv] tx sg frames",
> > > "[drv] tx sg bytes",
> > > - "[drv] tx realloc frames",
> > > "[drv] rx sg frames",
> > > "[drv] rx sg bytes",
> > > "[drv] enqueue portal busy",
> > > --
> > > 2.25.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing
2020-12-10 18:06 ` [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing Ioana Ciornei
2020-12-11 7:21 ` Jon Nettleton
@ 2020-12-11 14:01 ` Ioana Ciornei
2020-12-11 16:29 ` Daniel Thompson
1 sibling, 1 reply; 8+ messages in thread
From: Ioana Ciornei @ 2020-12-11 14:01 UTC (permalink / raw)
To: Daniel Thompson; +Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
On Thu, Dec 10, 2020 at 08:06:36PM +0200, Ioana Ciornei wrote:
> [Added also the netdev mailing list, I haven't heard of linux-netdev
> before but kept it]
>
> On Thu, Dec 10, 2020 at 05:31:56PM +0000, Daniel Thompson wrote:
> > Hi Ioana
>
> Hi Daniel,
>
> >
> > On Mon, Jun 29, 2020 at 06:47:11PM +0000, Ioana Ciornei wrote:
> > > Instead of realloc-ing the skb on the Tx path when the provided headroom
> > > is smaller than the HW requirements, create a Scatter/Gather frame
> > > descriptor with only one entry.
> > >
> > > Remove the '[drv] tx realloc frames' counter exposed previously through
> > > ethtool since it is no longer used.
> > >
> > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > > ---
> >
> > I've been chasing down a networking regression on my LX2160A board
> > (Honeycomb LX2K based on CEx7 LX2160A COM) that first appeared in v5.9.
> >
> > It makes the board unreliable opening outbound connections meaning
> > things like `apt update` or `git fetch` often can't open the connection.
> > It does not happen all the time but is sufficient to make the boards
> > built-in networking useless for workstation use.
> >
> > The problem is strongly linked to warnings in the logs so I used the
> > warnings to bisect down to locate the cause of the regression and it
> > pinpointed this patch. I have confirmed that in both v5.9 and v5.10-rc7
> > that reverting this patch (and fixing up the merge issues) fixes the
> > regression and the warnings stop appearing.
> >
> > A typical example of the warning is below (io-pgtable-arm.c:281 is an
> > error path that I guess would cause dma_map_page_attrs() to return
> > an error):
> >
> > [ 714.464927] WARNING: CPU: 13 PID: 0 at
> > drivers/iommu/io-pgtable-arm.c:281 __arm_lpae_map+0x2d4/0x30c
> > [ 714.464930] Modules linked in: snd_seq_dummy(E) snd_hrtimer(E)
> > snd_seq(E) snd_seq_device(E) snd_timer(E) snd(E) soundcore(E) bridge(E)
> > stp(E) llc(E) rfkill(E) caam_jr(E) crypto_engine(E) rng_core(E)
> > joydev(E) evdev(E) dpaa2_caam(E) caamhash_desc(E) caamalg_desc(E)
> > authenc(E) libdes(E) dpaa2_console(E) ofpart(E) caam(E) sg(E) error(E)
> > lm90(E) at24(E) spi_nor(E) mtd(E) sbsa_gwdt(E) qoriq_thermal(E)
> > layerscape_edac_mod(E) qoriq_cpufreq(E) drm(E) fuse(E) configfs(E)
> > ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E)
> > mbcache(E) jbd2(E) hid_generic(E) usbhid(E) hid(E) dm_crypt(E) dm_mod(E)
> > sd_mod(E) fsl_dpaa2_ptp(E) ptp_qoriq(E) fsl_dpaa2_eth(E)
> > xhci_plat_hcd(E) xhci_hcd(E) usbcore(E) aes_ce_blk(E) crypto_simd(E)
> > cryptd(E) aes_ce_cipher(E) ghash_ce(E) gf128mul(E) at803x(E) libaes(E)
> > fsl_mc_dpio(E) pcs_lynx(E) rtc_pcf2127(E) sha2_ce(E) phylink(E)
> > xgmac_mdio(E) regmap_spi(E) of_mdio(E) sha256_arm64(E)
> > i2c_mux_pca954x(E) fixed_phy(E) i2c_mux(E) sha1_ce(E) ptp(E) libphy(E)
> > [ 714.465131] pps_core(E) ahci_qoriq(E) libahci_platform(E) nvme(E)
> > libahci(E) nvme_core(E) t10_pi(E) libata(E) crc_t10dif(E)
> > crct10dif_generic(E) crct10dif_common(E) dwc3(E) scsi_mod(E) udc_core(E)
> > roles(E) ulpi(E) sdhci_of_esdhc(E) sdhci_pltfm(E) sdhci(E)
> > spi_nxp_fspi(E) i2c_imx(E) fixed(E)
> > [ 714.465192] CPU: 13 PID: 0 Comm: swapper/13 Tainted: G W E
> > 5.10.0-rc7-00001-gba98d13279ca #52
> > [ 714.465196] Hardware name: SolidRun LX2160A Honeycomb (DT)
> > [ 714.465202] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> > [ 714.465207] pc : __arm_lpae_map+0x2d4/0x30c
> > [ 714.465211] lr : __arm_lpae_map+0x114/0x30c
> > [ 714.465215] sp : ffff80001006b340
> > [ 714.465219] x29: ffff80001006b340 x28: 0000002086538003
> > [ 714.465227] x27: 0000000000000a20 x26: 0000000000001000
> > [ 714.465236] x25: 0000000000000f44 x24: 00000020adf8d000
> > [ 714.465245] x23: 0000000000000001 x22: 0000fffffaeca000
> > [ 714.465253] x21: 0000000000000003 x20: ffff19b60d64d200
> > [ 714.465261] x19: 00000000000000ca x18: 0000000000000000
> > [ 714.465270] x17: 0000000000000000 x16: ffffcccb7cf3ca20
> > [ 714.465278] x15: 0000000000000000 x14: 0000000000000000
> > [ 714.465286] x13: 0000000000000003 x12: 0000000000000010
> > [ 714.465294] x11: 0000000000000000 x10: 0000000000000002
> > [ 714.465302] x9 : ffffcccb7d5b6e78 x8 : 00000000000001ff
> > [ 714.465311] x7 : ffff19b606538650 x6 : ffff19b606538000
> > [ 714.465319] x5 : 0000000000000009 x4 : 0000000000000f44
> > [ 714.465327] x3 : 0000000000001000 x2 : 00000020adf8d000
> > [ 714.465335] x1 : 0000000000000002 x0 : 0000000000000003
> > [ 714.465343] Call trace:
> > [ 714.465348] __arm_lpae_map+0x2d4/0x30c
> > [ 714.465353] __arm_lpae_map+0x114/0x30c
> > [ 714.465357] __arm_lpae_map+0x114/0x30c
> > [ 714.465362] __arm_lpae_map+0x114/0x30c
> > [ 714.465366] arm_lpae_map+0xf4/0x180
> > [ 714.465373] arm_smmu_map+0x4c/0xc0
> > [ 714.465379] __iommu_map+0x100/0x2bc
> > [ 714.465385] iommu_map_atomic+0x20/0x30
> > [ 714.465391] __iommu_dma_map+0xb0/0x110
> > [ 714.465397] iommu_dma_map_page+0xb8/0x120
> > [ 714.465404] dma_map_page_attrs+0x1a8/0x210
> > [ 714.465413] __dpaa2_eth_tx+0x384/0xbd0 [fsl_dpaa2_eth]
> > [ 714.465421] dpaa2_eth_tx+0x84/0x134 [fsl_dpaa2_eth]
> > [ 714.465427] dev_hard_start_xmit+0x10c/0x2b0
> > [ 714.465433] sch_direct_xmit+0x1a0/0x550
> > [ 714.465438] __qdisc_run+0x140/0x670
> > [ 714.465443] __dev_queue_xmit+0x6c4/0xa74
> > [ 714.465449] dev_queue_xmit+0x20/0x2c
> > [ 714.465463] br_dev_queue_push_xmit+0xc4/0x1a0 [bridge]
> > [ 714.465476] br_forward_finish+0xdc/0xf0 [bridge]
> > [ 714.465489] __br_forward+0x160/0x1c0 [bridge]
> > [ 714.465502] br_forward+0x13c/0x160 [bridge]
> > [ 714.465514] br_dev_xmit+0x228/0x3b0 [bridge]
> > [ 714.465520] dev_hard_start_xmit+0x10c/0x2b0
> > [ 714.465526] __dev_queue_xmit+0x8f0/0xa74
> > [ 714.465531] dev_queue_xmit+0x20/0x2c
> > [ 714.465538] arp_xmit+0xc0/0xd0
> > [ 714.465544] arp_send_dst+0x78/0xa0
> > [ 714.465550] arp_solicit+0xf4/0x260
> > [ 714.465554] neigh_probe+0x64/0xb0
> > [ 714.465560] neigh_timer_handler+0x2f4/0x400
> > [ 714.465566] call_timer_fn+0x3c/0x184
> > [ 714.465572] __run_timers.part.0+0x2bc/0x370
> > [ 714.465578] run_timer_softirq+0x48/0x80
> > [ 714.465583] __do_softirq+0x120/0x36c
> > [ 714.465589] irq_exit+0xac/0x100
> > [ 714.465596] __handle_domain_irq+0x8c/0xf0
> > [ 714.465600] gic_handle_irq+0xcc/0x14c
> > [ 714.465605] el1_irq+0xc4/0x180
> > [ 714.465610] arch_cpu_idle+0x18/0x30
> > [ 714.465617] default_idle_call+0x4c/0x180
> > [ 714.465623] do_idle+0x238/0x2b0
> > [ 714.465629] cpu_startup_entry+0x30/0xa0
> > [ 714.465636] secondary_start_kernel+0x134/0x180
> > [ 714.465640] ---[ end trace a84a7f61b559005f ]---
> >
> >
> > Given it is the iommu code that is provoking the warning I should
> > probably mention that the board I have requires
> > arm-smmu.disable_bypass=0 on the kernel command line in order to boot.
> > Also if it matters I am running the latest firmware from Solidrun
> > which is based on LSDK-20.04.
> >
>
> Hmmm, from what I remember I think I tested this with the smmu bypassed
> so that is why I didn't catch it.
>
> > Is there any reason for this code not to be working for LX2160A?
>
> I wouldn't expect this to be LX2160A specific but rather a bug in the
> implementation.. sorry.
>
> Let me reproduce it and see if I can get to the bottom of it and I will
> get back with some more info.
>
Hi Daniel,
It seems that the dma-unmapping on the SGT buffer was incorrectly done
with a zero size since on the Tx path I initialized the improper field.
Could you test the following diff and let me know if you can generate
the WARNINGs anymore?
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -878,7 +878,7 @@ static int dpaa2_eth_build_sg_fd_single_buf(struct dpaa2_eth_priv *priv,
swa = (struct dpaa2_eth_swa *)sgt_buf;
swa->type = DPAA2_ETH_SWA_SINGLE;
swa->single.skb = skb;
- swa->sg.sgt_size = sgt_buf_size;
+ swa->single.sgt_size = sgt_buf_size;
/* Separately map the SGT buffer */
sgt_addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_BIDIRECTIONAL);
Ioana
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing
2020-12-11 14:01 ` Ioana Ciornei
@ 2020-12-11 16:29 ` Daniel Thompson
2020-12-11 16:54 ` Ioana Ciornei
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Thompson @ 2020-12-11 16:29 UTC (permalink / raw)
To: Ioana Ciornei; +Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
On Fri, Dec 11, 2020 at 02:01:28PM +0000, Ioana Ciornei wrote:
> On Thu, Dec 10, 2020 at 08:06:36PM +0200, Ioana Ciornei wrote:
> > [Added also the netdev mailing list, I haven't heard of linux-netdev
> > before but kept it]
> >
> > On Thu, Dec 10, 2020 at 05:31:56PM +0000, Daniel Thompson wrote:
> > > Hi Ioana
> >
> > Hi Daniel,
> >
> > >
> > > On Mon, Jun 29, 2020 at 06:47:11PM +0000, Ioana Ciornei wrote:
> > > > Instead of realloc-ing the skb on the Tx path when the provided headroom
> > > > is smaller than the HW requirements, create a Scatter/Gather frame
> > > > descriptor with only one entry.
> > > >
> > > > Remove the '[drv] tx realloc frames' counter exposed previously through
> > > > ethtool since it is no longer used.
> > > >
> > > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > > > ---
> > >
> > > I've been chasing down a networking regression on my LX2160A board
> > > (Honeycomb LX2K based on CEx7 LX2160A COM) that first appeared in v5.9.
> > >
> > > It makes the board unreliable opening outbound connections meaning
> > > things like `apt update` or `git fetch` often can't open the connection.
> > > It does not happen all the time but is sufficient to make the boards
> > > built-in networking useless for workstation use.
> > >
> > > The problem is strongly linked to warnings in the logs so I used the
> > > warnings to bisect down to locate the cause of the regression and it
> > > pinpointed this patch. I have confirmed that in both v5.9 and v5.10-rc7
> > > that reverting this patch (and fixing up the merge issues) fixes the
> > > regression and the warnings stop appearing.
> > >
> > > A typical example of the warning is below (io-pgtable-arm.c:281 is an
> > > error path that I guess would cause dma_map_page_attrs() to return
> > > an error):
> > >
> > > [ 714.464927] WARNING: CPU: 13 PID: 0 at
> > > drivers/iommu/io-pgtable-arm.c:281 __arm_lpae_map+0x2d4/0x30c
> > > [ 714.464930] Modules linked in: snd_seq_dummy(E) snd_hrtimer(E)
> > > snd_seq(E) snd_seq_device(E) snd_timer(E) snd(E) soundcore(E) bridge(E)
> > > stp(E) llc(E) rfkill(E) caam_jr(E) crypto_engine(E) rng_core(E)
> > > joydev(E) evdev(E) dpaa2_caam(E) caamhash_desc(E) caamalg_desc(E)
> > > authenc(E) libdes(E) dpaa2_console(E) ofpart(E) caam(E) sg(E) error(E)
> > > lm90(E) at24(E) spi_nor(E) mtd(E) sbsa_gwdt(E) qoriq_thermal(E)
> > > layerscape_edac_mod(E) qoriq_cpufreq(E) drm(E) fuse(E) configfs(E)
> > > ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E)
> > > mbcache(E) jbd2(E) hid_generic(E) usbhid(E) hid(E) dm_crypt(E) dm_mod(E)
> > > sd_mod(E) fsl_dpaa2_ptp(E) ptp_qoriq(E) fsl_dpaa2_eth(E)
> > > xhci_plat_hcd(E) xhci_hcd(E) usbcore(E) aes_ce_blk(E) crypto_simd(E)
> > > cryptd(E) aes_ce_cipher(E) ghash_ce(E) gf128mul(E) at803x(E) libaes(E)
> > > fsl_mc_dpio(E) pcs_lynx(E) rtc_pcf2127(E) sha2_ce(E) phylink(E)
> > > xgmac_mdio(E) regmap_spi(E) of_mdio(E) sha256_arm64(E)
> > > i2c_mux_pca954x(E) fixed_phy(E) i2c_mux(E) sha1_ce(E) ptp(E) libphy(E)
> > > [ 714.465131] pps_core(E) ahci_qoriq(E) libahci_platform(E) nvme(E)
> > > libahci(E) nvme_core(E) t10_pi(E) libata(E) crc_t10dif(E)
> > > crct10dif_generic(E) crct10dif_common(E) dwc3(E) scsi_mod(E) udc_core(E)
> > > roles(E) ulpi(E) sdhci_of_esdhc(E) sdhci_pltfm(E) sdhci(E)
> > > spi_nxp_fspi(E) i2c_imx(E) fixed(E)
> > > [ 714.465192] CPU: 13 PID: 0 Comm: swapper/13 Tainted: G W E
> > > 5.10.0-rc7-00001-gba98d13279ca #52
> > > [ 714.465196] Hardware name: SolidRun LX2160A Honeycomb (DT)
> > > [ 714.465202] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> > > [ 714.465207] pc : __arm_lpae_map+0x2d4/0x30c
> > > [ 714.465211] lr : __arm_lpae_map+0x114/0x30c
> > > [ 714.465215] sp : ffff80001006b340
> > > [ 714.465219] x29: ffff80001006b340 x28: 0000002086538003
> > > [ 714.465227] x27: 0000000000000a20 x26: 0000000000001000
> > > [ 714.465236] x25: 0000000000000f44 x24: 00000020adf8d000
> > > [ 714.465245] x23: 0000000000000001 x22: 0000fffffaeca000
> > > [ 714.465253] x21: 0000000000000003 x20: ffff19b60d64d200
> > > [ 714.465261] x19: 00000000000000ca x18: 0000000000000000
> > > [ 714.465270] x17: 0000000000000000 x16: ffffcccb7cf3ca20
> > > [ 714.465278] x15: 0000000000000000 x14: 0000000000000000
> > > [ 714.465286] x13: 0000000000000003 x12: 0000000000000010
> > > [ 714.465294] x11: 0000000000000000 x10: 0000000000000002
> > > [ 714.465302] x9 : ffffcccb7d5b6e78 x8 : 00000000000001ff
> > > [ 714.465311] x7 : ffff19b606538650 x6 : ffff19b606538000
> > > [ 714.465319] x5 : 0000000000000009 x4 : 0000000000000f44
> > > [ 714.465327] x3 : 0000000000001000 x2 : 00000020adf8d000
> > > [ 714.465335] x1 : 0000000000000002 x0 : 0000000000000003
> > > [ 714.465343] Call trace:
> > > [ 714.465348] __arm_lpae_map+0x2d4/0x30c
> > > [ 714.465353] __arm_lpae_map+0x114/0x30c
> > > [ 714.465357] __arm_lpae_map+0x114/0x30c
> > > [ 714.465362] __arm_lpae_map+0x114/0x30c
> > > [ 714.465366] arm_lpae_map+0xf4/0x180
> > > [ 714.465373] arm_smmu_map+0x4c/0xc0
> > > [ 714.465379] __iommu_map+0x100/0x2bc
> > > [ 714.465385] iommu_map_atomic+0x20/0x30
> > > [ 714.465391] __iommu_dma_map+0xb0/0x110
> > > [ 714.465397] iommu_dma_map_page+0xb8/0x120
> > > [ 714.465404] dma_map_page_attrs+0x1a8/0x210
> > > [ 714.465413] __dpaa2_eth_tx+0x384/0xbd0 [fsl_dpaa2_eth]
> > > [ 714.465421] dpaa2_eth_tx+0x84/0x134 [fsl_dpaa2_eth]
> > > [ 714.465427] dev_hard_start_xmit+0x10c/0x2b0
> > > [ 714.465433] sch_direct_xmit+0x1a0/0x550
> > > [ 714.465438] __qdisc_run+0x140/0x670
> > > [ 714.465443] __dev_queue_xmit+0x6c4/0xa74
> > > [ 714.465449] dev_queue_xmit+0x20/0x2c
> > > [ 714.465463] br_dev_queue_push_xmit+0xc4/0x1a0 [bridge]
> > > [ 714.465476] br_forward_finish+0xdc/0xf0 [bridge]
> > > [ 714.465489] __br_forward+0x160/0x1c0 [bridge]
> > > [ 714.465502] br_forward+0x13c/0x160 [bridge]
> > > [ 714.465514] br_dev_xmit+0x228/0x3b0 [bridge]
> > > [ 714.465520] dev_hard_start_xmit+0x10c/0x2b0
> > > [ 714.465526] __dev_queue_xmit+0x8f0/0xa74
> > > [ 714.465531] dev_queue_xmit+0x20/0x2c
> > > [ 714.465538] arp_xmit+0xc0/0xd0
> > > [ 714.465544] arp_send_dst+0x78/0xa0
> > > [ 714.465550] arp_solicit+0xf4/0x260
> > > [ 714.465554] neigh_probe+0x64/0xb0
> > > [ 714.465560] neigh_timer_handler+0x2f4/0x400
> > > [ 714.465566] call_timer_fn+0x3c/0x184
> > > [ 714.465572] __run_timers.part.0+0x2bc/0x370
> > > [ 714.465578] run_timer_softirq+0x48/0x80
> > > [ 714.465583] __do_softirq+0x120/0x36c
> > > [ 714.465589] irq_exit+0xac/0x100
> > > [ 714.465596] __handle_domain_irq+0x8c/0xf0
> > > [ 714.465600] gic_handle_irq+0xcc/0x14c
> > > [ 714.465605] el1_irq+0xc4/0x180
> > > [ 714.465610] arch_cpu_idle+0x18/0x30
> > > [ 714.465617] default_idle_call+0x4c/0x180
> > > [ 714.465623] do_idle+0x238/0x2b0
> > > [ 714.465629] cpu_startup_entry+0x30/0xa0
> > > [ 714.465636] secondary_start_kernel+0x134/0x180
> > > [ 714.465640] ---[ end trace a84a7f61b559005f ]---
> > >
> > >
> > > Given it is the iommu code that is provoking the warning I should
> > > probably mention that the board I have requires
> > > arm-smmu.disable_bypass=0 on the kernel command line in order to boot.
> > > Also if it matters I am running the latest firmware from Solidrun
> > > which is based on LSDK-20.04.
> > >
> >
> > Hmmm, from what I remember I think I tested this with the smmu bypassed
> > so that is why I didn't catch it.
> >
> > > Is there any reason for this code not to be working for LX2160A?
> >
> > I wouldn't expect this to be LX2160A specific but rather a bug in the
> > implementation.. sorry.
> >
> > Let me reproduce it and see if I can get to the bottom of it and I will
> > get back with some more info.
> >
>
> Hi Daniel,
>
> It seems that the dma-unmapping on the SGT buffer was incorrectly done
> with a zero size since on the Tx path I initialized the improper field.
>
> Could you test the following diff and let me know if you can generate
> the WARNINGs anymore?
I fired this up and, with your change, I've not been able to trigger
the warning with the tests that I used the drive my bisect.
Thanks for the quick response.
Daniel.
>
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> @@ -878,7 +878,7 @@ static int dpaa2_eth_build_sg_fd_single_buf(struct dpaa2_eth_priv *priv,
> swa = (struct dpaa2_eth_swa *)sgt_buf;
> swa->type = DPAA2_ETH_SWA_SINGLE;
> swa->single.skb = skb;
> - swa->sg.sgt_size = sgt_buf_size;
> + swa->single.sgt_size = sgt_buf_size;
>
> /* Separately map the SGT buffer */
> sgt_addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_BIDIRECTIONAL);
>
>
> Ioana
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing
2020-12-11 16:29 ` Daniel Thompson
@ 2020-12-11 16:54 ` Ioana Ciornei
2020-12-12 15:58 ` Jon Nettleton
0 siblings, 1 reply; 8+ messages in thread
From: Ioana Ciornei @ 2020-12-11 16:54 UTC (permalink / raw)
To: Daniel Thompson; +Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
On Fri, Dec 11, 2020 at 04:29:14PM +0000, Daniel Thompson wrote:
> On Fri, Dec 11, 2020 at 02:01:28PM +0000, Ioana Ciornei wrote:
> > On Thu, Dec 10, 2020 at 08:06:36PM +0200, Ioana Ciornei wrote:
> > > [Added also the netdev mailing list, I haven't heard of linux-netdev
> > > before but kept it]
> > >
> > > On Thu, Dec 10, 2020 at 05:31:56PM +0000, Daniel Thompson wrote:
> > > > Hi Ioana
> > >
> > > Hi Daniel,
> > >
> > > >
> > > > On Mon, Jun 29, 2020 at 06:47:11PM +0000, Ioana Ciornei wrote:
> > > > > Instead of realloc-ing the skb on the Tx path when the provided headroom
> > > > > is smaller than the HW requirements, create a Scatter/Gather frame
> > > > > descriptor with only one entry.
> > > > >
> > > > > Remove the '[drv] tx realloc frames' counter exposed previously through
> > > > > ethtool since it is no longer used.
> > > > >
> > > > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > > > > ---
> > > >
> > > > I've been chasing down a networking regression on my LX2160A board
> > > > (Honeycomb LX2K based on CEx7 LX2160A COM) that first appeared in v5.9.
> > > >
> > > > It makes the board unreliable opening outbound connections meaning
> > > > things like `apt update` or `git fetch` often can't open the connection.
> > > > It does not happen all the time but is sufficient to make the boards
> > > > built-in networking useless for workstation use.
> > > >
> > > > The problem is strongly linked to warnings in the logs so I used the
> > > > warnings to bisect down to locate the cause of the regression and it
> > > > pinpointed this patch. I have confirmed that in both v5.9 and v5.10-rc7
> > > > that reverting this patch (and fixing up the merge issues) fixes the
> > > > regression and the warnings stop appearing.
> > > >
> > > > A typical example of the warning is below (io-pgtable-arm.c:281 is an
> > > > error path that I guess would cause dma_map_page_attrs() to return
> > > > an error):
> > > >
> > > > [ 714.464927] WARNING: CPU: 13 PID: 0 at
> > > > drivers/iommu/io-pgtable-arm.c:281 __arm_lpae_map+0x2d4/0x30c
> > > > [ 714.464930] Modules linked in: snd_seq_dummy(E) snd_hrtimer(E)
> > > > snd_seq(E) snd_seq_device(E) snd_timer(E) snd(E) soundcore(E) bridge(E)
> > > > stp(E) llc(E) rfkill(E) caam_jr(E) crypto_engine(E) rng_core(E)
> > > > joydev(E) evdev(E) dpaa2_caam(E) caamhash_desc(E) caamalg_desc(E)
> > > > authenc(E) libdes(E) dpaa2_console(E) ofpart(E) caam(E) sg(E) error(E)
> > > > lm90(E) at24(E) spi_nor(E) mtd(E) sbsa_gwdt(E) qoriq_thermal(E)
> > > > layerscape_edac_mod(E) qoriq_cpufreq(E) drm(E) fuse(E) configfs(E)
> > > > ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E)
> > > > mbcache(E) jbd2(E) hid_generic(E) usbhid(E) hid(E) dm_crypt(E) dm_mod(E)
> > > > sd_mod(E) fsl_dpaa2_ptp(E) ptp_qoriq(E) fsl_dpaa2_eth(E)
> > > > xhci_plat_hcd(E) xhci_hcd(E) usbcore(E) aes_ce_blk(E) crypto_simd(E)
> > > > cryptd(E) aes_ce_cipher(E) ghash_ce(E) gf128mul(E) at803x(E) libaes(E)
> > > > fsl_mc_dpio(E) pcs_lynx(E) rtc_pcf2127(E) sha2_ce(E) phylink(E)
> > > > xgmac_mdio(E) regmap_spi(E) of_mdio(E) sha256_arm64(E)
> > > > i2c_mux_pca954x(E) fixed_phy(E) i2c_mux(E) sha1_ce(E) ptp(E) libphy(E)
> > > > [ 714.465131] pps_core(E) ahci_qoriq(E) libahci_platform(E) nvme(E)
> > > > libahci(E) nvme_core(E) t10_pi(E) libata(E) crc_t10dif(E)
> > > > crct10dif_generic(E) crct10dif_common(E) dwc3(E) scsi_mod(E) udc_core(E)
> > > > roles(E) ulpi(E) sdhci_of_esdhc(E) sdhci_pltfm(E) sdhci(E)
> > > > spi_nxp_fspi(E) i2c_imx(E) fixed(E)
> > > > [ 714.465192] CPU: 13 PID: 0 Comm: swapper/13 Tainted: G W E
> > > > 5.10.0-rc7-00001-gba98d13279ca #52
> > > > [ 714.465196] Hardware name: SolidRun LX2160A Honeycomb (DT)
> > > > [ 714.465202] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> > > > [ 714.465207] pc : __arm_lpae_map+0x2d4/0x30c
> > > > [ 714.465211] lr : __arm_lpae_map+0x114/0x30c
> > > > [ 714.465215] sp : ffff80001006b340
> > > > [ 714.465219] x29: ffff80001006b340 x28: 0000002086538003
> > > > [ 714.465227] x27: 0000000000000a20 x26: 0000000000001000
> > > > [ 714.465236] x25: 0000000000000f44 x24: 00000020adf8d000
> > > > [ 714.465245] x23: 0000000000000001 x22: 0000fffffaeca000
> > > > [ 714.465253] x21: 0000000000000003 x20: ffff19b60d64d200
> > > > [ 714.465261] x19: 00000000000000ca x18: 0000000000000000
> > > > [ 714.465270] x17: 0000000000000000 x16: ffffcccb7cf3ca20
> > > > [ 714.465278] x15: 0000000000000000 x14: 0000000000000000
> > > > [ 714.465286] x13: 0000000000000003 x12: 0000000000000010
> > > > [ 714.465294] x11: 0000000000000000 x10: 0000000000000002
> > > > [ 714.465302] x9 : ffffcccb7d5b6e78 x8 : 00000000000001ff
> > > > [ 714.465311] x7 : ffff19b606538650 x6 : ffff19b606538000
> > > > [ 714.465319] x5 : 0000000000000009 x4 : 0000000000000f44
> > > > [ 714.465327] x3 : 0000000000001000 x2 : 00000020adf8d000
> > > > [ 714.465335] x1 : 0000000000000002 x0 : 0000000000000003
> > > > [ 714.465343] Call trace:
> > > > [ 714.465348] __arm_lpae_map+0x2d4/0x30c
> > > > [ 714.465353] __arm_lpae_map+0x114/0x30c
> > > > [ 714.465357] __arm_lpae_map+0x114/0x30c
> > > > [ 714.465362] __arm_lpae_map+0x114/0x30c
> > > > [ 714.465366] arm_lpae_map+0xf4/0x180
> > > > [ 714.465373] arm_smmu_map+0x4c/0xc0
> > > > [ 714.465379] __iommu_map+0x100/0x2bc
> > > > [ 714.465385] iommu_map_atomic+0x20/0x30
> > > > [ 714.465391] __iommu_dma_map+0xb0/0x110
> > > > [ 714.465397] iommu_dma_map_page+0xb8/0x120
> > > > [ 714.465404] dma_map_page_attrs+0x1a8/0x210
> > > > [ 714.465413] __dpaa2_eth_tx+0x384/0xbd0 [fsl_dpaa2_eth]
> > > > [ 714.465421] dpaa2_eth_tx+0x84/0x134 [fsl_dpaa2_eth]
> > > > [ 714.465427] dev_hard_start_xmit+0x10c/0x2b0
> > > > [ 714.465433] sch_direct_xmit+0x1a0/0x550
> > > > [ 714.465438] __qdisc_run+0x140/0x670
> > > > [ 714.465443] __dev_queue_xmit+0x6c4/0xa74
> > > > [ 714.465449] dev_queue_xmit+0x20/0x2c
> > > > [ 714.465463] br_dev_queue_push_xmit+0xc4/0x1a0 [bridge]
> > > > [ 714.465476] br_forward_finish+0xdc/0xf0 [bridge]
> > > > [ 714.465489] __br_forward+0x160/0x1c0 [bridge]
> > > > [ 714.465502] br_forward+0x13c/0x160 [bridge]
> > > > [ 714.465514] br_dev_xmit+0x228/0x3b0 [bridge]
> > > > [ 714.465520] dev_hard_start_xmit+0x10c/0x2b0
> > > > [ 714.465526] __dev_queue_xmit+0x8f0/0xa74
> > > > [ 714.465531] dev_queue_xmit+0x20/0x2c
> > > > [ 714.465538] arp_xmit+0xc0/0xd0
> > > > [ 714.465544] arp_send_dst+0x78/0xa0
> > > > [ 714.465550] arp_solicit+0xf4/0x260
> > > > [ 714.465554] neigh_probe+0x64/0xb0
> > > > [ 714.465560] neigh_timer_handler+0x2f4/0x400
> > > > [ 714.465566] call_timer_fn+0x3c/0x184
> > > > [ 714.465572] __run_timers.part.0+0x2bc/0x370
> > > > [ 714.465578] run_timer_softirq+0x48/0x80
> > > > [ 714.465583] __do_softirq+0x120/0x36c
> > > > [ 714.465589] irq_exit+0xac/0x100
> > > > [ 714.465596] __handle_domain_irq+0x8c/0xf0
> > > > [ 714.465600] gic_handle_irq+0xcc/0x14c
> > > > [ 714.465605] el1_irq+0xc4/0x180
> > > > [ 714.465610] arch_cpu_idle+0x18/0x30
> > > > [ 714.465617] default_idle_call+0x4c/0x180
> > > > [ 714.465623] do_idle+0x238/0x2b0
> > > > [ 714.465629] cpu_startup_entry+0x30/0xa0
> > > > [ 714.465636] secondary_start_kernel+0x134/0x180
> > > > [ 714.465640] ---[ end trace a84a7f61b559005f ]---
> > > >
> > > >
> > > > Given it is the iommu code that is provoking the warning I should
> > > > probably mention that the board I have requires
> > > > arm-smmu.disable_bypass=0 on the kernel command line in order to boot.
> > > > Also if it matters I am running the latest firmware from Solidrun
> > > > which is based on LSDK-20.04.
> > > >
> > >
> > > Hmmm, from what I remember I think I tested this with the smmu bypassed
> > > so that is why I didn't catch it.
> > >
> > > > Is there any reason for this code not to be working for LX2160A?
> > >
> > > I wouldn't expect this to be LX2160A specific but rather a bug in the
> > > implementation.. sorry.
> > >
> > > Let me reproduce it and see if I can get to the bottom of it and I will
> > > get back with some more info.
> > >
> >
> > Hi Daniel,
> >
> > It seems that the dma-unmapping on the SGT buffer was incorrectly done
> > with a zero size since on the Tx path I initialized the improper field.
> >
> > Could you test the following diff and let me know if you can generate
> > the WARNINGs anymore?
>
> I fired this up and, with your change, I've not been able to trigger
> the warning with the tests that I used the drive my bisect.
>
Great, thanks for testing this.
I will take care of sending the fix to -net.
Ioana
> Thanks for the quick response.
>
>
> Daniel.
>
>
> >
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > @@ -878,7 +878,7 @@ static int dpaa2_eth_build_sg_fd_single_buf(struct dpaa2_eth_priv *priv,
> > swa = (struct dpaa2_eth_swa *)sgt_buf;
> > swa->type = DPAA2_ETH_SWA_SINGLE;
> > swa->single.skb = skb;
> > - swa->sg.sgt_size = sgt_buf_size;
> > + swa->single.sgt_size = sgt_buf_size;
> >
> > /* Separately map the SGT buffer */
> > sgt_addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_BIDIRECTIONAL);
> >
> >
> > Ioana
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing
2020-12-11 16:54 ` Ioana Ciornei
@ 2020-12-12 15:58 ` Jon Nettleton
2020-12-12 19:31 ` Ioana Ciornei
0 siblings, 1 reply; 8+ messages in thread
From: Jon Nettleton @ 2020-12-12 15:58 UTC (permalink / raw)
To: Ioana Ciornei
Cc: Daniel Thompson, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
On Fri, Dec 11, 2020 at 5:56 PM Ioana Ciornei <ioana.ciornei@nxp.com> wrote:
>
> On Fri, Dec 11, 2020 at 04:29:14PM +0000, Daniel Thompson wrote:
> > On Fri, Dec 11, 2020 at 02:01:28PM +0000, Ioana Ciornei wrote:
> > > On Thu, Dec 10, 2020 at 08:06:36PM +0200, Ioana Ciornei wrote:
> > > > [Added also the netdev mailing list, I haven't heard of linux-netdev
> > > > before but kept it]
> > > >
> > > > On Thu, Dec 10, 2020 at 05:31:56PM +0000, Daniel Thompson wrote:
> > > > > Hi Ioana
> > > >
> > > > Hi Daniel,
> > > >
> > > > >
> > > > > On Mon, Jun 29, 2020 at 06:47:11PM +0000, Ioana Ciornei wrote:
> > > > > > Instead of realloc-ing the skb on the Tx path when the provided headroom
> > > > > > is smaller than the HW requirements, create a Scatter/Gather frame
> > > > > > descriptor with only one entry.
> > > > > >
> > > > > > Remove the '[drv] tx realloc frames' counter exposed previously through
> > > > > > ethtool since it is no longer used.
> > > > > >
> > > > > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > > > > > ---
> > > > >
> > > > > I've been chasing down a networking regression on my LX2160A board
> > > > > (Honeycomb LX2K based on CEx7 LX2160A COM) that first appeared in v5.9.
> > > > >
> > > > > It makes the board unreliable opening outbound connections meaning
> > > > > things like `apt update` or `git fetch` often can't open the connection.
> > > > > It does not happen all the time but is sufficient to make the boards
> > > > > built-in networking useless for workstation use.
> > > > >
> > > > > The problem is strongly linked to warnings in the logs so I used the
> > > > > warnings to bisect down to locate the cause of the regression and it
> > > > > pinpointed this patch. I have confirmed that in both v5.9 and v5.10-rc7
> > > > > that reverting this patch (and fixing up the merge issues) fixes the
> > > > > regression and the warnings stop appearing.
> > > > >
> > > > > A typical example of the warning is below (io-pgtable-arm.c:281 is an
> > > > > error path that I guess would cause dma_map_page_attrs() to return
> > > > > an error):
> > > > >
> > > > > [ 714.464927] WARNING: CPU: 13 PID: 0 at
> > > > > drivers/iommu/io-pgtable-arm.c:281 __arm_lpae_map+0x2d4/0x30c
> > > > > [ 714.464930] Modules linked in: snd_seq_dummy(E) snd_hrtimer(E)
> > > > > snd_seq(E) snd_seq_device(E) snd_timer(E) snd(E) soundcore(E) bridge(E)
> > > > > stp(E) llc(E) rfkill(E) caam_jr(E) crypto_engine(E) rng_core(E)
> > > > > joydev(E) evdev(E) dpaa2_caam(E) caamhash_desc(E) caamalg_desc(E)
> > > > > authenc(E) libdes(E) dpaa2_console(E) ofpart(E) caam(E) sg(E) error(E)
> > > > > lm90(E) at24(E) spi_nor(E) mtd(E) sbsa_gwdt(E) qoriq_thermal(E)
> > > > > layerscape_edac_mod(E) qoriq_cpufreq(E) drm(E) fuse(E) configfs(E)
> > > > > ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E)
> > > > > mbcache(E) jbd2(E) hid_generic(E) usbhid(E) hid(E) dm_crypt(E) dm_mod(E)
> > > > > sd_mod(E) fsl_dpaa2_ptp(E) ptp_qoriq(E) fsl_dpaa2_eth(E)
> > > > > xhci_plat_hcd(E) xhci_hcd(E) usbcore(E) aes_ce_blk(E) crypto_simd(E)
> > > > > cryptd(E) aes_ce_cipher(E) ghash_ce(E) gf128mul(E) at803x(E) libaes(E)
> > > > > fsl_mc_dpio(E) pcs_lynx(E) rtc_pcf2127(E) sha2_ce(E) phylink(E)
> > > > > xgmac_mdio(E) regmap_spi(E) of_mdio(E) sha256_arm64(E)
> > > > > i2c_mux_pca954x(E) fixed_phy(E) i2c_mux(E) sha1_ce(E) ptp(E) libphy(E)
> > > > > [ 714.465131] pps_core(E) ahci_qoriq(E) libahci_platform(E) nvme(E)
> > > > > libahci(E) nvme_core(E) t10_pi(E) libata(E) crc_t10dif(E)
> > > > > crct10dif_generic(E) crct10dif_common(E) dwc3(E) scsi_mod(E) udc_core(E)
> > > > > roles(E) ulpi(E) sdhci_of_esdhc(E) sdhci_pltfm(E) sdhci(E)
> > > > > spi_nxp_fspi(E) i2c_imx(E) fixed(E)
> > > > > [ 714.465192] CPU: 13 PID: 0 Comm: swapper/13 Tainted: G W E
> > > > > 5.10.0-rc7-00001-gba98d13279ca #52
> > > > > [ 714.465196] Hardware name: SolidRun LX2160A Honeycomb (DT)
> > > > > [ 714.465202] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> > > > > [ 714.465207] pc : __arm_lpae_map+0x2d4/0x30c
> > > > > [ 714.465211] lr : __arm_lpae_map+0x114/0x30c
> > > > > [ 714.465215] sp : ffff80001006b340
> > > > > [ 714.465219] x29: ffff80001006b340 x28: 0000002086538003
> > > > > [ 714.465227] x27: 0000000000000a20 x26: 0000000000001000
> > > > > [ 714.465236] x25: 0000000000000f44 x24: 00000020adf8d000
> > > > > [ 714.465245] x23: 0000000000000001 x22: 0000fffffaeca000
> > > > > [ 714.465253] x21: 0000000000000003 x20: ffff19b60d64d200
> > > > > [ 714.465261] x19: 00000000000000ca x18: 0000000000000000
> > > > > [ 714.465270] x17: 0000000000000000 x16: ffffcccb7cf3ca20
> > > > > [ 714.465278] x15: 0000000000000000 x14: 0000000000000000
> > > > > [ 714.465286] x13: 0000000000000003 x12: 0000000000000010
> > > > > [ 714.465294] x11: 0000000000000000 x10: 0000000000000002
> > > > > [ 714.465302] x9 : ffffcccb7d5b6e78 x8 : 00000000000001ff
> > > > > [ 714.465311] x7 : ffff19b606538650 x6 : ffff19b606538000
> > > > > [ 714.465319] x5 : 0000000000000009 x4 : 0000000000000f44
> > > > > [ 714.465327] x3 : 0000000000001000 x2 : 00000020adf8d000
> > > > > [ 714.465335] x1 : 0000000000000002 x0 : 0000000000000003
> > > > > [ 714.465343] Call trace:
> > > > > [ 714.465348] __arm_lpae_map+0x2d4/0x30c
> > > > > [ 714.465353] __arm_lpae_map+0x114/0x30c
> > > > > [ 714.465357] __arm_lpae_map+0x114/0x30c
> > > > > [ 714.465362] __arm_lpae_map+0x114/0x30c
> > > > > [ 714.465366] arm_lpae_map+0xf4/0x180
> > > > > [ 714.465373] arm_smmu_map+0x4c/0xc0
> > > > > [ 714.465379] __iommu_map+0x100/0x2bc
> > > > > [ 714.465385] iommu_map_atomic+0x20/0x30
> > > > > [ 714.465391] __iommu_dma_map+0xb0/0x110
> > > > > [ 714.465397] iommu_dma_map_page+0xb8/0x120
> > > > > [ 714.465404] dma_map_page_attrs+0x1a8/0x210
> > > > > [ 714.465413] __dpaa2_eth_tx+0x384/0xbd0 [fsl_dpaa2_eth]
> > > > > [ 714.465421] dpaa2_eth_tx+0x84/0x134 [fsl_dpaa2_eth]
> > > > > [ 714.465427] dev_hard_start_xmit+0x10c/0x2b0
> > > > > [ 714.465433] sch_direct_xmit+0x1a0/0x550
> > > > > [ 714.465438] __qdisc_run+0x140/0x670
> > > > > [ 714.465443] __dev_queue_xmit+0x6c4/0xa74
> > > > > [ 714.465449] dev_queue_xmit+0x20/0x2c
> > > > > [ 714.465463] br_dev_queue_push_xmit+0xc4/0x1a0 [bridge]
> > > > > [ 714.465476] br_forward_finish+0xdc/0xf0 [bridge]
> > > > > [ 714.465489] __br_forward+0x160/0x1c0 [bridge]
> > > > > [ 714.465502] br_forward+0x13c/0x160 [bridge]
> > > > > [ 714.465514] br_dev_xmit+0x228/0x3b0 [bridge]
> > > > > [ 714.465520] dev_hard_start_xmit+0x10c/0x2b0
> > > > > [ 714.465526] __dev_queue_xmit+0x8f0/0xa74
> > > > > [ 714.465531] dev_queue_xmit+0x20/0x2c
> > > > > [ 714.465538] arp_xmit+0xc0/0xd0
> > > > > [ 714.465544] arp_send_dst+0x78/0xa0
> > > > > [ 714.465550] arp_solicit+0xf4/0x260
> > > > > [ 714.465554] neigh_probe+0x64/0xb0
> > > > > [ 714.465560] neigh_timer_handler+0x2f4/0x400
> > > > > [ 714.465566] call_timer_fn+0x3c/0x184
> > > > > [ 714.465572] __run_timers.part.0+0x2bc/0x370
> > > > > [ 714.465578] run_timer_softirq+0x48/0x80
> > > > > [ 714.465583] __do_softirq+0x120/0x36c
> > > > > [ 714.465589] irq_exit+0xac/0x100
> > > > > [ 714.465596] __handle_domain_irq+0x8c/0xf0
> > > > > [ 714.465600] gic_handle_irq+0xcc/0x14c
> > > > > [ 714.465605] el1_irq+0xc4/0x180
> > > > > [ 714.465610] arch_cpu_idle+0x18/0x30
> > > > > [ 714.465617] default_idle_call+0x4c/0x180
> > > > > [ 714.465623] do_idle+0x238/0x2b0
> > > > > [ 714.465629] cpu_startup_entry+0x30/0xa0
> > > > > [ 714.465636] secondary_start_kernel+0x134/0x180
> > > > > [ 714.465640] ---[ end trace a84a7f61b559005f ]---
> > > > >
> > > > >
> > > > > Given it is the iommu code that is provoking the warning I should
> > > > > probably mention that the board I have requires
> > > > > arm-smmu.disable_bypass=0 on the kernel command line in order to boot.
> > > > > Also if it matters I am running the latest firmware from Solidrun
> > > > > which is based on LSDK-20.04.
> > > > >
> > > >
> > > > Hmmm, from what I remember I think I tested this with the smmu bypassed
> > > > so that is why I didn't catch it.
> > > >
> > > > > Is there any reason for this code not to be working for LX2160A?
> > > >
> > > > I wouldn't expect this to be LX2160A specific but rather a bug in the
> > > > implementation.. sorry.
> > > >
> > > > Let me reproduce it and see if I can get to the bottom of it and I will
> > > > get back with some more info.
> > > >
> > >
> > > Hi Daniel,
> > >
> > > It seems that the dma-unmapping on the SGT buffer was incorrectly done
> > > with a zero size since on the Tx path I initialized the improper field.
> > >
> > > Could you test the following diff and let me know if you can generate
> > > the WARNINGs anymore?
> >
> > I fired this up and, with your change, I've not been able to trigger
> > the warning with the tests that I used the drive my bisect.
> >
>
> Great, thanks for testing this.
>
> I will take care of sending the fix to -net.
>
> Ioana
Ioana,
Please CC me when you send the patch to -net, I will put my Tested-by: on it.
Thanks
Jon
>
> > Thanks for the quick response.
> >
> >
> > Daniel.
> >
> >
> > >
> > > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > > @@ -878,7 +878,7 @@ static int dpaa2_eth_build_sg_fd_single_buf(struct dpaa2_eth_priv *priv,
> > > swa = (struct dpaa2_eth_swa *)sgt_buf;
> > > swa->type = DPAA2_ETH_SWA_SINGLE;
> > > swa->single.skb = skb;
> > > - swa->sg.sgt_size = sgt_buf_size;
> > > + swa->single.sgt_size = sgt_buf_size;
> > >
> > > /* Separately map the SGT buffer */
> > > sgt_addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_BIDIRECTIONAL);
> > >
> > >
> > > Ioana
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing
2020-12-12 15:58 ` Jon Nettleton
@ 2020-12-12 19:31 ` Ioana Ciornei
0 siblings, 0 replies; 8+ messages in thread
From: Ioana Ciornei @ 2020-12-12 19:31 UTC (permalink / raw)
To: Jon Nettleton
Cc: Daniel Thompson, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
On Sat, Dec 12, 2020 at 04:58:56PM +0100, Jon Nettleton wrote:
> On Fri, Dec 11, 2020 at 5:56 PM Ioana Ciornei <ioana.ciornei@nxp.com> wrote:
> >
> > On Fri, Dec 11, 2020 at 04:29:14PM +0000, Daniel Thompson wrote:
> > > On Fri, Dec 11, 2020 at 02:01:28PM +0000, Ioana Ciornei wrote:
> > > > On Thu, Dec 10, 2020 at 08:06:36PM +0200, Ioana Ciornei wrote:
> > > > > [Added also the netdev mailing list, I haven't heard of linux-netdev
> > > > > before but kept it]
> > > > >
> > > > > On Thu, Dec 10, 2020 at 05:31:56PM +0000, Daniel Thompson wrote:
> > > > > > Hi Ioana
> > > > >
> > > > > Hi Daniel,
> > > > >
> > > > > >
> > > > > > On Mon, Jun 29, 2020 at 06:47:11PM +0000, Ioana Ciornei wrote:
> > > > > > > Instead of realloc-ing the skb on the Tx path when the provided headroom
> > > > > > > is smaller than the HW requirements, create a Scatter/Gather frame
> > > > > > > descriptor with only one entry.
> > > > > > >
> > > > > > > Remove the '[drv] tx realloc frames' counter exposed previously through
> > > > > > > ethtool since it is no longer used.
> > > > > > >
> > > > > > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > > > > > > ---
> > > > > >
> > > > > > I've been chasing down a networking regression on my LX2160A board
> > > > > > (Honeycomb LX2K based on CEx7 LX2160A COM) that first appeared in v5.9.
> > > > > >
> > > > > > It makes the board unreliable opening outbound connections meaning
> > > > > > things like `apt update` or `git fetch` often can't open the connection.
> > > > > > It does not happen all the time but is sufficient to make the boards
> > > > > > built-in networking useless for workstation use.
> > > > > >
> > > > > > The problem is strongly linked to warnings in the logs so I used the
> > > > > > warnings to bisect down to locate the cause of the regression and it
> > > > > > pinpointed this patch. I have confirmed that in both v5.9 and v5.10-rc7
> > > > > > that reverting this patch (and fixing up the merge issues) fixes the
> > > > > > regression and the warnings stop appearing.
> > > > > >
> > > > > > A typical example of the warning is below (io-pgtable-arm.c:281 is an
> > > > > > error path that I guess would cause dma_map_page_attrs() to return
> > > > > > an error):
> > > > > >
> > > > > > [ 714.464927] WARNING: CPU: 13 PID: 0 at
> > > > > > drivers/iommu/io-pgtable-arm.c:281 __arm_lpae_map+0x2d4/0x30c
> > > > > > [ 714.464930] Modules linked in: snd_seq_dummy(E) snd_hrtimer(E)
> > > > > > snd_seq(E) snd_seq_device(E) snd_timer(E) snd(E) soundcore(E) bridge(E)
> > > > > > stp(E) llc(E) rfkill(E) caam_jr(E) crypto_engine(E) rng_core(E)
> > > > > > joydev(E) evdev(E) dpaa2_caam(E) caamhash_desc(E) caamalg_desc(E)
> > > > > > authenc(E) libdes(E) dpaa2_console(E) ofpart(E) caam(E) sg(E) error(E)
> > > > > > lm90(E) at24(E) spi_nor(E) mtd(E) sbsa_gwdt(E) qoriq_thermal(E)
> > > > > > layerscape_edac_mod(E) qoriq_cpufreq(E) drm(E) fuse(E) configfs(E)
> > > > > > ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E)
> > > > > > mbcache(E) jbd2(E) hid_generic(E) usbhid(E) hid(E) dm_crypt(E) dm_mod(E)
> > > > > > sd_mod(E) fsl_dpaa2_ptp(E) ptp_qoriq(E) fsl_dpaa2_eth(E)
> > > > > > xhci_plat_hcd(E) xhci_hcd(E) usbcore(E) aes_ce_blk(E) crypto_simd(E)
> > > > > > cryptd(E) aes_ce_cipher(E) ghash_ce(E) gf128mul(E) at803x(E) libaes(E)
> > > > > > fsl_mc_dpio(E) pcs_lynx(E) rtc_pcf2127(E) sha2_ce(E) phylink(E)
> > > > > > xgmac_mdio(E) regmap_spi(E) of_mdio(E) sha256_arm64(E)
> > > > > > i2c_mux_pca954x(E) fixed_phy(E) i2c_mux(E) sha1_ce(E) ptp(E) libphy(E)
> > > > > > [ 714.465131] pps_core(E) ahci_qoriq(E) libahci_platform(E) nvme(E)
> > > > > > libahci(E) nvme_core(E) t10_pi(E) libata(E) crc_t10dif(E)
> > > > > > crct10dif_generic(E) crct10dif_common(E) dwc3(E) scsi_mod(E) udc_core(E)
> > > > > > roles(E) ulpi(E) sdhci_of_esdhc(E) sdhci_pltfm(E) sdhci(E)
> > > > > > spi_nxp_fspi(E) i2c_imx(E) fixed(E)
> > > > > > [ 714.465192] CPU: 13 PID: 0 Comm: swapper/13 Tainted: G W E
> > > > > > 5.10.0-rc7-00001-gba98d13279ca #52
> > > > > > [ 714.465196] Hardware name: SolidRun LX2160A Honeycomb (DT)
> > > > > > [ 714.465202] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> > > > > > [ 714.465207] pc : __arm_lpae_map+0x2d4/0x30c
> > > > > > [ 714.465211] lr : __arm_lpae_map+0x114/0x30c
> > > > > > [ 714.465215] sp : ffff80001006b340
> > > > > > [ 714.465219] x29: ffff80001006b340 x28: 0000002086538003
> > > > > > [ 714.465227] x27: 0000000000000a20 x26: 0000000000001000
> > > > > > [ 714.465236] x25: 0000000000000f44 x24: 00000020adf8d000
> > > > > > [ 714.465245] x23: 0000000000000001 x22: 0000fffffaeca000
> > > > > > [ 714.465253] x21: 0000000000000003 x20: ffff19b60d64d200
> > > > > > [ 714.465261] x19: 00000000000000ca x18: 0000000000000000
> > > > > > [ 714.465270] x17: 0000000000000000 x16: ffffcccb7cf3ca20
> > > > > > [ 714.465278] x15: 0000000000000000 x14: 0000000000000000
> > > > > > [ 714.465286] x13: 0000000000000003 x12: 0000000000000010
> > > > > > [ 714.465294] x11: 0000000000000000 x10: 0000000000000002
> > > > > > [ 714.465302] x9 : ffffcccb7d5b6e78 x8 : 00000000000001ff
> > > > > > [ 714.465311] x7 : ffff19b606538650 x6 : ffff19b606538000
> > > > > > [ 714.465319] x5 : 0000000000000009 x4 : 0000000000000f44
> > > > > > [ 714.465327] x3 : 0000000000001000 x2 : 00000020adf8d000
> > > > > > [ 714.465335] x1 : 0000000000000002 x0 : 0000000000000003
> > > > > > [ 714.465343] Call trace:
> > > > > > [ 714.465348] __arm_lpae_map+0x2d4/0x30c
> > > > > > [ 714.465353] __arm_lpae_map+0x114/0x30c
> > > > > > [ 714.465357] __arm_lpae_map+0x114/0x30c
> > > > > > [ 714.465362] __arm_lpae_map+0x114/0x30c
> > > > > > [ 714.465366] arm_lpae_map+0xf4/0x180
> > > > > > [ 714.465373] arm_smmu_map+0x4c/0xc0
> > > > > > [ 714.465379] __iommu_map+0x100/0x2bc
> > > > > > [ 714.465385] iommu_map_atomic+0x20/0x30
> > > > > > [ 714.465391] __iommu_dma_map+0xb0/0x110
> > > > > > [ 714.465397] iommu_dma_map_page+0xb8/0x120
> > > > > > [ 714.465404] dma_map_page_attrs+0x1a8/0x210
> > > > > > [ 714.465413] __dpaa2_eth_tx+0x384/0xbd0 [fsl_dpaa2_eth]
> > > > > > [ 714.465421] dpaa2_eth_tx+0x84/0x134 [fsl_dpaa2_eth]
> > > > > > [ 714.465427] dev_hard_start_xmit+0x10c/0x2b0
> > > > > > [ 714.465433] sch_direct_xmit+0x1a0/0x550
> > > > > > [ 714.465438] __qdisc_run+0x140/0x670
> > > > > > [ 714.465443] __dev_queue_xmit+0x6c4/0xa74
> > > > > > [ 714.465449] dev_queue_xmit+0x20/0x2c
> > > > > > [ 714.465463] br_dev_queue_push_xmit+0xc4/0x1a0 [bridge]
> > > > > > [ 714.465476] br_forward_finish+0xdc/0xf0 [bridge]
> > > > > > [ 714.465489] __br_forward+0x160/0x1c0 [bridge]
> > > > > > [ 714.465502] br_forward+0x13c/0x160 [bridge]
> > > > > > [ 714.465514] br_dev_xmit+0x228/0x3b0 [bridge]
> > > > > > [ 714.465520] dev_hard_start_xmit+0x10c/0x2b0
> > > > > > [ 714.465526] __dev_queue_xmit+0x8f0/0xa74
> > > > > > [ 714.465531] dev_queue_xmit+0x20/0x2c
> > > > > > [ 714.465538] arp_xmit+0xc0/0xd0
> > > > > > [ 714.465544] arp_send_dst+0x78/0xa0
> > > > > > [ 714.465550] arp_solicit+0xf4/0x260
> > > > > > [ 714.465554] neigh_probe+0x64/0xb0
> > > > > > [ 714.465560] neigh_timer_handler+0x2f4/0x400
> > > > > > [ 714.465566] call_timer_fn+0x3c/0x184
> > > > > > [ 714.465572] __run_timers.part.0+0x2bc/0x370
> > > > > > [ 714.465578] run_timer_softirq+0x48/0x80
> > > > > > [ 714.465583] __do_softirq+0x120/0x36c
> > > > > > [ 714.465589] irq_exit+0xac/0x100
> > > > > > [ 714.465596] __handle_domain_irq+0x8c/0xf0
> > > > > > [ 714.465600] gic_handle_irq+0xcc/0x14c
> > > > > > [ 714.465605] el1_irq+0xc4/0x180
> > > > > > [ 714.465610] arch_cpu_idle+0x18/0x30
> > > > > > [ 714.465617] default_idle_call+0x4c/0x180
> > > > > > [ 714.465623] do_idle+0x238/0x2b0
> > > > > > [ 714.465629] cpu_startup_entry+0x30/0xa0
> > > > > > [ 714.465636] secondary_start_kernel+0x134/0x180
> > > > > > [ 714.465640] ---[ end trace a84a7f61b559005f ]---
> > > > > >
> > > > > >
> > > > > > Given it is the iommu code that is provoking the warning I should
> > > > > > probably mention that the board I have requires
> > > > > > arm-smmu.disable_bypass=0 on the kernel command line in order to boot.
> > > > > > Also if it matters I am running the latest firmware from Solidrun
> > > > > > which is based on LSDK-20.04.
> > > > > >
> > > > >
> > > > > Hmmm, from what I remember I think I tested this with the smmu bypassed
> > > > > so that is why I didn't catch it.
> > > > >
> > > > > > Is there any reason for this code not to be working for LX2160A?
> > > > >
> > > > > I wouldn't expect this to be LX2160A specific but rather a bug in the
> > > > > implementation.. sorry.
> > > > >
> > > > > Let me reproduce it and see if I can get to the bottom of it and I will
> > > > > get back with some more info.
> > > > >
> > > >
> > > > Hi Daniel,
> > > >
> > > > It seems that the dma-unmapping on the SGT buffer was incorrectly done
> > > > with a zero size since on the Tx path I initialized the improper field.
> > > >
> > > > Could you test the following diff and let me know if you can generate
> > > > the WARNINGs anymore?
> > >
> > > I fired this up and, with your change, I've not been able to trigger
> > > the warning with the tests that I used the drive my bisect.
> > >
> >
> > Great, thanks for testing this.
> >
> > I will take care of sending the fix to -net.
> >
> > Ioana
>
> Ioana,
>
> Please CC me when you send the patch to -net, I will put my Tested-by: on it.
>
Hi Jon,
I sent out the patch yesterday. You are also copied.
https://lore.kernel.org/netdev/20201211171607.108034-1-ciorneiioana@gmail.com/
Ioana
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-12-12 19:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200629184712.12449-2-ioana.ciornei () nxp ! com>
[not found] ` <20201210173156.mbizovo6rxvkda73@holly.lan>
2020-12-10 18:06 ` [PATCH RESEND net-next 1/2] dpaa2-eth: send a scatter-gather FD instead of realloc-ing Ioana Ciornei
2020-12-11 7:21 ` Jon Nettleton
2020-12-11 14:01 ` Ioana Ciornei
2020-12-11 16:29 ` Daniel Thompson
2020-12-11 16:54 ` Ioana Ciornei
2020-12-12 15:58 ` Jon Nettleton
2020-12-12 19:31 ` Ioana Ciornei
2020-06-29 18:47 [PATCH RESEND net-next 0/2] " Ioana Ciornei
2020-06-29 18:47 ` [PATCH RESEND net-next 1/2] " Ioana Ciornei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).