On 12.04.2023 00:38, Volodymyr Babchuk wrote:
> 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.
If provided via wrappers, that'll be fine of course.
>>> +{
>>> + 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.
Wait. Saturating on overflow might still be a reasonable concept. But
here you convert an underflow to the "saturated" value.
Jan
