Re: [PATCH net-next] mctp: Add MCTP-over-serial transport binding

On Mon, Nov 22, 2021 at 04:23:10PM +0800, Jeremy Kerr wrote:

Hi Greg,

quoted

ida_destroy() will not be a no-op if you have allocated some things
in the past.  It should always be called when your module is removed.

Or at least that is how it used to be, if this has changed in the
past year, then I am mistaken here.

I think Greg is remembering how the IDA behaved before it was converted
to use the radix tree back in 2016 (0a835c4f090a).  About two-thirds
of the users of the IDA and IDR forgot to call ida_destroy/idr_destroy,
so rather than fix those places, I decided to make those data structures
no longer require a destructor.

I was going by this bit of the comment on ida_destroy:

   * Calling this function frees all IDs and releases all resources used
   * by an IDA.  When this call returns, the IDA is empty and can be reused
   * or freed.  If the IDA is already empty, there is no need to call this
   * function.

[From a documentation improvement in 50d97d50715]

Looking at ida_destroy, it's iterating the xarray and freeing all !value
entries. ida_free will free a (allocated) value entry once all bits are
clear, so the comment looks correct to me - there's nothing left to free
if the ida is empty.

However, I'm definitely no ida/idr/xarray expert! Happy to be corrected
here - and I'll send a patch to clarify that comment too, if so.

Cheers,


Jeremy