> On Mar 1, 2023, at 2:18 PM, Uros Bizjak <ubizjak@xxxxxxxxx> wrote: > > On Wed, Mar 1, 2023 at 7:52 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: >> >>> On Wed, 1 Mar 2023 19:43:34 +0100 >>> Uros Bizjak <ubizjak@xxxxxxxxx> wrote: >>> >>> On Wed, Mar 1, 2023 at 5:38 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: >>>> >>>> On Wed, 1 Mar 2023 11:28:47 +0100 >>>> Uros Bizjak <ubizjak@xxxxxxxxx> wrote: >>>> >>>>> These benefits are the reason the change to try_cmpxchg was accepted >>>>> also in the linear code elsewhere in the linux kernel, e.g. [2,3] to >>>>> name a few commits, with a thumbs-up and a claim that the new code is >>>>> actually *clearer* at the merge commit [4]. >>>> >>>> I'll say it here too. I really like Joel's suggestion of having a >>>> cmpxchg_success() that does not have the added side effect of modifying the >>>> old variable. >>>> >>>> I think that would allow for the arch optimizations that you are trying to >>>> achieve, as well as remove the side effect that might cause issues down the >>>> road. >>> >>> Attached patch implements this suggestion. >> >> I like it! Me too :) > Thanks! > >> Anyway to make this more generic? > > 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 > > if (cmpxchg (ptr, 0, 1) == 0) > > that can not be converted to try_cmpxchg, but can use cmpxchg_success. I would prefer if we can put it in generic headers instead of duplicating across ftrace and RCU. thanks, - Joel > > Uros.
