Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Joel Fernandes <joel@joelfernandes.org>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH] rcu: use try_cmpxchg in check_cpu_stall
Date:   Wed, 1 Mar 2023 14:46:25 -0500
Message-Id: <00F4E7D7-6E54-4BC0-9735-4038F8DBAE20@joelfernandes.org>
References: <20230301144355.338c62de@gandalf.local.home>
Cc:     Uros Bizjak <ubizjak@gmail.com>,
        "Paul E. McKenney" <paulmck@kernel.org>, rcu@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Frederic Weisbecker <frederic@kernel.org>,
        Neeraj Upadhyay <quic_neeraju@quicinc.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Lai Jiangshan <jiangshanlai@gmail.com>
In-Reply-To: <20230301144355.338c62de@gandalf.local.home>
To:     Steven Rostedt <rostedt@goodmis.org>
Precedence: bulk


> On Mar 1, 2023, at 2:43 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>=20
> =EF=BB=BFOn Wed, 1 Mar 2023 20:18:11 +0100
> Uros Bizjak <ubizjak@gmail.com> wrote:
>=20
>> If we want to put the definition in generic headers, then we also need
>> to define acquire/release/relaxed and 64bit variants. ATM, we have two
>> sites that require this definition and I think that for now we could
>> live with two instances of the same definition in two separate
>> subsystems. But this would definitely be a good future addition. There
>> is some code in the form of
>>=20
>> if (cmpxchg (ptr, 0, 1) =3D=3D 0)
>>=20
>> that can not be converted to try_cmpxchg, but can use cmpxchg_success.
>=20
> And even modify code that uses temp variables. For example, where you
> modified the ring buffer code to use try_cmpxchg(), I could convert your:
>=20
> static int rb_head_page_replace(struct buffer_page *old,
>                struct buffer_page *new)
> {
>    unsigned long *ptr =3D (unsigned long *)&old->list.prev->next;
>    unsigned long val;
>=20
>    val =3D *ptr & ~RB_FLAG_MASK;
>    val |=3D RB_PAGE_HEAD;
>=20
>    return try_cmpxchg(ptr, &val, (unsigned long)&new->list);
> }
>=20
> Into just:
>=20
> static int rb_head_page_replace(struct buffer_page *old,
>                struct buffer_page *new)
> {
>    unsigned long *ptr =3D (unsigned long *)&old->list.prev->next;
>    unsigned long val;
>=20
>    val =3D *ptr & ~RB_FLAG_MASK;
>=20
>    return cmpxchg_success(ptr, val | RB_PAGE_HEAD, (unsigned long)&new->li=
st);
> }

Nice! Looks much better.

Thanks,

 - Joel

>=20
> -- Steve