* Re: [PATCH net-next v3 1/6] net: phy: mscc: migrate to phy_select/restore_page functions
From: Andreas Schwab @ 2018-11-19 16:32 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexandre Belloni, Quentin Schulz, davem, f.fainelli,
allan.nielsen, linux-kernel, netdev, thomas.petazzoni,
linux-riscv
In-Reply-To: <20181119162529.GA31955@lunn.ch>
On Nov 19 2018, Andrew Lunn <andrew@lunn.ch> wrote:
> Lock debugging: prove locking correctness
404
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH net-next v3 1/6] net: phy: mscc: migrate to phy_select/restore_page functions
From: Andrew Lunn @ 2018-11-19 16:44 UTC (permalink / raw)
To: Andreas Schwab
Cc: Alexandre Belloni, f.fainelli, Quentin Schulz, netdev,
linux-kernel, allan.nielsen, thomas.petazzoni, linux-riscv, davem
In-Reply-To: <mvmo9al9gfl.fsf@suse.de>
On Mon, Nov 19, 2018 at 05:32:14PM +0100, Andreas Schwab wrote:
> On Nov 19 2018, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Lock debugging: prove locking correctness
>
Lets see how much this gets messed up by email...
.config - Linux/arm 4.20.0-rc2 Kernel Configuration
> Kernel hacking > Lock Debugging (spinlocks, mutexes, etc...) ──────
┌───────── Lock Debugging (spinlocks, mutexes, etc...) ──────────┐
│ Arrow keys navigate the menu. <Enter> selects submenus ---> │
│ (or empty submenus ----). Highlighted letters are hotkeys. │
│ Pressing <Y> includes, <N> excludes, <M> modularizes │
│ features. Press <Esc><Esc> to exit, <?> for Help, </> for │
│ ┌────────────────────────────────────────────────────────────┐ │
│ │ [*] Lock debugging: prove locking correctness │ │
│ │ [ ] Lock usage statistics │ │
│ │ -*- RT Mutex debugging, deadlock detection │ │
│ │ -*- Spinlock and rw-lock debugging: basic checks │ │
│ │ -*- Mutex debugging: basic checks │ │
│ │ -*- Wait/wound mutex debugging: Slowpath testing │ │
│ │ -*- RW Semaphore debugging: basic checks │ │
│ │ -*- Lock debugging: detect incorrect freeing of live loc│ │
│ │ [ ] Lock dependency engine debugging │ │
│ │ [*] Sleep inside atomic section checking │ │
│ │ [ ] Locking API boot-time self-tests │ │
│ │ < > torture tests for locking │ │
│ │ < > Wait/wound mutex selftests │ │
│ └────────────────────────────────────────────────────────────┘ │
├────────────────────────────────────────────────────────────────┤
│ <Select> < Exit > < Help > < Save > < Load > │
└────────────────────────────────────────────────────────────────┘
If you cannot see it at all, it probably means you are missing a
dependency:
│ Depends on: DEBUG_KERNEL [=y] && \ │
│ LOCK_DEBUGGING_SUPPORT [=y] │
Andrew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply
* Re: [PATCH v2 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
From: Antoine Tenart @ 2018-11-19 16:48 UTC (permalink / raw)
To: Maxime Chevallier
Cc: Alexandre Belloni, Grygorii Strashko, Quentin Schulz, Manu Gautam,
Tony Lindgren, netdev, Antoine Tenart, Sekhar Nori,
Russell King - ARM Linux, Kishon Vijay Abraham I, Maxime Ripard,
Chen-Yu Tsai, Chunfeng Yun, linux-mediatek, linux-arm-kernel,
Vivek Gautam, Carlo Caione, linux-amlogic, David S. Miller,
linux-kernel, Matthias Brugger
In-Reply-To: <20181119092632.7048bc46@bootlin.com>
Hi,
On Mon, Nov 19, 2018 at 09:26:32AM +0100, Maxime Chevallier wrote:
>
> On Fri, 9 Nov 2018 17:47:54 -0600
> Grygorii Strashko <grygorii.strashko@ti.com> wrote:
>
> >Convert mvebu-cp110-comphy PHY driver to use recently introduced
> >PHY_MODE_ETHERNET and phy_set_mode_ext().
>
> Sorry I missed your V2, hopefully I tested the right version this time.
> Tested on MCBin, this works just fine.
>
> Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Thank you for testing!
Acked-by: Antoine Tenart <antoine.tenart@bootlin.com>
Antoine
> >Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> >---
> > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +-----
> > drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 83 ++++++++++++++-----------
> > 2 files changed, 48 insertions(+), 54 deletions(-)
> >
> >diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> >index 7a37a37..731793a 100644
> >--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> >+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> >@@ -1165,28 +1165,13 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
> > */
> > static int mvpp22_comphy_init(struct mvpp2_port *port)
> > {
> >- enum phy_mode mode;
> > int ret;
> >
> > if (!port->comphy)
> > return 0;
> >
> >- switch (port->phy_interface) {
> >- case PHY_INTERFACE_MODE_SGMII:
> >- case PHY_INTERFACE_MODE_1000BASEX:
> >- mode = PHY_MODE_SGMII;
> >- break;
> >- case PHY_INTERFACE_MODE_2500BASEX:
> >- mode = PHY_MODE_2500SGMII;
> >- break;
> >- case PHY_INTERFACE_MODE_10GKR:
> >- mode = PHY_MODE_10GKR;
> >- break;
> >- default:
> >- return -EINVAL;
> >- }
> >-
> >- ret = phy_set_mode(port->comphy, mode);
> >+ ret = phy_set_mode_ext(port->comphy, PHY_MODE_ETHERNET,
> >+ port->phy_interface);
> > if (ret)
> > return ret;
> >
> >diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> >index 79b52c3..7dee72b 100644
> >--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> >+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> >@@ -9,6 +9,7 @@
> > #include <linux/iopoll.h>
> > #include <linux/mfd/syscon.h>
> > #include <linux/module.h>
> >+#include <linux/phy.h>
> > #include <linux/phy/phy.h>
> > #include <linux/platform_device.h>
> > #include <linux/regmap.h>
> >@@ -131,26 +132,26 @@ struct mvebu_comhy_conf {
> >
> > static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
> > /* lane 0 */
> >- MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
> >- MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
> >+ MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
> >+ MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> > /* lane 1 */
> >- MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
> >- MVEBU_COMPHY_CONF(1, 2, PHY_MODE_2500SGMII, 0x1),
> >+ MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> >+ MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> > /* lane 2 */
> >- MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
> >- MVEBU_COMPHY_CONF(2, 0, PHY_MODE_2500SGMII, 0x1),
> >- MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
> >+ MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_SGMII, 0x1),
> >+ MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> >+ MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_10GKR, 0x1),
> > /* lane 3 */
> >- MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
> >- MVEBU_COMPHY_CONF(3, 1, PHY_MODE_2500SGMII, 0x2),
> >+ MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_SGMII, 0x2),
> >+ MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_2500BASEX, 0x2),
> > /* lane 4 */
> >- MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
> >- MVEBU_COMPHY_CONF(4, 0, PHY_MODE_2500SGMII, 0x2),
> >- MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
> >- MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
> >+ MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_SGMII, 0x2),
> >+ MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_2500BASEX, 0x2),
> >+ MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_10GKR, 0x2),
> >+ MVEBU_COMPHY_CONF(4, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
> > /* lane 5 */
> >- MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
> >- MVEBU_COMPHY_CONF(5, 2, PHY_MODE_2500SGMII, 0x1),
> >+ MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> >+ MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> > };
> >
> > struct mvebu_comphy_priv {
> >@@ -163,10 +164,12 @@ struct mvebu_comphy_lane {
> > struct mvebu_comphy_priv *priv;
> > unsigned id;
> > enum phy_mode mode;
> >+ int submode;
> > int port;
> > };
> >
> >-static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> >+static int mvebu_comphy_get_mux(int lane, int port,
> >+ enum phy_mode mode, int submode)
> > {
> > int i, n = ARRAY_SIZE(mvebu_comphy_cp110_modes);
> >
> >@@ -177,7 +180,7 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> > for (i = 0; i < n; i++) {
> > if (mvebu_comphy_cp110_modes[i].lane == lane &&
> > mvebu_comphy_cp110_modes[i].port == port &&
> >- mvebu_comphy_cp110_modes[i].mode == mode)
> >+ mvebu_comphy_cp110_modes[i].mode == submode)
> > break;
> > }
> >
> >@@ -187,8 +190,7 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> > return mvebu_comphy_cp110_modes[i].mux;
> > }
> >
> >-static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> >- enum phy_mode mode)
> >+static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane)
> > {
> > struct mvebu_comphy_priv *priv = lane->priv;
> > u32 val;
> >@@ -206,14 +208,14 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> > MVEBU_COMPHY_SERDES_CFG0_HALF_BUS |
> > MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xf) |
> > MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xf));
> >- if (mode == PHY_MODE_10GKR)
> >+ if (lane->submode == PHY_INTERFACE_MODE_10GKR)
> > val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xe) |
> > MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xe);
> >- else if (mode == PHY_MODE_2500SGMII)
> >+ else if (lane->submode == PHY_INTERFACE_MODE_2500BASEX)
> > val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x8) |
> > MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x8) |
> > MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
> >- else if (mode == PHY_MODE_SGMII)
> >+ else if (lane->submode == PHY_INTERFACE_MODE_SGMII)
> > val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x6) |
> > MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x6) |
> > MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
> >@@ -243,7 +245,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> > /* refclk selection */
> > val = readl(priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
> > val &= ~MVEBU_COMPHY_MISC_CTRL0_REFCLK_SEL;
> >- if (mode == PHY_MODE_10GKR)
> >+ if (lane->submode == PHY_INTERFACE_MODE_10GKR)
> > val |= MVEBU_COMPHY_MISC_CTRL0_ICP_FORCE;
> > writel(val, priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
> >
> >@@ -261,8 +263,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> > writel(val, priv->base + MVEBU_COMPHY_LOOPBACK(lane->id));
> > }
> >
> >-static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
> >- enum phy_mode mode)
> >+static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane)
> > {
> > struct mvebu_comphy_priv *priv = lane->priv;
> > u32 val;
> >@@ -303,13 +304,13 @@ static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
> > return 0;
> > }
> >
> >-static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
> >+static int mvebu_comphy_set_mode_sgmii(struct phy *phy)
> > {
> > struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
> > struct mvebu_comphy_priv *priv = lane->priv;
> > u32 val;
> >
> >- mvebu_comphy_ethernet_init_reset(lane, mode);
> >+ mvebu_comphy_ethernet_init_reset(lane);
> >
> > val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
> > val &= ~MVEBU_COMPHY_RX_CTRL1_CLK8T_EN;
> >@@ -330,7 +331,7 @@ static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
> > val |= MVEBU_COMPHY_GEN1_S0_TX_EMPH(0x1);
> > writel(val, priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
> >
> >- return mvebu_comphy_init_plls(lane, PHY_MODE_SGMII);
> >+ return mvebu_comphy_init_plls(lane);
> > }
> >
> > static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
> >@@ -339,7 +340,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
> > struct mvebu_comphy_priv *priv = lane->priv;
> > u32 val;
> >
> >- mvebu_comphy_ethernet_init_reset(lane, PHY_MODE_10GKR);
> >+ mvebu_comphy_ethernet_init_reset(lane);
> >
> > val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
> > val |= MVEBU_COMPHY_RX_CTRL1_RXCLK2X_SEL |
> >@@ -469,7 +470,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
> > val |= MVEBU_COMPHY_EXT_SELV_RX_SAMPL(0x1a);
> > writel(val, priv->base + MVEBU_COMPHY_EXT_SELV(lane->id));
> >
> >- return mvebu_comphy_init_plls(lane, PHY_MODE_10GKR);
> >+ return mvebu_comphy_init_plls(lane);
> > }
> >
> > static int mvebu_comphy_power_on(struct phy *phy)
> >@@ -479,7 +480,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
> > int ret, mux;
> > u32 val;
> >
> >- mux = mvebu_comphy_get_mux(lane->id, lane->port, lane->mode);
> >+ mux = mvebu_comphy_get_mux(lane->id, lane->port,
> >+ lane->mode, lane->submode);
> > if (mux < 0)
> > return -ENOTSUPP;
> >
> >@@ -492,12 +494,12 @@ static int mvebu_comphy_power_on(struct phy *phy)
> > val |= mux << MVEBU_COMPHY_SELECTOR_PHY(lane->id);
> > regmap_write(priv->regmap, MVEBU_COMPHY_SELECTOR, val);
> >
> >- switch (lane->mode) {
> >- case PHY_MODE_SGMII:
> >- case PHY_MODE_2500SGMII:
> >- ret = mvebu_comphy_set_mode_sgmii(phy, lane->mode);
> >+ switch (lane->submode) {
> >+ case PHY_INTERFACE_MODE_SGMII:
> >+ case PHY_INTERFACE_MODE_2500BASEX:
> >+ ret = mvebu_comphy_set_mode_sgmii(phy);
> > break;
> >- case PHY_MODE_10GKR:
> >+ case PHY_INTERFACE_MODE_10GKR:
> > ret = mvebu_comphy_set_mode_10gkr(phy);
> > break;
> > default:
> >@@ -517,10 +519,17 @@ static int mvebu_comphy_set_mode(struct phy *phy,
> > {
> > struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
> >
> >- if (mvebu_comphy_get_mux(lane->id, lane->port, mode) < 0)
> >+ if (mode != PHY_MODE_ETHERNET)
> >+ return -EINVAL;
> >+
> >+ if (submode == PHY_INTERFACE_MODE_1000BASEX)
> >+ submode = PHY_INTERFACE_MODE_SGMII;
> >+
> >+ if (mvebu_comphy_get_mux(lane->id, lane->port, mode, submode) < 0)
> > return -EINVAL;
> >
> > lane->mode = mode;
> >+ lane->submode = submode;
> > return 0;
> > }
> >
>
>
>
> --
> Maxime Chevallier, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Jerome Glisse @ 2018-11-19 16:48 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Kenneth Lee, Tim Sell, linux-doc, Alexander Shishkin, Zaibo Xu,
zhangfei.gao, linuxarm, haojian.zhuang, Christoph Lameter,
Hao Fang, Gavin Schenk, RDMA mailing list, Zhou Wang,
Jason Gunthorpe, Doug Ledford, Uwe Kleine-König,
David Kershner, Kenneth Lee, Johan Hovold, Cyrille Pitchen
In-Reply-To: <20181119104801.GF8268@mtr-leonro.mtl.com>
On Mon, Nov 19, 2018 at 12:48:01PM +0200, Leon Romanovsky wrote:
> On Mon, Nov 19, 2018 at 05:19:10PM +0800, Kenneth Lee wrote:
> > On Mon, Nov 19, 2018 at 05:14:05PM +0800, Kenneth Lee wrote:
> > > On Thu, Nov 15, 2018 at 04:54:55PM +0200, Leon Romanovsky wrote:
> > > > On Thu, Nov 15, 2018 at 04:51:09PM +0800, Kenneth Lee wrote:
> > > > > On Wed, Nov 14, 2018 at 06:00:17PM +0200, Leon Romanovsky wrote:
> > > > > > On Wed, Nov 14, 2018 at 10:58:09AM +0800, Kenneth Lee wrote:
> > > > > > > > On Mon, Nov 12, 2018 at 03:58:02PM +0800, Kenneth Lee wrote:
[...]
> > > > memory exposed to user is properly protected from security point of view.
> > > > 3. "stop using the page for a while for the copying" - I'm not fully
> > > > understand this claim, maybe this article will help you to better
> > > > describe : https://lwn.net/Articles/753027/
> > >
> > > This topic was being discussed in RFCv2. The key problem here is that:
> > >
> > > The device need to hold the memory for its own calculation, but the CPU/software
> > > want to stop it for a while for synchronizing with disk or COW.
> > >
> > > If the hardware support SVM/SVA (Shared Virtual Memory/Address), it is easy, the
> > > device share page table with CPU, the device will raise a page fault when the
> > > CPU downgrade the PTE to read-only.
> > >
> > > If the hardware cannot share page table with the CPU, we then need to have
> > > some way to change the device page table. This is what happen in ODP. It
> > > invalidates the page table in device upon mmu_notifier call back. But this cannot
> > > solve the COW problem: if the user process A share a page P with device, and A
> > > forks a new process B, and it continue to write to the page. By COW, the
> > > process B will keep the page P, while A will get a new page P'. But you have
> > > no way to let the device know it should use P' rather than P.
>
> I didn't hear about such issue and we supported fork for a long time.
>
Just to comment on this, any infiniband driver which use umem and do
not have ODP (here ODP for me means listening to mmu notifier so all
infiniband driver except mlx5) will be affected by same issue AFAICT.
AFAICT there is no special thing happening after fork() inside any of
those driver. So if parent create a umem mr before fork() and program
hardware with it then after fork() the parent might start using new
page for the umem range while the old memory is use by the child. The
reverse is also true (parent using old memory and child new memory)
bottom line you can not predict which memory the child or the parent
will use for the range after fork().
So no matter what you consider the child or the parent, what the hw
will use for the mr is unlikely to match what the CPU use for the
same virtual address. In other word:
Before fork:
CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
Case 1:
CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
CPU child: virtual addr ptr1 -> physical address = 0xDEAD
HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
Case 2:
CPU parent: virtual addr ptr1 -> physical address = 0xBEEF
CPU child: virtual addr ptr1 -> physical address = 0xCAFE
HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
This apply for every single page and is not predictable. This only
apply to private memory (mmap() with MAP_PRIVATE)
I am not familiar enough with RDMA user space API contract to know
if this is an issue or not.
Note that this can not be fix, no one should have done umem without
ODP like mlx5. For this to work properly you need sane hardware like
mlx5.
Cheers,
Jérôme
^ permalink raw reply
* Re: [PATCH v2 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
From: Russell King - ARM Linux @ 2018-11-19 17:07 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, Kishon Vijay Abraham I, netdev, Sekhar Nori,
linux-kernel, linux-arm-kernel, Tony Lindgren, linux-amlogic,
linux-mediatek, Alexandre Belloni, Antoine Tenart, Quentin Schulz,
Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
Chunfeng Yun, Matthias Brugger, Manu Gautam
In-Reply-To: <20181109234755.21687-5-grygorii.strashko@ti.com>
On Fri, Nov 09, 2018 at 05:47:54PM -0600, Grygorii Strashko wrote:
> Convert mvebu-cp110-comphy PHY driver to use recently introduced
> PHY_MODE_ETHERNET and phy_set_mode_ext().
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +-----
> drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 83 ++++++++++++++-----------
> 2 files changed, 48 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 7a37a37..731793a 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1165,28 +1165,13 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
> */
> static int mvpp22_comphy_init(struct mvpp2_port *port)
> {
> - enum phy_mode mode;
> int ret;
>
> if (!port->comphy)
> return 0;
>
> - switch (port->phy_interface) {
> - case PHY_INTERFACE_MODE_SGMII:
> - case PHY_INTERFACE_MODE_1000BASEX:
> - mode = PHY_MODE_SGMII;
> - break;
> - case PHY_INTERFACE_MODE_2500BASEX:
> - mode = PHY_MODE_2500SGMII;
> - break;
> - case PHY_INTERFACE_MODE_10GKR:
> - mode = PHY_MODE_10GKR;
> - break;
> - default:
> - return -EINVAL;
> - }
> -
> - ret = phy_set_mode(port->comphy, mode);
> + ret = phy_set_mode_ext(port->comphy, PHY_MODE_ETHERNET,
> + port->phy_interface);
> if (ret)
> return ret;
>
> diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> index 79b52c3..7dee72b 100644
> --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> @@ -9,6 +9,7 @@
> #include <linux/iopoll.h>
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> +#include <linux/phy.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> @@ -131,26 +132,26 @@ struct mvebu_comhy_conf {
>
> static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
> /* lane 0 */
> - MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
> - MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
> + MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> /* lane 1 */
> - MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
> - MVEBU_COMPHY_CONF(1, 2, PHY_MODE_2500SGMII, 0x1),
> + MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> /* lane 2 */
> - MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
> - MVEBU_COMPHY_CONF(2, 0, PHY_MODE_2500SGMII, 0x1),
> - MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
> + MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> + MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_10GKR, 0x1),
> /* lane 3 */
> - MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
> - MVEBU_COMPHY_CONF(3, 1, PHY_MODE_2500SGMII, 0x2),
> + MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_SGMII, 0x2),
> + MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_2500BASEX, 0x2),
> /* lane 4 */
> - MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
> - MVEBU_COMPHY_CONF(4, 0, PHY_MODE_2500SGMII, 0x2),
> - MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
> - MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_SGMII, 0x2),
> + MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_2500BASEX, 0x2),
> + MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_10GKR, 0x2),
> + MVEBU_COMPHY_CONF(4, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
> /* lane 5 */
> - MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
> - MVEBU_COMPHY_CONF(5, 2, PHY_MODE_2500SGMII, 0x1),
> + MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> + MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> };
>
> struct mvebu_comphy_priv {
> @@ -163,10 +164,12 @@ struct mvebu_comphy_lane {
> struct mvebu_comphy_priv *priv;
> unsigned id;
> enum phy_mode mode;
> + int submode;
> int port;
> };
>
> -static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> +static int mvebu_comphy_get_mux(int lane, int port,
> + enum phy_mode mode, int submode)
> {
> int i, n = ARRAY_SIZE(mvebu_comphy_cp110_modes);
>
> @@ -177,7 +180,7 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> for (i = 0; i < n; i++) {
> if (mvebu_comphy_cp110_modes[i].lane == lane &&
> mvebu_comphy_cp110_modes[i].port == port &&
> - mvebu_comphy_cp110_modes[i].mode == mode)
> + mvebu_comphy_cp110_modes[i].mode == submode)
This looks like it could be a future source of problems, since we
have both a "mode" and a "submode". Is there any reason not to
rename mvebu_comphy_cp110_modes's .mode member to be .submode ?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* [PATCH net-next] MAINTAINERS: Add myself as third phylib maintainer
From: Heiner Kallweit @ 2018-11-19 7:01 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
Add myself as third phylib maintainer.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 7ff8865a9..2988ecbf6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5540,6 +5540,7 @@ F: net/bridge/
ETHERNET PHY LIBRARY
M: Andrew Lunn <andrew@lunn.ch>
M: Florian Fainelli <f.fainelli@gmail.com>
+M: Heiner Kallweit <hkallweit1@gmail.com>
L: netdev@vger.kernel.org
S: Maintained
F: Documentation/ABI/testing/sysfs-bus-mdio
--
2.19.1
^ permalink raw reply related
* Re: selftests: net: udpgro.sh hangs on DUT devices running Linux -next
From: Naresh Kamboju @ 2018-11-19 7:06 UTC (permalink / raw)
To: pabeni
Cc: netdev, David S. Miller, Willem de Bruijn, Anders Roxell,
Rafael Tinoco
In-Reply-To: <ca4189ce99d6256f894cf55577a21ef7ad142262.camel@redhat.com>
Hi Paolo,
On Sun, 18 Nov 2018 at 05:01, Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> On Fri, 2018-11-16 at 14:55 +0530, Naresh Kamboju wrote:
> > Kernel selftests: net: udpgro.sh hangs / waits forever on x86_64 and
> > arm32 devices running Linux -next. Test getting PASS on arm64 devices.
> >
> > Do you see this problem ?
> >
> > Short error log:
> > ---------------------
> > ip6tables v1.6.1: can't initialize ip6tables table `nat': Table does
> > not exist (do you need to insmod?)
>
> Thank you for the report.
>
> It looks like your kernel config has
>
> # CONFIG_NF_NAT_IPV6 is not set
>
> Can you please confirm ?
# CONFIG_NF_NAT_IPV6 is not set on the devices where the test is getting hang.
>
> net selftests do not explicitly ask for that, despiting using such
> functionality (my bad).
>
> I'll be travelling up to Monday (included). I'll have a better look
> after that.
Thank you.
- Naresh
>
> Cheers,
>
> Paolo
>
^ permalink raw reply
* Re: [GIT] Networking
From: pr-tracker-bot @ 2018-11-19 17:30 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20181118.191242.1498284515936666603.davem@davemloft.net>
The pull request you sent on Sun, 18 Nov 2018 19:12:42 -0800 (PST):
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net master
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/f2ce1065e767fc7da106a5f5381d1e8f842dc6f4
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* RE: [PATCH v2 2/2] dpaa_eth: add ethtool coalesce control
From: Madalin-cristian Bucur @ 2018-11-19 7:11 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, Leo Li, Roy Pledge,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20181116.194219.2027125174407236599.davem@davemloft.net>
> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Saturday, November 17, 2018 5:42 AM
> To: Madalin-cristian Bucur <madalin.bucur@nxp.com>
> Subject: Re: [PATCH v2 2/2] dpaa_eth: add ethtool coalesce control
>
> From: Madalin Bucur <madalin.bucur@nxp.com>
> Date: Tue, 13 Nov 2018 18:29:51 +0200
>
> > + for_each_cpu(cpu, cpus) {
> > + portal = qman_get_affine_portal(cpu);
> > + res = qman_portal_set_iperiod(portal, period);
> > + if (res)
> > + return res;
> > + res = qman_dqrr_set_ithresh(portal, thresh);
> > + if (res)
> > + return res;
>
> Nope, you can't do it like this.
>
> If any intermediate change fails, you have to unwind all of the
> changes made up until that point.
>
> Which means you'll have to store the previous setting somewhere
> and reinstall those saved values in the error path.
Thank you, I'll come back with a v3.
Madalin
^ permalink raw reply
* Re: [PATCH net] sctp: always set frag_point on pmtu change
From: Xin Long @ 2018-11-19 7:20 UTC (permalink / raw)
To: jakub.audykowicz
Cc: linux-sctp, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner,
davem, network dev
In-Reply-To: <20181118204736.7178-1-jakub.audykowicz@gmail.com>
On Mon, Nov 19, 2018 at 5:49 AM Jakub Audykowicz
<jakub.audykowicz@gmail.com> wrote:
>
> Calling send on a connected SCTP socket results in kernel panic if
> spp_pathmtu was configured manually before an association is established
> and it was not reconfigured to another value once the association is
> established.
>
> Steps to reproduce:
> 1. Set up a listening SCTP server socket.
> 2. Set up an SCTP client socket.
> 3. Configure client socket using setsockopt SCTP_PEER_ADDR_PARAMS with
> spp_pathmtu set to a legal value (e.g. 1000) and
> SPP_PMTUD_DISABLE set in spp_flags.
> 4. Connect client to server.
> 5. Send message from client to server.
>
> At this point oom-killer is invoked, which will eventually lead to:
> [ 5.197262] Out of memory and no killable processes...
> [ 5.198107] Kernel panic - not syncing: System is deadlocked on memory
>
> Commit 2f5e3c9df693 ("sctp: introduce sctp_assoc_update_frag_point")
> introduces sctp_assoc_update_frag_point, but this function is not called
> in this case, causing frag_point to be zero:
> void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu)
> {
> - if (asoc->pathmtu != pmtu)
> + if (asoc->pathmtu != pmtu) {
> asoc->pathmtu = pmtu;
> + sctp_assoc_update_frag_point(asoc);
> + }
>
> In this scenario, on association establishment, asoc->pathmtu is already
> 1000 and pmtu will be as well. Before this commit the frag_point was being
> correctly set in the scenario described. Moving the call outside the if
> block fixes the issue.
>
> I will be providing a separate patch to lksctp-tools with a simple test
> reproducing this problem ("func_tests: frag_point should never be zero").
>
> I have also taken the liberty to introduce a sanity check in chunk.c to
> set the frag_point to a non-negative value in order to avoid chunking
> endlessly (which is the reason for the eventual panic).
>
> Fixes: 2f5e3c9df693 ("sctp: introduce sctp_assoc_update_frag_point")
> Signed-off-by: Jakub Audykowicz <jakub.audykowicz@gmail.com>
> ---
> include/net/sctp/constants.h | 3 +++
> net/sctp/associola.c | 13 +++++++------
> net/sctp/chunk.c | 6 ++++++
> 3 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
> index 8dadc74c22e7..90316fab6f04 100644
> --- a/include/net/sctp/constants.h
> +++ b/include/net/sctp/constants.h
> @@ -293,6 +293,9 @@ enum { SCTP_MAX_GABS = 16 };
> */
> #define SCTP_DEFAULT_MINSEGMENT 512 /* MTU size ... if no mtu disc */
>
> +/* An association's fragmentation point should never be non-positive */
> +#define SCTP_FRAG_POINT_MIN 1
> +
> #define SCTP_SECRET_SIZE 32 /* Number of octets in a 256 bits. */
>
> #define SCTP_SIGNATURE_SIZE 20 /* size of a SLA-1 signature */
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 6a28b96e779e..44d71a1af62e 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1431,13 +1431,14 @@ void sctp_assoc_update_frag_point(struct sctp_association *asoc)
>
> void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu)
> {
> - if (asoc->pathmtu != pmtu) {
> - asoc->pathmtu = pmtu;
> - sctp_assoc_update_frag_point(asoc);
> - }
> + pr_debug("%s: before asoc:%p, pmtu:%d, frag_point:%d\n",
> + __func__, asoc, asoc->pathmtu, asoc->frag_point);
> +
> + asoc->pathmtu = pmtu;
> + sctp_assoc_update_frag_point(asoc);
>
> - pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
> - asoc->pathmtu, asoc->frag_point);
> + pr_debug("%s: after asoc:%p, pmtu:%d, frag_point:%d\n",
> + __func__, asoc, asoc->pathmtu, asoc->frag_point);
> }
The idea was whenever asoc->pathmtu changes, frag_point should
be updated, but we missed one place in sctp_association_init().
Another issue is after 4-shakehand, the client's asoc->intl_enable
may be changed from 0 to 1, which means the frag_point should
also be updated, since [1]:
void sctp_assoc_update_frag_point(struct sctp_association *asoc)
{
int frag = sctp_mtu_payload(sctp_sk(asoc->base.sk), asoc->pathmtu,
sctp_datachk_len(&asoc->stream)); <--- [1]
So one fix for both issues is:
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index 0a78cdf..19d596d 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -1327,4 +1327,5 @@ void sctp_stream_interleave_init(struct
sctp_stream *stream)
asoc = container_of(stream, struct sctp_association, stream);
stream->si = asoc->intl_enable ? &sctp_stream_interleave_1
: &sctp_stream_interleave_0;
+ sctp_assoc_update_frag_point(asoc);
}
>
> /* Update the association's pmtu and frag_point by going through all the
> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> index ce8087846f05..9f0e64ddbd9c 100644
> --- a/net/sctp/chunk.c
> +++ b/net/sctp/chunk.c
> @@ -190,6 +190,12 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> /* This is the biggest possible DATA chunk that can fit into
> * the packet
> */
> + if (asoc->frag_point < SCTP_FRAG_POINT_MIN) {
> + pr_warn("%s: asoc:%p->frag_point is less than allowed (%d<%d)",
> + __func__, asoc, asoc->frag_point, SCTP_FRAG_POINT_MIN);
> + pr_warn("forcing minimum value to avoid chunking endlessly");
> + asoc->frag_point = SCTP_FRAG_POINT_MIN;
> + }
> max_data = asoc->frag_point;
This won't happen if we sync frag_point on time like the above.
^ permalink raw reply related
* [PATCH net-next v2 0/2] net: bcmgenet: fix aborted suspend
From: Doug Berger @ 2018-11-19 17:56 UTC (permalink / raw)
To: David S. Miller
Cc: Florian Fainelli, netdev, linux-kernel, YueHaibing, Doug Berger
It is not enough to return an error code from the driver suspend
routine. The driver must also restore the device functionality.
This commit corrects the issue introduced by commit 0db55093b566
("net: bcmgenet: return correct value 'ret' from bcmgenet_power_down")
by calling the driver resume function if the suspend function returns
an error.
Changes in v2:
- Removed accidentally included write to HFB register
Doug Berger (2):
net: bcmgenet: code movement
net: bcmgenet: abort suspend on error
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 63 +++++++++++-----------
drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 2 +
2 files changed, 35 insertions(+), 30 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next v2 1/2] net: bcmgenet: code movement
From: Doug Berger @ 2018-11-19 17:56 UTC (permalink / raw)
To: David S. Miller
Cc: Florian Fainelli, netdev, linux-kernel, YueHaibing, Doug Berger
In-Reply-To: <1542650162-9229-1-git-send-email-opendmb@gmail.com>
From: Doug Berger <doug.berger@broadcom.com>
This commit switches the order of bcmgenet_suspend and bcmgenet_resume
in the file to prevent the need for a forward declaration in the next
commit and to make the review of that commit easier.
Signed-off-by: Doug Berger <doug.berger@broadcom.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 60 +++++++++++++-------------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index bf88749505a9..01c8499fd87f 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3612,36 +3612,6 @@ static int bcmgenet_remove(struct platform_device *pdev)
}
#ifdef CONFIG_PM_SLEEP
-static int bcmgenet_suspend(struct device *d)
-{
- struct net_device *dev = dev_get_drvdata(d);
- struct bcmgenet_priv *priv = netdev_priv(dev);
- int ret = 0;
-
- if (!netif_running(dev))
- return 0;
-
- netif_device_detach(dev);
-
- bcmgenet_netif_stop(dev);
-
- if (!device_may_wakeup(d))
- phy_suspend(dev->phydev);
-
- /* Prepare the device for Wake-on-LAN and switch to the slow clock */
- if (device_may_wakeup(d) && priv->wolopts) {
- ret = bcmgenet_power_down(priv, GENET_POWER_WOL_MAGIC);
- clk_prepare_enable(priv->clk_wol);
- } else if (priv->internal_phy) {
- ret = bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
- }
-
- /* Turn off the clocks */
- clk_disable_unprepare(priv->clk);
-
- return ret;
-}
-
static int bcmgenet_resume(struct device *d)
{
struct net_device *dev = dev_get_drvdata(d);
@@ -3719,6 +3689,36 @@ static int bcmgenet_resume(struct device *d)
clk_disable_unprepare(priv->clk);
return ret;
}
+
+static int bcmgenet_suspend(struct device *d)
+{
+ struct net_device *dev = dev_get_drvdata(d);
+ struct bcmgenet_priv *priv = netdev_priv(dev);
+ int ret = 0;
+
+ if (!netif_running(dev))
+ return 0;
+
+ netif_device_detach(dev);
+
+ bcmgenet_netif_stop(dev);
+
+ if (!device_may_wakeup(d))
+ phy_suspend(dev->phydev);
+
+ /* Prepare the device for Wake-on-LAN and switch to the slow clock */
+ if (device_may_wakeup(d) && priv->wolopts) {
+ ret = bcmgenet_power_down(priv, GENET_POWER_WOL_MAGIC);
+ clk_prepare_enable(priv->clk_wol);
+ } else if (priv->internal_phy) {
+ ret = bcmgenet_power_down(priv, GENET_POWER_PASSIVE);
+ }
+
+ /* Turn off the clocks */
+ clk_disable_unprepare(priv->clk);
+
+ return ret;
+}
#endif /* CONFIG_PM_SLEEP */
static SIMPLE_DEV_PM_OPS(bcmgenet_pm_ops, bcmgenet_suspend, bcmgenet_resume);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 2/2] net: bcmgenet: abort suspend on error
From: Doug Berger @ 2018-11-19 17:56 UTC (permalink / raw)
To: David S. Miller
Cc: Florian Fainelli, netdev, linux-kernel, YueHaibing, Doug Berger
In-Reply-To: <1542650162-9229-1-git-send-email-opendmb@gmail.com>
From: Doug Berger <doug.berger@broadcom.com>
If an error occurs during suspension of the driver the driver should
restore the hardware configuration and return an error to force the
system to resume.
Fixes: 0db55093b566 ("net: bcmgenet: return correct value 'ret' from bcmgenet_power_down")
Signed-off-by: Doug Berger <doug.berger@broadcom.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 +++
drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 01c8499fd87f..983245c0867c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3717,6 +3717,9 @@ static int bcmgenet_suspend(struct device *d)
/* Turn off the clocks */
clk_disable_unprepare(priv->clk);
+ if (ret)
+ bcmgenet_resume(d);
+
return ret;
}
#endif /* CONFIG_PM_SLEEP */
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c b/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
index 2fbd027f0148..57582efa362d 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
@@ -186,6 +186,8 @@ void bcmgenet_wol_power_up_cfg(struct bcmgenet_priv *priv,
}
reg = bcmgenet_umac_readl(priv, UMAC_MPD_CTRL);
+ if (!(reg & MPD_EN))
+ return; /* already powered up so skip the rest */
reg &= ~MPD_EN;
bcmgenet_umac_writel(priv, reg, UMAC_MPD_CTRL);
--
2.7.4
^ permalink raw reply related
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Jason Gunthorpe @ 2018-11-19 18:27 UTC (permalink / raw)
To: Jerome Glisse
Cc: Leon Romanovsky, Kenneth Lee, Tim Sell, linux-doc,
Alexander Shishkin, Zaibo Xu, zhangfei.gao, linuxarm,
haojian.zhuang, Christoph Lameter, Hao Fang, Gavin Schenk,
RDMA mailing list, Zhou Wang, Doug Ledford, Uwe Kleine-König,
David Kershner, Kenneth Lee, Johan Hovold, Cyrille Pitchen
In-Reply-To: <20181119164853.GA4593@redhat.com>
On Mon, Nov 19, 2018 at 11:48:54AM -0500, Jerome Glisse wrote:
> Just to comment on this, any infiniband driver which use umem and do
> not have ODP (here ODP for me means listening to mmu notifier so all
> infiniband driver except mlx5) will be affected by same issue AFAICT.
>
> AFAICT there is no special thing happening after fork() inside any of
> those driver. So if parent create a umem mr before fork() and program
> hardware with it then after fork() the parent might start using new
> page for the umem range while the old memory is use by the child. The
> reverse is also true (parent using old memory and child new memory)
> bottom line you can not predict which memory the child or the parent
> will use for the range after fork().
>
> So no matter what you consider the child or the parent, what the hw
> will use for the mr is unlikely to match what the CPU use for the
> same virtual address. In other word:
>
> Before fork:
> CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
> HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
>
> Case 1:
> CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
> CPU child: virtual addr ptr1 -> physical address = 0xDEAD
> HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
>
> Case 2:
> CPU parent: virtual addr ptr1 -> physical address = 0xBEEF
> CPU child: virtual addr ptr1 -> physical address = 0xCAFE
> HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
IIRC this is solved in IB by automatically calling
madvise(MADV_DONTFORK) before creating the MR.
MADV_DONTFORK
.. This is useful to prevent copy-on-write semantics from changing the
physical location of a page if the parent writes to it after a
fork(2) ..
Jason
^ permalink raw reply
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Jason Gunthorpe @ 2018-11-19 18:49 UTC (permalink / raw)
To: Kenneth Lee
Cc: Leon Romanovsky, Kenneth Lee, Tim Sell, linux-doc,
Alexander Shishkin, Zaibo Xu, zhangfei.gao, linuxarm,
haojian.zhuang, Christoph Lameter, Hao Fang, Gavin Schenk,
RDMA mailing list, Zhou Wang, Doug Ledford, Uwe Kleine-König,
David Kershner, Johan Hovold, Cyrille Pitchen, Sagar Dharia
In-Reply-To: <20181119091405.GE157308@Turing-Arch-b>
On Mon, Nov 19, 2018 at 05:14:05PM +0800, Kenneth Lee wrote:
> If the hardware cannot share page table with the CPU, we then need to have
> some way to change the device page table. This is what happen in ODP. It
> invalidates the page table in device upon mmu_notifier call back. But this cannot
> solve the COW problem: if the user process A share a page P with device, and A
> forks a new process B, and it continue to write to the page. By COW, the
> process B will keep the page P, while A will get a new page P'. But you have
> no way to let the device know it should use P' rather than P.
Is this true? I thought mmu_notifiers covered all these cases.
The mm_notifier for A should fire if B causes the physical address of
A's pages to change via COW.
And this causes the device page tables to re-synchronize.
> In WarpDrive/uacce, we make this simple. If you support IOMMU and it support
> SVM/SVA. Everything will be fine just like ODP implicit mode. And you don't need
> to write any code for that. Because it has been done by IOMMU framework. If it
Looks like the IOMMU code uses mmu_notifier, so it is identical to
IB's ODP. The only difference is that IB tends to have the IOMMU page
table in the device, not in the CPU.
The only case I know if that is different is the new-fangled CAPI
stuff where the IOMMU can directly use the CPU's page table and the
IOMMU page table (in device or CPU) is eliminated.
Anyhow, I don't think a single instance of hardware should justify an
entire new subsystem. Subsystems are hard to make and without multiple
hardware examples there is no way to expect that it would cover any
future use cases.
If all your driver needs is to mmap some PCI bar space, route
interrupts and do DMA mapping then mediated VFIO is probably a good
choice.
If it needs to do a bunch of other stuff, not related to PCI bar
space, interrupts and DMA mapping (ie special code for compression,
crypto, AI, whatever) then you should probably do what Jerome said and
make a drivers/char/hisillicon_foo_bar.c that exposes just what your
hardware does.
If you have networking involved in here then consider RDMA,
particularly if this functionality is already part of the same
hardware that the hns infiniband driver is servicing.
'computational MRs' are a reasonable approach to a side-car offload of
already existing RDMA support.
Jason
^ permalink raw reply
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Jason Gunthorpe @ 2018-11-19 18:53 UTC (permalink / raw)
To: Jerome Glisse
Cc: Leon Romanovsky, Kenneth Lee, Tim Sell, linux-doc,
Alexander Shishkin, Zaibo Xu, zhangfei.gao, linuxarm,
haojian.zhuang, Christoph Lameter, Hao Fang, Gavin Schenk,
RDMA mailing list, Zhou Wang, Doug Ledford, Uwe Kleine-König,
David Kershner, Kenneth Lee, Johan Hovold, Cyrille Pitchen
In-Reply-To: <20181119184215.GB4593@redhat.com>
On Mon, Nov 19, 2018 at 01:42:16PM -0500, Jerome Glisse wrote:
> On Mon, Nov 19, 2018 at 11:27:52AM -0700, Jason Gunthorpe wrote:
> > On Mon, Nov 19, 2018 at 11:48:54AM -0500, Jerome Glisse wrote:
> >
> > > Just to comment on this, any infiniband driver which use umem and do
> > > not have ODP (here ODP for me means listening to mmu notifier so all
> > > infiniband driver except mlx5) will be affected by same issue AFAICT.
> > >
> > > AFAICT there is no special thing happening after fork() inside any of
> > > those driver. So if parent create a umem mr before fork() and program
> > > hardware with it then after fork() the parent might start using new
> > > page for the umem range while the old memory is use by the child. The
> > > reverse is also true (parent using old memory and child new memory)
> > > bottom line you can not predict which memory the child or the parent
> > > will use for the range after fork().
> > >
> > > So no matter what you consider the child or the parent, what the hw
> > > will use for the mr is unlikely to match what the CPU use for the
> > > same virtual address. In other word:
> > >
> > > Before fork:
> > > CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
> > > HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
> > >
> > > Case 1:
> > > CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
> > > CPU child: virtual addr ptr1 -> physical address = 0xDEAD
> > > HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
> > >
> > > Case 2:
> > > CPU parent: virtual addr ptr1 -> physical address = 0xBEEF
> > > CPU child: virtual addr ptr1 -> physical address = 0xCAFE
> > > HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
> >
> > IIRC this is solved in IB by automatically calling
> > madvise(MADV_DONTFORK) before creating the MR.
> >
> > MADV_DONTFORK
> > .. This is useful to prevent copy-on-write semantics from changing the
> > physical location of a page if the parent writes to it after a
> > fork(2) ..
>
> This would work around the issue but this is not transparent ie
> range marked with DONTFORK no longer behave as expected from the
> application point of view.
Do you know what the difference is? The man page really gives no
hint..
Does it sometimes unmap the pages during fork?
I actually wonder if the kernel is a bit broken here, we have the same
problem with O_DIRECT and other stuff, right?
Really, if I have a get_user_pages FOLL_WRITE on a page and we fork,
then shouldn't the COW immediately be broken during the fork?
The kernel can't guarentee that an ongoing DMA will not write to those
pages, and it breaks the fork semantic to write to both processes.
> Also it relies on userspace doing the right thing (which is not
> something i usualy trust :)).
Well, if they do it wrong they get to keep all the broken bits :)
Jason
^ permalink raw reply
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Jerome Glisse @ 2018-11-19 19:17 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Leon Romanovsky, Kenneth Lee, Tim Sell, linux-doc,
Alexander Shishkin, Zaibo Xu, zhangfei.gao, linuxarm,
haojian.zhuang, Christoph Lameter, Hao Fang, Gavin Schenk,
RDMA mailing list, Zhou Wang, Doug Ledford, Uwe Kleine-König,
David Kershner, Kenneth Lee, Johan Hovold, Cyrille Pitchen
In-Reply-To: <20181119185333.GC4890@ziepe.ca>
On Mon, Nov 19, 2018 at 11:53:33AM -0700, Jason Gunthorpe wrote:
> On Mon, Nov 19, 2018 at 01:42:16PM -0500, Jerome Glisse wrote:
> > On Mon, Nov 19, 2018 at 11:27:52AM -0700, Jason Gunthorpe wrote:
> > > On Mon, Nov 19, 2018 at 11:48:54AM -0500, Jerome Glisse wrote:
> > >
> > > > Just to comment on this, any infiniband driver which use umem and do
> > > > not have ODP (here ODP for me means listening to mmu notifier so all
> > > > infiniband driver except mlx5) will be affected by same issue AFAICT.
> > > >
> > > > AFAICT there is no special thing happening after fork() inside any of
> > > > those driver. So if parent create a umem mr before fork() and program
> > > > hardware with it then after fork() the parent might start using new
> > > > page for the umem range while the old memory is use by the child. The
> > > > reverse is also true (parent using old memory and child new memory)
> > > > bottom line you can not predict which memory the child or the parent
> > > > will use for the range after fork().
> > > >
> > > > So no matter what you consider the child or the parent, what the hw
> > > > will use for the mr is unlikely to match what the CPU use for the
> > > > same virtual address. In other word:
> > > >
> > > > Before fork:
> > > > CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
> > > > HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
> > > >
> > > > Case 1:
> > > > CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
> > > > CPU child: virtual addr ptr1 -> physical address = 0xDEAD
> > > > HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
> > > >
> > > > Case 2:
> > > > CPU parent: virtual addr ptr1 -> physical address = 0xBEEF
> > > > CPU child: virtual addr ptr1 -> physical address = 0xCAFE
> > > > HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
> > >
> > > IIRC this is solved in IB by automatically calling
> > > madvise(MADV_DONTFORK) before creating the MR.
> > >
> > > MADV_DONTFORK
> > > .. This is useful to prevent copy-on-write semantics from changing the
> > > physical location of a page if the parent writes to it after a
> > > fork(2) ..
> >
> > This would work around the issue but this is not transparent ie
> > range marked with DONTFORK no longer behave as expected from the
> > application point of view.
>
> Do you know what the difference is? The man page really gives no
> hint..
>
> Does it sometimes unmap the pages during fork?
It is handled in kernel/fork.c look for DONTCOPY, basicaly it just
leave empty page table in the child process so child will have to
fault in new page. This also means that child will get 0 as initial
value for all memory address under DONTCOPY/DONTFORK which breaks
application expectation of what fork() do.
>
> I actually wonder if the kernel is a bit broken here, we have the same
> problem with O_DIRECT and other stuff, right?
No it is not, O_DIRECT is fine. The only corner case i can think
of with O_DIRECT is one thread launching an O_DIRECT that write
to private anonymous memory (other O_DIRECT case do not matter)
while another thread call fork() then what the child get can be
undefined ie either it get the data before the O_DIRECT finish
or it gets the result of the O_DIRECT. But this is realy what
you should expect when doing such thing without synchronization.
So O_DIRECT is fine.
>
> Really, if I have a get_user_pages FOLL_WRITE on a page and we fork,
> then shouldn't the COW immediately be broken during the fork?
>
> The kernel can't guarentee that an ongoing DMA will not write to those
> pages, and it breaks the fork semantic to write to both processes.
Fixing that would incur a high cost: need to grow struct page, need
to copy potentialy gigabyte of memory during fork() ... this would be
a serious performance regression for many folks just to work around an
abuse of device driver. So i don't think anything on that front would
be welcome.
umem without proper ODP and VFIO are the only bad user i know of (for
VFIO you can argue that it is part of the API contract and thus that
it is not an abuse but it is not spell out loud in documentation). I
have been trying to push back on any people trying to push thing that
would make the same mistake or at least making sure they understand
what is happening.
What really need to happen is people fixing their hardware and do the
right thing (good software engineer versus evil hardware engineer ;))
Cheers,
Jérôme
^ permalink raw reply
* Re: [PATCHv2 net] ipvs: call ip_vs_dst_notifier earlier than ipv6_dev_notf
From: Simon Horman @ 2018-11-19 8:55 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Xin Long, Julian Anastasov, network dev, netfilter-devel, davem,
Hans Schillstrom
In-Reply-To: <20181117181457.ncy5lfou3kvij4h3@salvia>
On Sat, Nov 17, 2018 at 07:14:57PM +0100, Pablo Neira Ayuso wrote:
> On Sat, Nov 17, 2018 at 09:19:52PM +0900, Xin Long wrote:
> > On Sat, Nov 17, 2018 at 8:15 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > >
> > > On Fri, Nov 16, 2018 at 06:37:19AM -0800, Simon Horman wrote:
> > > > On Fri, Nov 16, 2018 at 09:10:16AM +0200, Julian Anastasov wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > On Thu, 15 Nov 2018, Xin Long wrote:
> > > > >
> > > > > > ip_vs_dst_event is supposed to clean up all dst used in ipvs'
> > > > > > destinations when a net dev is going down. But it works only
> > > > > > when the dst's dev is the same as the dev from the event.
> > > > > >
> > > > > > Now with the same priority but late registration,
> > > > > > ip_vs_dst_notifier is always called later than ipv6_dev_notf
> > > > > > where the dst's dev is set to lo for NETDEV_DOWN event.
> > > > > >
> > > > > > As the dst's dev lo is not the same as the dev from the event
> > > > > > in ip_vs_dst_event, ip_vs_dst_notifier doesn't actually work.
> > > > > > Also as these dst have to wait for dest_trash_timer to clean
> > > > > > them up. It would cause some non-permanent kernel warnings:
> > > > > >
> > > > > > unregister_netdevice: waiting for br0 to become free. Usage count = 3
> > > > > >
> > > > > > To fix it, call ip_vs_dst_notifier earlier than ipv6_dev_notf
> > > > > > by increasing its priority to ADDRCONF_NOTIFY_PRIORITY + 5.
> > > > > >
> > > > > > Note that for ipv4 route fib_netdev_notifier doesn't set dst's
> > > > > > dev to lo in NETDEV_DOWN event, so this fix is only needed when
> > > > > > IP_VS_IPV6 is defined.
> > > > > >
> > > > > > v1->v2:
> > > > > > - apply it only when CONFIG_IP_VS_IPV6 is defined.
> > > > > >
> > > > > > Fixes: 7a4f0761fce3 ("IPVS: init and cleanup restructuring")
> > > > > > Reported-by: Li Shuang <shuali@redhat.com>
> > > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > >
> > > > > Acked-by: Julian Anastasov <ja@ssi.bg>
> > > >
> > > > Thanks,
> > > >
> > > > Pablo, could you consider this for nf?
> > > >
> > > > Acked-by: Simon Horman <horms@verge.net.au>
> > >
> > > Applied, thanks Simon.
> > Hi Pablo,
> >
> > The one you just applied is the v1, I'm afraid you need
> > to revert and apply the v2, which fixed a build error
> > when IPv6 is disabled.
>
> Fixed, thanks!
Thanks Pablo!
^ permalink raw reply
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Christopher Lameter @ 2018-11-19 19:19 UTC (permalink / raw)
To: Jerome Glisse
Cc: Jason Gunthorpe, Leon Romanovsky, Kenneth Lee, Tim Sell,
linux-doc, Alexander Shishkin, Zaibo Xu, zhangfei.gao, linuxarm,
haojian.zhuang, Hao Fang, Gavin Schenk, RDMA mailing list,
Zhou Wang, Doug Ledford, Uwe Kleine-König, David Kershner,
Kenneth Lee, Johan Hovold, Cyrille Pitchen
In-Reply-To: <20181119184215.GB4593@redhat.com>
On Mon, 19 Nov 2018, Jerome Glisse wrote:
> > IIRC this is solved in IB by automatically calling
> > madvise(MADV_DONTFORK) before creating the MR.
> >
> > MADV_DONTFORK
> > .. This is useful to prevent copy-on-write semantics from changing the
> > physical location of a page if the parent writes to it after a
> > fork(2) ..
>
> This would work around the issue but this is not transparent ie
> range marked with DONTFORK no longer behave as expected from the
> application point of view.
Why would anyone expect a range registered via MR behave as normal? Device
I/O is going on into that range. Memory is already special.
> Also it relies on userspace doing the right thing (which is not
> something i usualy trust :)).
This has been established practice for 15 or so years in a couple of use
cases. Again user space already has to be doing special things in order to
handle RDMA is that area.
^ permalink raw reply
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Jerome Glisse @ 2018-11-19 19:25 UTC (permalink / raw)
To: Christopher Lameter
Cc: Jason Gunthorpe, Leon Romanovsky, Kenneth Lee, Tim Sell,
linux-doc, Alexander Shishkin, Zaibo Xu, zhangfei.gao, linuxarm,
haojian.zhuang, Hao Fang, Gavin Schenk, RDMA mailing list,
Zhou Wang, Doug Ledford, Uwe Kleine-König, David Kershner,
Kenneth Lee, Johan Hovold, Cyrille Pitchen
In-Reply-To: <010001672d69716a-743e5098-eb2f-41ca-bf98-6e17eaaba00f-000000@email.amazonses.com>
On Mon, Nov 19, 2018 at 07:19:04PM +0000, Christopher Lameter wrote:
> On Mon, 19 Nov 2018, Jerome Glisse wrote:
>
> > > IIRC this is solved in IB by automatically calling
> > > madvise(MADV_DONTFORK) before creating the MR.
> > >
> > > MADV_DONTFORK
> > > .. This is useful to prevent copy-on-write semantics from changing the
> > > physical location of a page if the parent writes to it after a
> > > fork(2) ..
> >
> > This would work around the issue but this is not transparent ie
> > range marked with DONTFORK no longer behave as expected from the
> > application point of view.
>
> Why would anyone expect a range registered via MR behave as normal? Device
> I/O is going on into that range. Memory is already special.
>
>
> > Also it relies on userspace doing the right thing (which is not
> > something i usualy trust :)).
>
> This has been established practice for 15 or so years in a couple of use
> cases. Again user space already has to be doing special things in order to
> handle RDMA is that area.
Yes RDMA as an existing historical track record and thus people
should now be aware of its limitation. What i am fighting against
is new addition to kernel that pretend to do SVA (share virtual
address) while their hardware is not realy doing SVA. SVA with
IOMMU and ATS/PASID is fine, SVA in software with device driver
that abide to mmu notifier is fine. Anything else is not.
So i am just worrying about new user and making sure they under-
stand what is happening and not sell to their user something
false.
Cheers,
Jérôme
^ permalink raw reply
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Jason Gunthorpe @ 2018-11-19 19:27 UTC (permalink / raw)
To: Jerome Glisse
Cc: Leon Romanovsky, Kenneth Lee, Tim Sell, linux-doc,
Alexander Shishkin, Zaibo Xu, zhangfei.gao, linuxarm,
haojian.zhuang, Christoph Lameter, Hao Fang, Gavin Schenk,
RDMA mailing list, Zhou Wang, Doug Ledford, Uwe Kleine-König,
David Kershner, Kenneth Lee, Johan Hovold, Cyrille Pitchen
In-Reply-To: <20181119191721.GC4593@redhat.com>
On Mon, Nov 19, 2018 at 02:17:21PM -0500, Jerome Glisse wrote:
> On Mon, Nov 19, 2018 at 11:53:33AM -0700, Jason Gunthorpe wrote:
> > On Mon, Nov 19, 2018 at 01:42:16PM -0500, Jerome Glisse wrote:
> > > On Mon, Nov 19, 2018 at 11:27:52AM -0700, Jason Gunthorpe wrote:
> > > > On Mon, Nov 19, 2018 at 11:48:54AM -0500, Jerome Glisse wrote:
> > > >
> > > > > Just to comment on this, any infiniband driver which use umem and do
> > > > > not have ODP (here ODP for me means listening to mmu notifier so all
> > > > > infiniband driver except mlx5) will be affected by same issue AFAICT.
> > > > >
> > > > > AFAICT there is no special thing happening after fork() inside any of
> > > > > those driver. So if parent create a umem mr before fork() and program
> > > > > hardware with it then after fork() the parent might start using new
> > > > > page for the umem range while the old memory is use by the child. The
> > > > > reverse is also true (parent using old memory and child new memory)
> > > > > bottom line you can not predict which memory the child or the parent
> > > > > will use for the range after fork().
> > > > >
> > > > > So no matter what you consider the child or the parent, what the hw
> > > > > will use for the mr is unlikely to match what the CPU use for the
> > > > > same virtual address. In other word:
> > > > >
> > > > > Before fork:
> > > > > CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
> > > > > HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
> > > > >
> > > > > Case 1:
> > > > > CPU parent: virtual addr ptr1 -> physical address = 0xCAFE
> > > > > CPU child: virtual addr ptr1 -> physical address = 0xDEAD
> > > > > HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
> > > > >
> > > > > Case 2:
> > > > > CPU parent: virtual addr ptr1 -> physical address = 0xBEEF
> > > > > CPU child: virtual addr ptr1 -> physical address = 0xCAFE
> > > > > HARDWARE: virtual addr ptr1 -> physical address = 0xCAFE
> > > >
> > > > IIRC this is solved in IB by automatically calling
> > > > madvise(MADV_DONTFORK) before creating the MR.
> > > >
> > > > MADV_DONTFORK
> > > > .. This is useful to prevent copy-on-write semantics from changing the
> > > > physical location of a page if the parent writes to it after a
> > > > fork(2) ..
> > >
> > > This would work around the issue but this is not transparent ie
> > > range marked with DONTFORK no longer behave as expected from the
> > > application point of view.
> >
> > Do you know what the difference is? The man page really gives no
> > hint..
> >
> > Does it sometimes unmap the pages during fork?
>
> It is handled in kernel/fork.c look for DONTCOPY, basicaly it just
> leave empty page table in the child process so child will have to
> fault in new page. This also means that child will get 0 as initial
> value for all memory address under DONTCOPY/DONTFORK which breaks
> application expectation of what fork() do.
Hum, I wonder why this API was selected then..
> > I actually wonder if the kernel is a bit broken here, we have the same
> > problem with O_DIRECT and other stuff, right?
>
> No it is not, O_DIRECT is fine. The only corner case i can think
> of with O_DIRECT is one thread launching an O_DIRECT that write
> to private anonymous memory (other O_DIRECT case do not matter)
> while another thread call fork() then what the child get can be
> undefined ie either it get the data before the O_DIRECT finish
> or it gets the result of the O_DIRECT. But this is realy what
> you should expect when doing such thing without synchronization.
>
> So O_DIRECT is fine.
?? How can O_DIRECT be fine but RDMA not? They use exactly the same
get_user_pages flow, right? Can we do what O_DIRECT does in RDMA and
be fine too?
AFAIK the only difference is the length of the race window. You'd have
to fork and fault during the shorter time O_DIRECT has get_user_pages
open.
> > Really, if I have a get_user_pages FOLL_WRITE on a page and we fork,
> > then shouldn't the COW immediately be broken during the fork?
> >
> > The kernel can't guarentee that an ongoing DMA will not write to those
> > pages, and it breaks the fork semantic to write to both processes.
>
> Fixing that would incur a high cost: need to grow struct page, need
> to copy potentialy gigabyte of memory during fork() ... this would be
> a serious performance regression for many folks just to work around an
> abuse of device driver. So i don't think anything on that front would
> be welcome.
Why? Keep track in each mm if there are any active get_user_pages
FOLL_WRITE pages in the mm, if yes then sweep the VMAs and fix the
issue for the FOLL_WRITE pages.
John is already working on being able to detect pages under GUP, so it
seems like a small step..
Since nearly all cases of fork don't have a GUP FOLL_WRITE active
there would be no performance hit.
> umem without proper ODP and VFIO are the only bad user i know of (for
> VFIO you can argue that it is part of the API contract and thus that
> it is not an abuse but it is not spell out loud in documentation). I
> have been trying to push back on any people trying to push thing that
> would make the same mistake or at least making sure they understand
> what is happening.
It is something we have to live with and support for the foreseeable
future.
> What really need to happen is people fixing their hardware and do the
> right thing (good software engineer versus evil hardware engineer ;))
Even ODP is no pancea, there are performance problems. What we really
need is CAPI like stuff, so you will tell Intel to redesign the CPU??
:)
Jason
^ permalink raw reply
* Re: [PATCH v2 00/25] at24: remove
From: Boris Brezillon @ 2018-11-19 9:08 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Sekhar Nori, Kevin Hilman, Russell King, Arnd Bergmann,
Greg Kroah-Hartman, David Woodhouse, Brian Norris, Marek Vasut,
Richard Weinberger, Nicolas Ferre, David S . Miller,
Grygorii Strashko, Srinivas Kandagatla, Andrew Lunn,
Florian Fainelli, Rob Herring, Frank Rowand, Wolfram Sang,
devicetree, netdev
In-Reply-To: <CAMRc=MfXWW9siNZNkocxmTU7mnoQO0KpeHQ3JmLcEfNscqCbDQ@mail.gmail.com>
On Mon, 19 Nov 2018 09:58:46 +0100
Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> niedz., 18 lis 2018 o 17:03 Boris Brezillon
> <boris.brezillon@bootlin.com> napisał(a):
> >
> > On Tue, 13 Nov 2018 15:01:08 +0100
> > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > > As far as merging of this series goes: I'd like to avoid dragging it over
> > > four releases. The series is logically split into five groups:
> > >
> > > patches 1-2: nvmem and mtd changes
> > > patches 3-9: davinci arch-specific changes
> > > patches 10-13: networking changes
> > > patches 14-24: davinci specific again
> > > patch 25: final at24 change
> > >
> > > With that I believe we can do the following: Greg KH could pick up the
> > > first two patches into his char-misc tree.
> >
> > The char-misc tree? Why not the MTD or NVMEM tree?
> >
>
> There is no NVMEM tree - Srinivas sends his patches to Greg to be
> applied to char-misc.
Okay, I didn't know that.
>
> The second patch depends on the first one so in order to avoid more
> merging problems I suggested that the MTD patch go through char-misc
> as well. If you see a better solution, please let me know.
No that's fine, as long as I have an immutable tag/branch I can pull if
needed.
^ permalink raw reply
* [PATCH] ipv4: allow newer DSCP values in ip rules
From: Pavel Balaev @ 2018-11-19 9:00 UTC (permalink / raw)
To: netdev
This patch adds ability to set newer RFC2597 DSCP values in ip rules.
Values are presents in /etc/iproute2/rt_dsfield:
0x28 AF11
0x30 AF12
0x38 AF13
0x48 AF21
...
If one tries to apply this values with iproure2 (ip rule add tos 0x28 ...)
he will get error from netlink: "Invalid tos" for this moment.
After this patch this one can use it:
$ ip r add default via 192.168.0.6 table test
$ ip ru add tos 0x80 table test
$ ip ru
0: from all lookup local
32764: from all tos CS4 lookup test
32766: from all lookup main
32767: from all lookup default
$ ip r get fibmatch 8.8.8.9 tos 0x80
default tos CS4 via 192.168.0.6 dev lan table test
Signed-off-by: Pavel Balaev <mail@void.so>
---
net/ipv4/fib_rules.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index f8eb78d0..7a6c5bfe 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -220,7 +220,7 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
int err = -EINVAL;
struct fib4_rule *rule4 = (struct fib4_rule *) rule;
- if (frh->tos & ~IPTOS_TOS_MASK) {
+ if (frh->tos & ~(IPTOS_TOS_MASK | IPTOS_PREC_MASK)) {
NL_SET_ERR_MSG(extack, "Invalid tos");
goto errout;
}
--
2.18.1
^ permalink raw reply related
* RE: [PATCH 00/10] add flow_rule infrastructure
From: Chopra, Manish @ 2018-11-19 9:19 UTC (permalink / raw)
To: Pablo Neira Ayuso, netdev@vger.kernel.org
Cc: davem@davemloft.net, thomas.lendacky@amd.com,
f.fainelli@gmail.com, Elior, Ariel, michael.chan@broadcom.com,
santosh@chelsio.com, madalin.bucur@nxp.com,
yisen.zhuang@huawei.com, salil.mehta@huawei.com,
jeffrey.t.kirsher@intel.com, tariqt@mellanox.com,
saeedm@mellanox.com, jiri@mellanox.com, idosch@mellanox.com,
jakub.kicinski@netronome.com,
"peppe.cavallaro@st.com" <peppe.cav
In-Reply-To: <20181116014139.25438-1-pablo@netfilter.org>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Pablo Neira Ayuso
> Sent: Friday, November 16, 2018 7:11 AM
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net; thomas.lendacky@amd.com;
> f.fainelli@gmail.com; Elior, Ariel <Ariel.Elior@cavium.com>;
> michael.chan@broadcom.com; santosh@chelsio.com;
> madalin.bucur@nxp.com; yisen.zhuang@huawei.com;
> salil.mehta@huawei.com; jeffrey.t.kirsher@intel.com; tariqt@mellanox.com;
> saeedm@mellanox.com; jiri@mellanox.com; idosch@mellanox.com;
> jakub.kicinski@netronome.com; peppe.cavallaro@st.com;
> grygorii.strashko@ti.com; andrew@lunn.ch;
> vivien.didelot@savoirfairelinux.com; alexandre.torgue@st.com;
> joabreu@synopsys.com; linux-net-drivers@solarflare.com;
> ganeshgr@chelsio.com
> Subject: [PATCH 00/10] add flow_rule infrastructure
>
> External Email
>
> This patchset introduces a kernel intermediate representation (IR) to express
> ACL hardware offloads, this is heavily based on the existing flow dissector
> infrastructure and the TC actions. This IR can be used by different frontend
> ACL interfaces such as ethtool_rxnfc and tc to represent ACL hardware
> offloads. Main goal is to simplify the development of ACL hardware offloads
> for the existing frontend interfaces, the idea is that driver developers do not
> need to add one specific parser for each ACL frontend, instead each frontend
> can just generate this flow_rule IR and pass it to drivers to populate the
> hardware IR.
>
> . ethtool_rxnfc tc
> | (ioctl) (netlink)
> | | | translate native
> Frontend | | | interface representation
> | | | to flow_rule IR
> | | |
> . \/ \/
> . flow_rule IR
> | |
> Drivers | | parsing of flow_rule IR
> | | to populate hardware IR
> | \/
> . hardware IR (driver)
>
> For design and implementation details, please have a look at:
>
> https://lwn.net/Articles/766695/
>
> As an example, with this patchset, it should be possible to simplify the
> existing net/qede driver which already has two parsers to populate the
> hardware IR, one for ethtool_rxnfc interface and another for tc.
>
> This batch is composed of 10 patches:
>
> Patch #1 adds the flow_match structure, this includes the
> flow_rule_match_key() interface to check for existing selectors
> that are in used in the rule and the flow_rule_match_*()
> functions to fetch the selector value and the mask. This
> also introduces the initial flow_rule structure skeleton to
> avoid a follow up patch that would update the same LoCs.
>
> Patch #2 makes changes to packet edit parser of mlx5e driver, to prepare
> introduction of the new flow_action to mangle packets.
>
> Patch #3 Introduce flow_action infrastructure. This infrastructure is
> based on the TC actions. Patch #8 extends it so it also
> supports two new actions that are only available through the
> ethtool_rxnfc interface.
>
> Patch #4 Add function to translate TC action to flow_action from
> cls_flower.
>
> Patch #5 Add infrastructure to fetch statistics into container structure
> and synchronize them to TC actions from cls_flower. Another
> preparation patch before patch #7, so we can stop exposing the
> TC action native layout to the drivers.
>
> Patch #6 Use flow_action infrastructure from drivers.
>
> Patch #7 Do not expose TC actions to drivers anymore, now that drivers
> have been converted to use the flow_action infrastructure after
> patch #5.
>
> Patch #8 Support to wake-up-on-lan and queue actions for the flow_action
> infrastructure, two actions supported by NICs. This is used by
> the ethtool_rx_flow interface.
>
> Patch #9 Add a function to translate from ethtool_rx_flow_spec structure
> to the flow_action structure. This is a simple enough for its
> first client: the ethtool_rxnfc interface in the bcm_sf2 driver.
>
> Patch #10 Update bcm_sf2 to use this new translator function and
> update codebase to configure hardware IR using the
> flow_action representation. This will allow later development
> of cls_flower using the same codebase from the driver.
>
> This patchset has passed here functional tests of the codepath that generates
> the flow_rule structure and the functions to implement the parsers that
> populate the hardware IR.
>
> Thanks.
>
> Pablo Neira Ayuso (10):
> flow_dissector: add flow_rule and flow_match structures and use them
> net/mlx5e: support for two independent packet edit actions
> flow_dissector: add flow action infrastructure
> cls_api: add translator to flow_action representation
> cls_flower: add statistics retrieval infrastructure and use it
> drivers: net: use flow action infrastructure
> cls_flower: don't expose TC actions to drivers anymore
> flow_dissector: add wake-up-on-lan and queue to flow_action
> flow_dissector: add basic ethtool_rx_flow_spec to flow_rule structure
> translator
> dsa: bcm_sf2: use flow_rule infrastructure
>
> drivers/net/dsa/bcm_sf2_cfp.c | 103 ++-
> drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 252 +++----
> .../net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 450 ++++++-------
> drivers/net/ethernet/intel/i40e/i40e_main.c | 178 ++---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 195 +++---
> drivers/net/ethernet/intel/igb/igb_main.c | 64 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 743 ++++++++++--------
> ---
> drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 2 +-
> .../net/ethernet/mellanox/mlxsw/spectrum_flower.c | 259 ++++---
> drivers/net/ethernet/netronome/nfp/flower/action.c | 196 +++---
> drivers/net/ethernet/netronome/nfp/flower/match.c | 416 ++++++------
> .../net/ethernet/netronome/nfp/flower/offload.c | 151 ++---
> drivers/net/ethernet/qlogic/qede/qede_filter.c | 93 ++-
> include/net/flow_dissector.h | 185 +++++
> include/net/pkt_cls.h | 23 +-
> net/core/flow_dissector.c | 341 ++++++++++
> net/sched/cls_api.c | 113 ++++
> net/sched/cls_flower.c | 42 +-
> 18 files changed, 2120 insertions(+), 1686 deletions(-)
>
> --
> 2.11.0
Today, we have different kernel interfaces(aRFS/ethtool/tc) to do flow offloading.
One big question arises in my mind that why we need these all different interfaces to do flow offloading and
why can't they just be combined into a single kernel interface somehow.
Having these flow offloading done through various hooks makes driver implementation really complex.
Specially when looking at ethtool/tc flows most of the use cases seems common to me -
1. Drop action can be done on tuples from both tc/ethtool flows.
2. Flow redirection is done from both ethtool/tc
(May be with just slight difference ? as Ethtool does steering on PF and VF/VF-queues
but tc doesn't have redirection on queues which can be extended over tc)
3. Could be more such common cases which could be the reason to combine them through single interface ?
Thanks !!
^ permalink raw reply
* Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
From: Jose Abreu @ 2018-11-19 9:20 UTC (permalink / raw)
To: Pablo Neira Ayuso, netdev
Cc: davem, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, jose.abreu, linux-net-drivers, ganeshgr,
ogerlitz
In-Reply-To: <20181119001519.12124-1-pablo@netfilter.org>
Hello Pablo,
On 19-11-2018 00:15, Pablo Neira Ayuso wrote:
> Hi,
>
> This patchset introduces a kernel intermediate representation (IR) to
> express ACL hardware offloads, as already described in previous RFC and
> v1 patchset [1] [2]. The idea is to normalize the frontend U/APIs to use
> the flow dissectors and the flow actions so drivers can reuse the
> existing TC offload driver codebase - that has been converted to use the
> flow_rule infrastructure.
>
> After this patch, as Or previously described, there is one extra layer:
>
> kernel frontend U/API X --> kernel parser Y --> IR --> driver --> HW API
> kernel frontend U/API Z --> kernel parser W --> IR --> driver --> HW API
>
> However, cost of this layer is very small, adding 1 million rules via
> tc -batch, perf shows:
>
> 0.06% tc [kernel.vmlinux] [k] tc_setup_flow_action
>
> at position 187 in the call graph, far from the top ten. The flow_match
> representation uses the flow dissector infrastructure, just like
> cls_flower, therefore, there is no need for conversion of the rule match
> side.
>
> The flow_action representation is very similar to the TC action plus
> this includes wake-up-on-lan and queue to CPU actions that are needed
> for the ethtool_rx_flow_spec interface in the bcm_sf2 driver, that is
> converted in this patchset to use it. It is now possible to add tc
> cls_flower support for bcm_sf2 and reuse the existing parser that was
> originally designed for the ethtool_rx_flow_spec interface.
>
> As requested, this new patchset also converts qlogic/qede to use this
> new infrastructure (see patch 12/12). This driver currently has two
> parsers, one for ethtool_rx_flow_spec and another for tc cls_flower.
> This driver supports for simple 5-tuple matching and available actions
> are packet drop and queue. This patch updates the driver code to use one
> single parser to populate HW IR.
>
> Thanks.
>
> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_766695_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY&m=tB4p4lWxJv3Np8Tg05scy415_MEU7RQO3Q6KGdcTMCg&s=EUCsiqp58Et5K2u9hHsxXyF6ep1yfhnASEBdXur33oQ&e=
> [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dnetdev-26m-3D154233253114506-26w-3D2&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY&m=tB4p4lWxJv3Np8Tg05scy415_MEU7RQO3Q6KGdcTMCg&s=1XOZeTYyELUCyM8yS76_bRQOVGOy19pnV9cH937FjkY&e=
>
> Pablo Neira Ayuso (12):
> flow_dissector: add flow_rule and flow_match structures and use them
> net/mlx5e: support for two independent packet edit actions
> flow_dissector: add flow action infrastructure
> cls_api: add translator to flow_action representation
> cls_flower: add statistics retrieval infrastructure and use it
> drivers: net: use flow action infrastructure
> cls_flower: don't expose TC actions to drivers anymore
> flow_dissector: add wake-up-on-lan and queue to flow_action
> flow_dissector: add basic ethtool_rx_flow_spec to flow_rule structure
> translator
> dsa: bcm_sf2: use flow_rule infrastructure
> qede: place ethtool_rx_flow_spec after code after TC flower codebase
> qede: use ethtool_rx_flow_rule() to remove duplicated parser code
>
> drivers/net/dsa/bcm_sf2_cfp.c | 108 +--
> drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 252 +++----
> .../net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 450 ++++++-------
> drivers/net/ethernet/intel/i40e/i40e_main.c | 178 ++---
> drivers/net/ethernet/intel/iavf/iavf_main.c | 195 +++---
> drivers/net/ethernet/intel/igb/igb_main.c | 64 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 743 ++++++++++-----------
> drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 2 +-
> .../net/ethernet/mellanox/mlxsw/spectrum_flower.c | 259 ++++---
> drivers/net/ethernet/netronome/nfp/flower/action.c | 196 +++---
> drivers/net/ethernet/netronome/nfp/flower/match.c | 417 ++++++------
> .../net/ethernet/netronome/nfp/flower/offload.c | 151 ++---
> drivers/net/ethernet/qlogic/qede/qede_filter.c | 537 ++++++---------
> include/net/flow_dissector.h | 185 +++++
> include/net/pkt_cls.h | 29 +-
> net/core/flow_dissector.c | 341 ++++++++++
> net/sched/cls_api.c | 113 ++++
> net/sched/cls_flower.c | 42 +-
> 18 files changed, 2279 insertions(+), 1983 deletions(-)
>
Although I was cc'ed in the thread I'm not seeing stmmac driver
in this conversion. Can you please add it ?
Thanks and Best Regards,
Jose Miguel Abreu
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox