RE: [PATCH v4 07/19] IB/mad: Convert ib_mad_private allocations from kmem_cache to kmalloc

["Weiny, Ira" <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

> >
> > +static struct ib_mad_private *alloc_mad_priv(struct ib_device *dev) {
> > +	return (kmalloc(sizeof(struct ib_mad_private_header) +
> > +			sizeof(struct ib_grh) +
> > +			dev->cached_dev_attrs.max_mad_size,
> GFP_ATOMIC));
> 
> Ouch!  GFP_ATOMIC?  I thought that generally all of the mad processing was
> done from workqueue context where sleeping is allowed?  In the two places
> where you removed kmem_cache_alloc() calls and replaced it with calls to this
> code, they both used GFP_KERNEL and now you have switched it to
> GFP_ATOMIC.  If there isn't a good reason for this, it should be switched back
> to GFP_KERNEL.

The original kmem_cache_allocs are actually both GFP_ATOMIC (1 usage, see below) and GFP_KERNEL (the 2 usages you reference).

My bad for not making this specific to the allocation.

I will research the original GFP_ATOMIC usage and if it is necessary have this function take gfp_t.  Otherwise if we can get away with GFP_KERNEL I agree that would be best.

> 
> > +}
> > +
> >  /*
> >   * Return 0 if SMP is to be sent
> >   * Return 1 if SMP was consumed locally (whether or not solicited) @@
> > -771,7 +776,8 @@ static int handle_outgoing_dr_smp(struct
> ib_mad_agent_private *mad_agent_priv,
> >  	}
> >  	local->mad_priv = NULL;
> >  	local->recv_mad_agent = NULL;
> > -	mad_priv = kmem_cache_alloc(ib_mad_cache, GFP_ATOMIC);

Original usage here... 

Thanks,
Ira

> > +
> > +	mad_priv = alloc_mad_priv(mad_agent_priv->agent.device);
> >  	if (!mad_priv) {
> >  		ret = -ENOMEM;
> >  		dev_err(&device->dev, "No memory for local response
> MAD\n"); @@