lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 12 Jun 2018 17:07:39 -0700 From: Matthew Wilcox <willy@...radead.org> To: Jason Gunthorpe <jgg@...pe.ca> Cc: hans.westgaard.ry@...cle.com, Doug Ledford <dledford@...hat.com>, Matthew Wilcox <mawilcox@...rosoft.com>, linux-rdma@...r.kernel.org, HÃ¥kon Bugge <haakon.bugge@...cle.com>, Parav Pandit <parav@...lanox.com>, Jack Morgenstein <jackm@....mellanox.co.il>, Pravin Shedge <pravin.shedge4linux@...il.com>, linux-kernel@...r.kernel.org Subject: Re: [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!
Powered by blists - more mailing lists