Re: [RFC PATCH 04/10] xen: add reference counter support

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hello Jan,

Jan Beulich <jbeulich@xxxxxxxx> writes:

> On 31.08.2022 16:10, Volodymyr Babchuk wrote:
>> --- /dev/null
>> +++ b/xen/include/xen/refcnt.h
>> @@ -0,0 +1,28 @@
>> +#ifndef __XEN_REFCNT_H__
>> +#define __XEN_REFCNT_H__
>> +
>> +#include <asm/atomic.h>
>> +
>> +typedef atomic_t refcnt_t;
>
> Like Linux has it, I think this would better be a separate struct. At
> least in debug builds, i.e. it could certainly use typesafe.h if that
> ended up to be a good fit (which I'm not sure it would, so this is
> merely a thought).

Sadly, TYPE_SAFE does not support pointers. e.g I can't get pointer to
an encapsulated value which is also passed as a pointer. I can expand
TYPE_SAFE with $FOO_x_ptr():

    static inline _type *_name##_x_ptr(_name##_t *n) { &return n->_name; }

or make custom encapsulation in refcnt.h. Which one you prefer?

>> +static inline void refcnt_init(refcnt_t *refcnt)
>> +{
>> +    atomic_set(refcnt, 1);
>> +}
>> +
>> +static inline void refcnt_get(refcnt_t *refcnt)
>> +{
>> +#ifndef NDEBUG
>> +    ASSERT(atomic_add_unless(refcnt, 1, 0) > 0);
>> +#else
>> +    atomic_add_unless(refcnt, 1, 0);
>> +#endif
>> +}

> I think this wants doing without any #ifdef-ary, e.g.
>
> static inline void refcnt_get(refcnt_t *refcnt)
> {
>     int ret = atomic_add_unless(refcnt, 1, 0);
>
>     ASSERT(ret > 0);
> }
>

Thanks, did as you suggested. I was afraid that compiler would complain
about unused ret in non-debug builds.

> I wonder though whether certain callers may not want to instead know
> whether a refcount was successfully obtained, i.e. whether instead of
> asserting here you don't want to return a boolean success indicator,
> which callers then would deal with (either by asserting or by suitably
> handling the case). See get_page() and page_get_owner_and_reference()
> for similar behavior we have (and use) already.

For now there are no such callers, so I don't want to implement unused
functionality. But, if you prefer this way, I'll do this.

[...]