[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH 04/10] xen: add reference counter support
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). > +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); } 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. > +static inline void refcnt_put(refcnt_t *refcnt, void (*destructor)(refcnt_t > *refcnt)) > +{ > + if ( atomic_dec_and_test(refcnt) ) > + destructor(refcnt); > +} No assertion here as to the count being positive? Also the entire file wants to use Xen's space indentation, not hard tabs. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |