diff mbox

[rdma-next,V1,1/4] IB/IPoIB: Convert IPoIB to memalloc_noio_* calls

Message ID 20170523113816.32657-2-leon@kernel.org (mailing list archive)
State Accepted
Headers show

Commit Message

Leon Romanovsky May 23, 2017, 11:38 a.m. UTC 
From: Leon Romanovsky <leonro@mellanox.com>

Commit 21caf2fc1931 ("mm: teach mm by current context info to not do I/O
during memory allocation") added the memalloc_noio_(save|restore) functions
to enable people to modify the MM behavior by disabling I/O during memory
allocation. This was further extended in Fixes: 934f3072c17c ("mm: clear
__GFP_FS when PF_MEMALLOC_NOIO is set"). memalloc_noio_* functions prevent
allocation paths recursing back into the filesystem without explicitly
changing the flags for every allocation site.

However the IPoIB hasn't been keeping up with the changes and missed
completely these memalloc_noio_* calls. This led to update of
allocation site with special QP creation flag, see commit 09b93088d750
("IB: Add a QP creation flag to use GFP_NOIO allocations"), while this
flag is supported by small number of drivers in IB stack.

Let's change it by updating to memalloc_noio_* calls and allow
for every driver underneath enjoy NOIO allocations.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
---
 drivers/infiniband/ulp/ipoib/ipoib_cm.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

Dennis Dalessandro May 29, 2017, 4:44 p.m. UTC | #1 
On 5/23/2017 7:38 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Commit 21caf2fc1931 ("mm: teach mm by current context info to not do I/O
> during memory allocation") added the memalloc_noio_(save|restore) functions
> to enable people to modify the MM behavior by disabling I/O during memory
> allocation. This was further extended in Fixes: 934f3072c17c ("mm: clear
> __GFP_FS when PF_MEMALLOC_NOIO is set"). memalloc_noio_* functions prevent
> allocation paths recursing back into the filesystem without explicitly
> changing the flags for every allocation site.
> 
> However the IPoIB hasn't been keeping up with the changes and missed
> completely these memalloc_noio_* calls. This led to update of
> allocation site with special QP creation flag, see commit 09b93088d750
> ("IB: Add a QP creation flag to use GFP_NOIO allocations"), while this
> flag is supported by small number of drivers in IB stack.
> 
> Let's change it by updating to memalloc_noio_* calls and allow
> for every driver underneath enjoy NOIO allocations.
> 
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> Signed-off-by: Leon Romanovsky <leon@kernel.org>

Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>

> @@ -1131,10 +1127,11 @@ static int ipoib_cm_tx_init(struct ipoib_cm_tx *p, u32 qpn,
>   			    struct sa_path_rec *pathrec)
>   {
>   	struct ipoib_dev_priv *priv = ipoib_priv(p->dev);
> +	unsigned int noio_flag;

>   	int ret;
>   
> -	p->tx_ring = __vmalloc(ipoib_sendq_size * sizeof *p->tx_ring,
> -			       GFP_NOIO, PAGE_KERNEL);
> +	noio_flag = memalloc_noio_save();

NIT: Maybe just call this "flags" since there could be other flags in 
current->flags when calling memalloc_noio_save()

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky May 29, 2017, 5:01 p.m. UTC | #2 
On Mon, May 29, 2017 at 12:44:46PM -0400, Dennis Dalessandro wrote:
> On 5/23/2017 7:38 AM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Commit 21caf2fc1931 ("mm: teach mm by current context info to not do I/O
> > during memory allocation") added the memalloc_noio_(save|restore) functions
> > to enable people to modify the MM behavior by disabling I/O during memory
> > allocation. This was further extended in Fixes: 934f3072c17c ("mm: clear
> > __GFP_FS when PF_MEMALLOC_NOIO is set"). memalloc_noio_* functions prevent
> > allocation paths recursing back into the filesystem without explicitly
> > changing the flags for every allocation site.
> >
> > However the IPoIB hasn't been keeping up with the changes and missed
> > completely these memalloc_noio_* calls. This led to update of
> > allocation site with special QP creation flag, see commit 09b93088d750
> > ("IB: Add a QP creation flag to use GFP_NOIO allocations"), while this
> > flag is supported by small number of drivers in IB stack.
> >
> > Let's change it by updating to memalloc_noio_* calls and allow
> > for every driver underneath enjoy NOIO allocations.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leon@kernel.org>
>
> Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
>
> > @@ -1131,10 +1127,11 @@ static int ipoib_cm_tx_init(struct ipoib_cm_tx *p, u32 qpn,
> >   			    struct sa_path_rec *pathrec)
> >   {
> >   	struct ipoib_dev_priv *priv = ipoib_priv(p->dev);
> > +	unsigned int noio_flag;
>
> >   	int ret;
> > -	p->tx_ring = __vmalloc(ipoib_sendq_size * sizeof *p->tx_ring,
> > -			       GFP_NOIO, PAGE_KERNEL);
> > +	noio_flag = memalloc_noio_save();
>
> NIT: Maybe just call this "flags" since there could be other flags in
> current->flags when calling memalloc_noio_save()

I didn't want to be different from other callers :)
➜  linux-rdma git:(master) ✗ git grep memalloc_noio_save
drivers/base/power/runtime.c:           noio_flag = memalloc_noio_save();
drivers/md/dm-bufio.c:          noio_flag = memalloc_noio_save();
drivers/md/dm-ioctl.c:  noio_flag = memalloc_noio_save();
drivers/usb/core/hub.c: noio_flag = memalloc_noio_save();
fs/ocfs2/cluster/tcp.c: noio_flag = memalloc_noio_save();
fs/ocfs2/cluster/tcp.c: noio_flag = memalloc_noio_save();
include/linux/sched/mm.h:static inline unsigned int memalloc_noio_save(void)
net/ceph/crypto.c:      noio_flag = memalloc_noio_save();
net/ceph/messenger.c:   noio_flag = memalloc_noio_save();

>
> -Denny
diff mbox

Patch

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index 7cbcfdac6529..31daeacaa1f9 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -39,6 +39,7 @@ 
 #include <linux/vmalloc.h>
 #include <linux/moduleparam.h>
 #include <linux/sched/signal.h>
+#include <linux/sched/mm.h>
 
 #include "ipoib.h"
 
@@ -1047,9 +1048,8 @@  static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
 		.sq_sig_type		= IB_SIGNAL_ALL_WR,
 		.qp_type		= IB_QPT_RC,
 		.qp_context		= tx,
-		.create_flags		= IB_QP_CREATE_USE_GFP_NOIO
+		.create_flags		= 0
 	};
-
 	struct ib_qp *tx_qp;
 
 	if (dev->features & NETIF_F_SG)
@@ -1057,10 +1057,6 @@  static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
 			min_t(u32, priv->ca->attrs.max_sge, MAX_SKB_FRAGS + 1);
 
 	tx_qp = ib_create_qp(priv->pd, &attr);
-	if (PTR_ERR(tx_qp) == -EINVAL) {
-		attr.create_flags &= ~IB_QP_CREATE_USE_GFP_NOIO;
-		tx_qp = ib_create_qp(priv->pd, &attr);
-	}
 	tx->max_send_sge = attr.cap.max_send_sge;
 	return tx_qp;
 }
@@ -1131,10 +1127,11 @@  static int ipoib_cm_tx_init(struct ipoib_cm_tx *p, u32 qpn,
 			    struct sa_path_rec *pathrec)
 {
 	struct ipoib_dev_priv *priv = ipoib_priv(p->dev);
+	unsigned int noio_flag;
 	int ret;
 
-	p->tx_ring = __vmalloc(ipoib_sendq_size * sizeof *p->tx_ring,
-			       GFP_NOIO, PAGE_KERNEL);
+	noio_flag = memalloc_noio_save();
+	p->tx_ring = vzalloc(ipoib_sendq_size * sizeof(*p->tx_ring));
 	if (!p->tx_ring) {
 		ret = -ENOMEM;
 		goto err_tx;
@@ -1142,9 +1139,10 @@  static int ipoib_cm_tx_init(struct ipoib_cm_tx *p, u32 qpn,
 	memset(p->tx_ring, 0, ipoib_sendq_size * sizeof *p->tx_ring);
 
 	p->qp = ipoib_cm_create_tx_qp(p->dev, p);
+	memalloc_noio_restore(noio_flag);
 	if (IS_ERR(p->qp)) {
 		ret = PTR_ERR(p->qp);
-		ipoib_warn(priv, "failed to allocate tx qp: %d\n", ret);
+		ipoib_warn(priv, "failed to create tx qp: %d\n", ret);
 		goto err_qp;
 	}