Skip to contentSkip to conversation

IB/ipoib: print only once when doesn't support IB_QP_CREATE_USE_GFP_NOIO

PATCH In review Leon Romanovsky 2 participants 5 messages Nov 1, 2016 – Nov 2, 2016 4 messages not in the archive InfiniBand

lore
Share & tools
lore

Leon Romanovsky

author
#0 lore raw

PATCH 1/1 IB/ipoib: print only once when doesn't support IB_QP_CREATE_USE_GFP_NOIO

From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>

Currently when the card doesn't support IB_QP_CREATE_USE_GFP_NOIO it warns on every QP creation, It becomes worse when driver works in connected mode we will see one print on each new connection, instead do it once.

Fixes 09b93088d7 ('Add a QP creation flag to use GFP_NOIO allocations')
Signed-off-by Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/ulp/ipoib/ipoib_cm.c +2 -2
index 4ad297d..917393b 100644
@@ -1053,8 +1053,8 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
 
 	tx_qp = ib_create_qp(priv->pd, &attr);
 	if (PTR_ERR(tx_qp) == -EINVAL) {
-		ipoib_warn(priv, "can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
-			   priv->ca->name);
+		pr_warn_once("can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
+			     priv->ca->name);
 		attr.create_flags &= ~IB_QP_CREATE_USE_GFP_NOIO;
 		tx_qp = ib_create_qp(priv->pd, &attr);
 	}
Signature · 6 lines
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1478003653-16248-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#2 lore raw
Quoted 18 lines · Leon Romanovsky

From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>

Currently when the card doesn't support IB_QP_CREATE_USE_GFP_NOIO it warns on every QP creation, It becomes worse when driver works in connected mode we will see one print on each new connection, instead do it once.

Fixes 09b93088d7 ('Add a QP creation flag to use GFP_NOIO allocations')
Signed-off-by Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/ulp/ipoib/ipoib_cm.c +0 -0
index 4ad297d..917393b 100644

drivers/infiniband/ulp/ipoib/ipoib_cm.c static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_ · line 1057

Quoted 7 lines
drivers/infiniband/ulp/ipoib/ipoib_cm.c +2 -2
@@ -1053,8 +1053,8 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
 
 	tx_qp = ib_create_qp(priv->pd, &attr);
 	if (PTR_ERR(tx_qp) == -EINVAL) {
-		ipoib_warn(priv, "can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
-			   priv->ca->name);
+		pr_warn_once("can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
+			     priv->ca->name);

But it will still re-print it for different device, right?

Quoted 10 lines
drivers/infiniband/ulp/ipoib/ipoib_cm.c +0 -0
 		attr.create_flags &= ~IB_QP_CREATE_USE_GFP_NOIO;
 		tx_qp = ib_create_qp(priv->pd, &attr);
 	}
Signature · 6 lines
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#4 lore raw
Quoted 20 lines · Yuval Shaia
Quoted 19 lines · Leon Romanovsky

From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>

Currently when the card doesn't support IB_QP_CREATE_USE_GFP_NOIO it warns on every QP creation, It becomes worse when driver works in connected mode we will see one print on each new connection, instead do it once.

Fixes 09b93088d7 ('Add a QP creation flag to use GFP_NOIO allocations')
Signed-off-by Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/ulp/ipoib/ipoib_cm.c +0 -0
index 4ad297d..917393b 100644
@@ -1053,8 +1053,8 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
Quoted 8 lines

drivers/infiniband/ulp/ipoib/ipoib_cm.c static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_ · line 1057

Quoted 6 lines
drivers/infiniband/ulp/ipoib/ipoib_cm.c +2 -2
 	tx_qp = ib_create_qp(priv->pd, &attr);
 	if (PTR_ERR(tx_qp) == -EINVAL) {
-		ipoib_warn(priv, "can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
-			   priv->ca->name);
+		pr_warn_once("can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
+			     priv->ca->name);

But it will still re-print it for different device, right?

Good question,

pr_warn_once is defined as alias to printk_once [1]. That printk_once is macro too [2] which will define local static read_once variable.

[1] http://lxr.free-electrons.com/source/include/linux/printk.h#L359
[2] http://lxr.free-electrons.com/source/include/linux/printk.h#L322

Quoted 11 lines
Quoted 11 lines
drivers/infiniband/ulp/ipoib/ipoib_cm.c +0 -1
 		attr.create_flags &= ~IB_QP_CREATE_USE_GFP_NOIO;
 		tx_qp = ib_create_qp(priv->pd, &attr);
 	}
