Re: [PATCH v3 1/6] xen: add reference counter support

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Jan Beulich <jbeulich@suse.com> 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