Re: [PATCH rdma-next 1/6] RDMA/uverbs: Introduce UCAP (User CAPabilities) API

[Leon Romanovsky <leon@kernel.org>]

On Tue, Mar 04, 2025 at 09:18:52AM -0400, Jason Gunthorpe wrote:
> On Wed, Feb 26, 2025 at 04:17:27PM +0200, Leon Romanovsky wrote:
> > +int ib_create_ucap(enum rdma_user_cap type)
> > +{
> > +	struct ib_ucap *ucap;
> > +	int ret;
> > +
> > +	if (type >= RDMA_UCAP_MAX)
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&ucaps_mutex);
> > +	ret = ib_ucaps_init();
> > +	if (ret)
> > +		goto unlock;
> > +
> > +	ucap = ucaps_list[type];
> > +	if (ucap) {
> > +		ucap->refcount++;
> > +		mutex_unlock(&ucaps_mutex);
> > +		return 0;
> > +	}
> > +
> > +	ucap = kzalloc(sizeof(*ucap), GFP_KERNEL);
> > +	if (!ucap) {
> > +		ret = -ENOMEM;
> > +		goto unlock;
> > +	}
> > +
> > +	device_initialize(&ucap->dev);
> > +	ucap->dev.class = &ucaps_class;
> > +	ucap->dev.devt = MKDEV(MAJOR(ucaps_base_dev), type);
> > +	ucap->dev.release = ucap_dev_release;
> > +	dev_set_name(&ucap->dev, ucap_names[type]);
> 
> Missing error handling on dev_set_name()

Most of the kernel users don't check dev_set_name(). It can't fail in
reality.

> 
> > +#define UCAP_ENABLED(ucaps, type) (!!((ucaps) & (1U << type)))
> 
> Missing () around type
> 
> Why not use a static inline?

I don't think that static inline gives any benefit here.

Thanks

> 
> Jason