--

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20161101141454.GD3617-2ukJVAZIZ/[email protected]

#6 lore raw
Quoted 30 lines · Leon Romanovsky
Quoted 20 lines · Yuval Shaia
Quoted 19 lines · Leon Romanovsky

From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>

Currently when the card doesn't support IB_QP_CREATE_USE_GFP_NOIO it warns on every QP creation, It becomes worse when driver works in connected mode we will see one print on each new connection, instead do it once.

Fixes 09b93088d7 ('Add a QP creation flag to use GFP_NOIO allocations')
Signed-off-by Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/ulp/ipoib/ipoib_cm.c +0 -0
index 4ad297d..917393b 100644
@@ -1053,8 +1053,8 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
Quoted 8 lines

drivers/infiniband/ulp/ipoib/ipoib_cm.c static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_ · line 1057

Quoted 6 lines
drivers/infiniband/ulp/ipoib/ipoib_cm.c +2 -2
 	tx_qp = ib_create_qp(priv->pd, &attr);
 	if (PTR_ERR(tx_qp) == -EINVAL) {
-		ipoib_warn(priv, "can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
-			   priv->ca->name);
+		pr_warn_once("can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
+			     priv->ca->name);

But it will still re-print it for different device, right?

Quoted 7 lines

Good question,

pr_warn_once is defined as alias to printk_once [1]. That printk_once is macro too [2] which will define local static read_once variable.

[1] http://lxr.free-electrons.com/source/include/linux/printk.h#L359
[2] http://lxr.free-electrons.com/source/include/linux/printk.h#L322

If only one HCA model is installed on the system then it should be fine, but wonder if more then one, would we like to see the warning again? i think yes.

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/infiniband/ulp/ipoib/ipoib_cm.c +0 -1
 		attr.create_flags &= ~IB_QP_CREATE_USE_GFP_NOIO;
 		tx_qp = ib_create_qp(priv->pd, &attr);
 	}
--

2.7.4

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#8 lore raw
Quoted 36 lines · Yuval Shaia
Quoted 35 lines · Leon Romanovsky
Quoted 20 lines · Yuval Shaia
Quoted 19 lines · Leon Romanovsky

From: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>

Currently when the card doesn't support IB_QP_CREATE_USE_GFP_NOIO it warns on every QP creation, It becomes worse when driver works in connected mode we will see one print on each new connection, instead do it once.

Fixes 09b93088d7 ('Add a QP creation flag to use GFP_NOIO allocations')
Signed-off-by Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/ulp/ipoib/ipoib_cm.c +0 -0
index 4ad297d..917393b 100644
@@ -1053,8 +1053,8 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
Quoted 8 lines

drivers/infiniband/ulp/ipoib/ipoib_cm.c static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_ · line 1057

Quoted 6 lines
drivers/infiniband/ulp/ipoib/ipoib_cm.c +2 -2
 	tx_qp = ib_create_qp(priv->pd, &attr);
 	if (PTR_ERR(tx_qp) == -EINVAL) {
-		ipoib_warn(priv, "can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
-			   priv->ca->name);
+		pr_warn_once("can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
+			     priv->ca->name);

But it will still re-print it for different device, right?

Good question,

pr_warn_once is defined as alias to printk_once [1]. That printk_once is macro too [2] which will define local static read_once variable.

Quoted 6 lines
Quoted 2 lines

[1] http://lxr.free-electrons.com/source/include/linux/printk.h#L359
[2] http://lxr.free-electrons.com/source/include/linux/printk.h#L322

If only one HCA model is installed on the system then it should be fine, but wonder if more then one, would we like to see the warning again? i think yes.

It is correct and valid question and not for the multiple devices only. Additional thing which was missed in such a trivial patch is the lost of context which is printed by ipoib_warn and doesn't print in the case of pr_warn.

Thanks for pointing and investing time in the review.

Doug,
Please drop it. We will respin it.

Quoted 15 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
drivers/infiniband/ulp/ipoib/ipoib_cm.c +0 -1
 		attr.create_flags &= ~IB_QP_CREATE_USE_GFP_NOIO;
 		tx_qp = ib_create_qp(priv->pd, &attr);
 	}
--

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html