From nobody Wed Dec 14 22:31:41 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4NXVTY4HKpz4kkYF; Wed, 14 Dec 2022 22:31:45 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-oa1-x2b.google.com (mail-oa1-x2b.google.com [IPv6:2001:4860:4864:20::2b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4NXVTY25sxz4Jjq; Wed, 14 Dec 2022 22:31:45 +0000 (UTC) (envelope-from mjguzik@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-oa1-x2b.google.com with SMTP id 586e51a60fabf-144bd860fdbso18702035fac.0; Wed, 14 Dec 2022 14:31:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=1vm8mQgonFMfFlLJdG6zELGiNPb8TeHlvuHeVeI4ldQ=; b=Kf+ssh4rPBTVnpzMeme8XZB8L0YbppEPd/DdzJL8mUtyhnpnwQViAafvjBhZPDnh22 H5LupHujPsDQgA2skMp76Bf6cN2RPyGNJ2L/RgC1xHEKO4R1EcREypczS7nfGsHc+3aF 6rkvzI/iHKxzqOYP5HD9iXtqAuBMhXDZ1dzcxZfw72bntLHNi1DuHMiZiUTaRd6ZJZZo AFHtpJNcgiiSPA9ClixzAaHz4YG2VQLYWvCY3vZwa7TLRztfD2hUXgybn0gF8SO6xePn fl5nvMZylywve1qnt6AlJ+UpT9TH3kWjF9h5Q3GpyMxn8suPH/1ehn5lQ3Z0G0V5SNo6 hEIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=1vm8mQgonFMfFlLJdG6zELGiNPb8TeHlvuHeVeI4ldQ=; b=2Pqjx7eiCXUkwvLDlyd2ld/2ZMOcQlCDRZO10Dz7/9iWK76lf7QIsSy751NXSiYoJZ zmi5fFlW/+r3US6sjoCmfDaal49bfu6Y0M13kXDn2UWVLFJm7cIW2+gZtS/GhsS/bbtC uY+0IvZq+i4xH5IaVTc8i6gzBq+GLj2zOJaIb7vtEI1NMhv8YIb6nMhzromxJ21rVIeP DXt9UGtALhkEYoR3bg2PCC2x2k1sw2y/lMC2YmuirI/HZIFgFex3nt8CXaWG6+4YDigq HBMzX3CDR6QgwgBfCfTFmQF5VCXSvDjRskFNO+AbM2Q77J/+W4iKIi3lxM6PKJ3gXvV3 eCQA== X-Gm-Message-State: AFqh2krLcVjH8pY8tNvdQjaRYKdJC//PEgr0o3aB8QeCMpCna1FbnSF5 QfaCsVTE29w7/l5e+obefU+r5yXidvHjxSDuatNOr1f6 X-Google-Smtp-Source: AA0mqf5i1pE3kyMDKJjxE8O2DiaULohjnXeGstuTeT4li4SLMSsmQzKKLqiEAYg+SQXyiMKFcEBy15OzZyTdev0MBmw= X-Received: by 2002:a05:6870:b293:b0:144:5f0d:9ff6 with SMTP id c19-20020a056870b29300b001445f0d9ff6mr550077oao.228.1671057102224; Wed, 14 Dec 2022 14:31:42 -0800 (PST) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Received: by 2002:a8a:dd0:0:b0:48f:2c93:a3ef with HTTP; Wed, 14 Dec 2022 14:31:41 -0800 (PST) In-Reply-To: References: <202212132047.2BDKl92M065327@gitrepo.freebsd.org> From: Mateusz Guzik Date: Wed, 14 Dec 2022 23:31:41 +0100 Message-ID: Subject: Re: git: e6bc24b0385a - main - kref: switch internal type to atomic_t and bring back const to kref_read To: Alexander Richardson Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4NXVTY25sxz4Jjq X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2001:4860:4864::/48, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On 12/14/22, Alexander Richardson wrote: > On Tue, 13 Dec 2022 at 20:47, Mateusz Guzik wrote: >> >> The branch main has been updated by mjg: >> >> URL: >> https://cgit.FreeBSD.org/src/commit/?id=e6bc24b0385a166526d20b2eb0cbb6a116350075 >> >> commit e6bc24b0385a166526d20b2eb0cbb6a116350075 >> Author: Mateusz Guzik >> AuthorDate: 2022-12-13 20:42:32 +0000 >> Commit: Mateusz Guzik >> 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? I agree it looks like crap, but I did not see a point in going down that rabbit hole right now. Note if one was to patch load_int, one should also patch all other types for consistency. But then there is also kcsan variants to sort out, making it into a rather big crunchfest for next to no gain from my pov. If anything I don't see why they added const in linux for this one. >> } >> >> 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; >> } > -- Mateusz Guzik