lkml.org 
[lkml]   [2018]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] IB/mad: Use IDR for agent IDs
On Tue, Jun 12, 2018 at 02:33:22PM -0600, Jason Gunthorpe wrote:
> > @@ -377,13 +378,24 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
> > goto error4;
> > }
> >
> > - spin_lock_irq(&port_priv->reg_lock);
> > - mad_agent_priv->agent.hi_tid = atomic_inc_return(&ib_mad_client_id);
> > + idr_preload(GFP_KERNEL);
> > + idr_lock(&ib_mad_clients);
> > + ret2 = idr_alloc_cyclic(&ib_mad_clients, mad_agent_priv, 0,
> > + (1 << 24), GFP_ATOMIC);
>
> I like this series, my only concern is this magic number here, at the
> very least it deserves a big comment explaining why it
> exists..

Yes, you're right. Maybe something like this ...

/*
* The mlx4 driver uses the top byte to distinguish which virtual function
* generated the MAD, so we must avoid using it.
*/
#define AGENT_ID_LIMIT (1 << 24)

...

ret2 = idr_alloc_cyclic(&ib_mad_clients, mad_agent_priv, 0,
AGENT_ID_LIMIT, GFP_ATOMIC);

> Let me see if I can get someone to give it some test time, I assume
> you haven't been able to test it it?

I don't have any IB hardware. Frankly I'm scared of Infiniband ;-)

> > +#define idr_lock(idr) xa_lock(&(idr)->idr_rt)
> > +#define idr_unlock(idr) xa_unlock(&(idr)->idr_rt)
> > +#define idr_lock_irq(idr) xa_lock_irq(&(idr)->idr_rt)
> > +#define idr_unlock_irq(idr) xa_unlock_irq(&(idr)->idr_rt)
> > +#define idr_lock_irqsave(idr, flags) \
> > + xa_lock_irqsave(&(idr)->idr_rt, flags)
> > +#define idr_unlock_irqrestore(idr, flags) \
> > + xa_unlock_irqrestore(&(idr)->idr_rt, flags)
> > +
>
> And you are Ok to take these through the rdma tree?

Yes, that's fine with me; I'm not planning on merging any IDR patches
this cycle. Thanks!

\
 
 \ /
  Last update: 2018-06-13 02:08    [W:0.113 / U:2.484 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site