Re: git: e6bc24b0385a - main - kref: switch internal type to atomic_t and bring back const to kref_read

On Tue, 13 Dec 2022 at 20:47, Mateusz Guzik <mjg@freebsd.org> wrote:
>
> The branch main has been updated by mjg:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=e6bc24b0385a166526d20b2eb0cbb6a116350075
>
> commit e6bc24b0385a166526d20b2eb0cbb6a116350075
> Author:     Mateusz Guzik <mjg@FreeBSD.org>
> AuthorDate: 2022-12-13 20:42:32 +0000
> Commit:     Mateusz Guzik <mjg@FreeBSD.org>
> CommitDate: 2022-12-13 20:46:58 +0000
>
>     kref: switch internal type to atomic_t and bring back const to kref_read
>
>     This unbreak drm-kmod build.
>
>     the const is part of Linux API
>
>     Unfortunately drm-kmod uses hand-rolled refcount* calls on a kref
>     object. For now go the easy route of keeping it operational by casting
>     stuff internally.
>
>     The general goal here is to make FreeBSD refcount API use an opaque
>     type, hence the ongoing removal of hand-rolled accesses.
>
>     Reported by:    emaste
> ---
>  sys/compat/linuxkpi/common/include/linux/kref.h | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/sys/compat/linuxkpi/common/include/linux/kref.h b/sys/compat/linuxkpi/common/include/linux/kref.h
> index 9a6814175223..5a1fd834a58d 100644
> --- a/sys/compat/linuxkpi/common/include/linux/kref.h
> +++ b/sys/compat/linuxkpi/common/include/linux/kref.h
> @@ -44,35 +44,35 @@
>
>  struct kref {
>         /* XXX In Linux this is a refcount_t */
> -       volatile u_int32_t refcount;
> +       atomic_t refcount;
>  };
>
>  static inline void
>  kref_init(struct kref *kref)
>  {
>
> -       refcount_init(&kref->refcount, 1);
> +       refcount_init((uint32_t *)&kref->refcount, 1);
>  }
>
>  static inline unsigned int
> -kref_read(struct kref *kref)
> +kref_read(const struct kref *kref)
>  {
>
> -       return (refcount_load(&kref->refcount));
> +       return (refcount_load(__DECONST(u_int32_t *, &kref->refcount)));

This is rather ugly IMO, can't we change refcount_load() to take a
const pointer? It looks like the problem might be
__atomic_load_int_relaxed(), but that could use const as well?
>  }
>
>  static inline void
>  kref_get(struct kref *kref)
>  {
>
> -       refcount_acquire(&kref->refcount);
> +       refcount_acquire((uint32_t *)&kref->refcount);
>  }
>
>  static inline int
>  kref_put(struct kref *kref, void (*rel)(struct kref *kref))
>  {
>
> -       if (refcount_release(&kref->refcount)) {
> +       if (refcount_release((uint32_t *)&kref->refcount)) {
>                 rel(kref);
>                 return 1;
>         }
> @@ -84,7 +84,7 @@ kref_put_lock(struct kref *kref, void (*rel)(struct kref *kref),
>      spinlock_t *lock)
>  {
>
> -       if (refcount_release(&kref->refcount)) {
> +       if (refcount_release((uint32_t *)&kref->refcount)) {
>                 spin_lock(lock);
>                 rel(kref);
>                 return (1);
> @@ -98,7 +98,7 @@ kref_sub(struct kref *kref, unsigned int count,
>  {
>
>         while (count--) {
> -               if (refcount_release(&kref->refcount)) {
> +               if (refcount_release((uint32_t *)&kref->refcount)) {
>                         rel(kref);
>                         return 1;
>                 }
> @@ -110,16 +110,16 @@ static inline int __must_check
>  kref_get_unless_zero(struct kref *kref)
>  {
>
> -       return refcount_acquire_if_not_zero(&kref->refcount);
> +       return refcount_acquire_if_not_zero((uint32_t *)&kref->refcount);
>  }
>
>  static inline int kref_put_mutex(struct kref *kref,
>      void (*release)(struct kref *kref), struct mutex *lock)
>  {
>         WARN_ON(release == NULL);
> -       if (unlikely(!refcount_release_if_not_last(&kref->refcount))) {
> +       if (unlikely(!refcount_release_if_not_last((uint32_t *)&kref->refcount))) {
>                 mutex_lock(lock);
> -               if (unlikely(!refcount_release(&kref->refcount))) {
> +               if (unlikely(!refcount_release((uint32_t *)&kref->refcount))) {
>                         mutex_unlock(lock);
>                         return 0;
>                 }