Jan Beulich <[email protected]> writes:
> On 14.03.2023 21:56, Volodymyr Babchuk wrote:
>> +static inline void refcnt_put(refcnt_t *refcnt, void (*destructor)(refcnt_t
>> *refcnt))
>
> Hmm, this means all callers need to pass (and agree on) the supposedly
> single destructor function that needs calling. Wouldn't the destructor
> function better be stored elsewhere (and supplied to e.g. refcnt_init())?
>
I tried to replicate Linux approach. They provide destructor function
every time. On other hand, kref_put() is often called from a wrapper
function (like pdev_put() in our case), so destructor in fact, is
provided only once.
>> +{
>> + int ret = atomic_dec_return(&refcnt->refcnt);
>> +
>> + if ( ret == 0 )
>> + destructor(refcnt);
>> +
>> + if ( unlikely(ret < 0))
>> + {
>> + atomic_set(&refcnt->refcnt, REFCNT_SATURATED);
>
> It's undefined whether *refcnt still exists once the destructor was
> called (which would have happened before we can make it here). While
> even the atomic_dec_return() above would already have acted in an
> unknown way in this case I don't think it's a good idea to access the
> object yet another time. (Same for the "negative" case in
> refcnt_get() then.)
Okay, then I'll remove saturation logic.
--
WBR, Volodymyr
