Re: [PATCH] mlx4: Use GFP_NOFS calls during the ipoib TX path when creating the QP

[Jiri Kosina]

On Tue, 11 Mar 2014, Or Gerlitz wrote:

> > It does seem that only using GFP_NOIO when we really need to would be
> > a very difficult problem--how can we carry information about whether a
> > particular packet is involved in freeing memory through all the layers
> > of, say, NFS, TCP, IPSEC, bonding, &c?
> 
> Agree with all the above... next,
> 
> If we don't have away to nicely overcome the layer violations here, 
> let's change IPoIB so they always ask the IB driver to allocate QPs used 
> for Connected Mode in a GFP_NOIO manner, to be practical I suggest the 
> following:
> 
> 1. Add new QP creation flag IB_QP_CREATE_USE_GFP to the existing 
> creation flags of struct ib_qp_init_attr and a new "gfp_t gfp" field to 
> that structure too
> 
> 2. in the IPoIB CM code, do the vzalloc allocation for new connection in 
> GFP_NOIO manner and issue the call to create QP with setting the 
> IB_QP_CREATE_USE_GFP flag and GFO_NOIO to the gfp field
> 
> 3. If the QP creation fails, with -EINVAL, issue a warning and retry the 
> QP creation attempt without the GFP setting
> 
> 4. implement in the mlx4 driver the support for GFP directives on QP 
> creation

1-4 make perfect sense to me.

> 5. for the rest of the IB drivers, return -EINVAL if 
> IB_QP_CREATE_USE_GFP is set

Umm, why this?

> This will allow to provide working solution for mlx4 users and gradually 
> add support for the rest of the IB drivers.

Oh, I see, so that's just a temporary measure.

> as for proper patch planning
> 
> patch #1 / items 1 and 5
> patch #2 / item 4
> patch #3 / item 3
> 
> Re item 5 -- I made a check and the ehca, ipath and mthca driver already
> return -EINVAL if provided with any creation flag, so you only need to patch
> the qib driver in qib_create_qp() to do that as well which is trivial.
> 
> As for the rest of the code, you practically have it all by now, just need to
> port the mlx4 changes you did to the suggested framework, remove the module
> param (which you don't like either) and add the new gfp_t field to
> ib_qp_init_attr
> 
> So sounds like a plan that makes sense?

Sounds fine by me.

Thanks,

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/