| Message ID | 1569923958-99413-1-git-send-email-mutsugi@allied-telesis.co.jp |
|---|---|
| State | Accepted, archived |
| Delegated to: | Christian Lamparter |
| Headers | show |
| Series | [OpenWrt-Devel] ipq40xx: essedma: Fix dead lock | expand |
On 01/10/2019 11:59, Masafumi UTSUGI wrote: > edma_read_append_stats() is called from kernel timer > (Bottom half context) but it used spin_lock() to take a lock. > Using ethtool command rarely causes deadlock because of this. > Change lock function to spin_lock_bh() to avoid this. > Hi, patch looks good, could you please rebase it for v4.19 and merge the fix directly into the essedma patch ? John > Signed-off-by: Masafumi UTSUGI <mutsugi@allied-telesis.co.jp> > --- > .../patches-4.14/715-essedma-fix-dead-lock.patch | 20 ++++++++++++++++++++ > .../patches-4.19/715-essedma-fix-dead-lock.patch | 20 ++++++++++++++++++++ > 2 files changed, 40 insertions(+) > create mode 100644 target/linux/ipq40xx/patches-4.14/715-essedma-fix-dead-lock.patch > create mode 100644 target/linux/ipq40xx/patches-4.19/715-essedma-fix-dead-lock.patch > > diff --git a/target/linux/ipq40xx/patches-4.14/715-essedma-fix-dead-lock.patch b/target/linux/ipq40xx/patches-4.14/715-essedma-fix-dead-lock.patch > new file mode 100644 > index 0000000..1c44924 > --- /dev/null > +++ b/target/linux/ipq40xx/patches-4.14/715-essedma-fix-dead-lock.patch > @@ -0,0 +1,20 @@ > +--- a/drivers/net/ethernet/qualcomm/essedma/edma_axi.c > ++++ b/drivers/net/ethernet/qualcomm/essedma/edma_axi.c > +@@ -230,7 +230,7 @@ > + int i; > + u32 stat; > + > +- spin_lock(&edma_cinfo->stats_lock); > ++ spin_lock_bh(&edma_cinfo->stats_lock); > + p = (uint32_t *)&(edma_cinfo->edma_ethstats); > + > + for (i = 0; i < EDMA_MAX_TRANSMIT_QUEUE; i++) { > +@@ -257,7 +257,7 @@ > + p++; > + } > + > +- spin_unlock(&edma_cinfo->stats_lock); > ++ spin_unlock_bh(&edma_cinfo->stats_lock); > + } > + > + static void edma_statistics_timer(unsigned long data) > diff --git a/target/linux/ipq40xx/patches-4.19/715-essedma-fix-dead-lock.patch b/target/linux/ipq40xx/patches-4.19/715-essedma-fix-dead-lock.patch > new file mode 100644 > index 0000000..1c44924 > --- /dev/null > +++ b/target/linux/ipq40xx/patches-4.19/715-essedma-fix-dead-lock.patch > @@ -0,0 +1,20 @@ > +--- a/drivers/net/ethernet/qualcomm/essedma/edma_axi.c > ++++ b/drivers/net/ethernet/qualcomm/essedma/edma_axi.c > +@@ -230,7 +230,7 @@ > + int i; > + u32 stat; > + > +- spin_lock(&edma_cinfo->stats_lock); > ++ spin_lock_bh(&edma_cinfo->stats_lock); > + p = (uint32_t *)&(edma_cinfo->edma_ethstats); > + > + for (i = 0; i < EDMA_MAX_TRANSMIT_QUEUE; i++) { > +@@ -257,7 +257,7 @@ > + p++; > + } > + > +- spin_unlock(&edma_cinfo->stats_lock); > ++ spin_unlock_bh(&edma_cinfo->stats_lock); > + } > + > + static void edma_statistics_timer(unsigned long data) >
On Sunday, October 20, 2019 11:39:41 AM CEST John Crispin wrote: > On 01/10/2019 11:59, Masafumi UTSUGI wrote: > > edma_read_append_stats() is called from kernel timer > > (Bottom half context) but it used spin_lock() to take a lock. > > Using ethtool command rarely causes deadlock because of this. > > Change lock function to spin_lock_bh() to avoid this. > > > > Hi, > patch looks good, could you please rebase it for v4.19 and merge the fix > directly into the essedma patch ? Oh, I've already done that yesterday? https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=3e8fc768f78c6b7f7025dd15264d113da9ad81b2
On 20/10/2019 13:07, Christian Lamparter wrote: > On Sunday, October 20, 2019 11:39:41 AM CEST John Crispin wrote: >> On 01/10/2019 11:59, Masafumi UTSUGI wrote: >>> edma_read_append_stats() is called from kernel timer >>> (Bottom half context) but it used spin_lock() to take a lock. >>> Using ethtool command rarely causes deadlock because of this. >>> Change lock function to spin_lock_bh() to avoid this. >>> >> >> Hi, >> patch looks good, could you please rebase it for v4.19 and merge the fix >> directly into the essedma patch ? > > Oh, I've already done that yesterday? > > https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=3e8fc768f78c6b7f7025dd15264d113da9ad81b2 > > > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel > Ok, the patchwork ticket was still open :-)
It seems that it is not quite proper fix. Yesterday I flashed a Linksys EA6450v3 with 19.07 build. I couldn't connect to the device using a 100 Mbit laptop. I had to use another laptop with a 1000 Mbit port to reach the device. It seems that after the patch we avoided a lock on 1000 Mbit but got a lock on lower ones.
diff --git a/target/linux/ipq40xx/patches-4.14/715-essedma-fix-dead-lock.patch b/target/linux/ipq40xx/patches-4.14/715-essedma-fix-dead-lock.patch new file mode 100644 index 0000000..1c44924 --- /dev/null +++ b/target/linux/ipq40xx/patches-4.14/715-essedma-fix-dead-lock.patch @@ -0,0 +1,20 @@ +--- a/drivers/net/ethernet/qualcomm/essedma/edma_axi.c ++++ b/drivers/net/ethernet/qualcomm/essedma/edma_axi.c +@@ -230,7 +230,7 @@ + int i; + u32 stat; + +- spin_lock(&edma_cinfo->stats_lock); ++ spin_lock_bh(&edma_cinfo->stats_lock); + p = (uint32_t *)&(edma_cinfo->edma_ethstats); + + for (i = 0; i < EDMA_MAX_TRANSMIT_QUEUE; i++) { +@@ -257,7 +257,7 @@ + p++; + } + +- spin_unlock(&edma_cinfo->stats_lock); ++ spin_unlock_bh(&edma_cinfo->stats_lock); + } + + static void edma_statistics_timer(unsigned long data) diff --git a/target/linux/ipq40xx/patches-4.19/715-essedma-fix-dead-lock.patch b/target/linux/ipq40xx/patches-4.19/715-essedma-fix-dead-lock.patch new file mode 100644 index 0000000..1c44924 --- /dev/null +++ b/target/linux/ipq40xx/patches-4.19/715-essedma-fix-dead-lock.patch @@ -0,0 +1,20 @@ +--- a/drivers/net/ethernet/qualcomm/essedma/edma_axi.c ++++ b/drivers/net/ethernet/qualcomm/essedma/edma_axi.c +@@ -230,7 +230,7 @@ + int i; + u32 stat; + +- spin_lock(&edma_cinfo->stats_lock); ++ spin_lock_bh(&edma_cinfo->stats_lock); + p = (uint32_t *)&(edma_cinfo->edma_ethstats); + + for (i = 0; i < EDMA_MAX_TRANSMIT_QUEUE; i++) { +@@ -257,7 +257,7 @@ + p++; + } + +- spin_unlock(&edma_cinfo->stats_lock); ++ spin_unlock_bh(&edma_cinfo->stats_lock); + } + + static void edma_statistics_timer(unsigned long data)
edma_read_append_stats() is called from kernel timer (Bottom half context) but it used spin_lock() to take a lock. Using ethtool command rarely causes deadlock because of this. Change lock function to spin_lock_bh() to avoid this. Signed-off-by: Masafumi UTSUGI <mutsugi@allied-telesis.co.jp> --- .../patches-4.14/715-essedma-fix-dead-lock.patch | 20 ++++++++++++++++++++ .../patches-4.19/715-essedma-fix-dead-lock.patch | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 target/linux/ipq40xx/patches-4.14/715-essedma-fix-dead-lock.patch create mode 100644 target/linux/ipq40xx/patches-4.19/715-essedma-fix-dead-lock.patch