Re: [PATCH] RDMA/srpt: Make slab cache names unique

[Bart Van Assche <bvanassche@acm.org>]

On 10/7/24 9:28 AM, Jens Axboe wrote:
> On 10/7/24 10:14 AM, Bart Van Assche wrote:
>> On 10/7/24 7:06 AM, Jens Axboe wrote:
>>> Still seems way over engineered, just use an atomic_long_t for a
>>> continually increasing index number.
>>
>> Even an atomic_long_t can wrap around and hence can result in duplicate
>> slab cache names. With my patch it is guaranteed that slab cache names
>> are unique. I'm not claiming that this patch is the best possible
>> solution but it's a working solution and a solution that doesn't require
>> too many changes to the ib_srpt driver.
> 
> Come on... The current patch doesn't even check if ida_alloc() got an ID.
> Without that, using some mechanism to alloc+free an index is surely less
> than useful.

Is it necessary in this case to check the ida_alloc() result? ida_free()
ignores negative values. So if ida_alloc() fails, the worst that can
happen is that a slab name with a negative number is passed to 
kmem_cache_create(). Additionally, if my interpretation of the ida code
is correct, it allocates memory in 128 byte chunks. So if allocation of
an ida fails, it means that less than 128 bytes of memory are left. More
than 128 bytes are required by kmem_cache_create(). Hence, if ida
allocation fails, the kmem cache creation will also fail and the slab
name with the negative number will not become visible in procfs.

Do you agree that in this case it is safe not to check whether
ida_alloc() succeeds?

Thanks,

Bart.