Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: qWOUaitVt9R+WqD7QJqRHHfkaX2aOKIaJ+8TeGJAWYXTlEdkA67lO3MBAVqn+3xzrLkkU/rumz
 In8bIH67ZQBZPQokk959lT+Ttqv7a3+zQ4rU4Dh+OJhu+M3vZY93tkP57xBbaZzOMPKS7az12V
 B7m6krJY5kfaD9nLNbS6HG3cRWY/0/5Rry3rQAVcfeVmnSZW01qV8ddVozMTHH0on082rakCU/
 wldnszTNUPgsjcDBpuBGzE6BtZ4gdqBkrMR2tzwbVLzsDFXXcGmBZftECoflzhWHC/nMC2j8aw
 kHE=
From:   Anup Patel <Anup.Patel@wdc.com>
To:     Peter Zijlstra <peterz@infradead.org>, Guo Ren <guoren@kernel.org>
CC:     linux-riscv <linux-riscv@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "linux-csky@vger.kernel.org" <linux-csky@vger.kernel.org>,
        linux-arch <linux-arch@vger.kernel.org>,
        Guo Ren <guoren@linux.alibaba.com>,
        Will Deacon <will@kernel.org>, Ingo Molnar <mingo@redhat.com>,
        Waiman Long <longman@redhat.com>,
        Arnd Bergmann <arnd@arndb.de>, Anup Patel <anup@brainfault.org>
Subject: RE: [PATCH v4 3/4] locking/qspinlock: Add
 ARCH_USE_QUEUED_SPINLOCKS_XCHG32
Thread-Topic: [PATCH v4 3/4] locking/qspinlock: Add
 ARCH_USE_QUEUED_SPINLOCKS_XCHG32
Thread-Index: AQHXIzQ13Qj+G1dtVUSTIu+6TgYVfKqamdaAgAA6hoCAAAIFgIAACjeQ
Date:   Mon, 29 Mar 2021 12:13:10 +0000
Message-ID: <DM6PR04MB6201B2774866802C268F7B628D7E9@DM6PR04MB6201.namprd04.prod.outlook.com>
References: <1616868399-82848-1-git-send-email-guoren@kernel.org>
 <1616868399-82848-4-git-send-email-guoren@kernel.org>
 <YGGGqftfr872/4CU@hirez.programming.kicks-ass.net>
 <CAJF2gTQNV+_txMHJw0cmtS-xcnuaCja-F7XBuOL_J0yN39c+uQ@mail.gmail.com>
 <YGG5c4QGq6q+lKZI@hirez.programming.kicks-ass.net>
In-Reply-To: <YGG5c4QGq6q+lKZI@hirez.programming.kicks-ass.net>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 5f4e7b6d-ee6b-4611-a54e-08d8f2ac04f5
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Mar 2021 12:13:10.2930
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 4EkHioaiR+r9p+H52PQ1Rllc7Zd4x6C83sUqeDZgNtdsdT2yrRSqLwtOJJBOTnXIm7ERCjEPdO47ZzdrKPejxg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR04MB1147
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Peter Zijlstra <peterz@infradead.org>
> Sent: 29 March 2021 16:57
> To: Guo Ren <guoren@kernel.org>
> Cc: linux-riscv <linux-riscv@lists.infradead.org>; Linux Kernel Mailing L=
ist
> <linux-kernel@vger.kernel.org>; linux-csky@vger.kernel.org; linux-arch
> <linux-arch@vger.kernel.org>; Guo Ren <guoren@linux.alibaba.com>; Will
> Deacon <will@kernel.org>; Ingo Molnar <mingo@redhat.com>; Waiman
> Long <longman@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Anup
> Patel <anup@brainfault.org>
> Subject: Re: [PATCH v4 3/4] locking/qspinlock: Add
> ARCH_USE_QUEUED_SPINLOCKS_XCHG32
>=20
> On Mon, Mar 29, 2021 at 07:19:29PM +0800, Guo Ren wrote:
> > On Mon, Mar 29, 2021 at 3:50 PM Peter Zijlstra <peterz@infradead.org>
> wrote:
> > >
> > > On Sat, Mar 27, 2021 at 06:06:38PM +0000, guoren@kernel.org wrote:
> > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > >
> > > > Some architectures don't have sub-word swap atomic instruction,
> > > > they only have the full word's one.
> > > >
> > > > The sub-word swap only improve the performance when:
> > > > NR_CPUS < 16K
> > > >  *  0- 7: locked byte
> > > >  *     8: pending
> > > >  *  9-15: not used
> > > >  * 16-17: tail index
> > > >  * 18-31: tail cpu (+1)
> > > >
> > > > The 9-15 bits are wasted to use xchg16 in xchg_tail.
> > > >
> > > > Please let architecture select xchg16/xchg32 to implement
> > > > xchg_tail.
> > >
> > > So I really don't like this, this pushes complexity into the generic
> > > code for something that's really not needed.
> > >
> > > Lots of RISC already implement sub-word atomics using word ll/sc.
> > > Obviously they're not sharing code like they should be :/ See for
> > > example arch/mips/kernel/cmpxchg.c.
> > I see, we've done two versions of this:
> >  - Using cmpxchg codes from MIPS by Michael
> >  - Re-write with assembly codes by Guo
> >
> > But using the full-word atomic xchg instructions implement xchg16 has
> > the semantic risk for atomic operations.
>=20
> What? -ENOPARSE
>=20
> > > Also, I really do think doing ticket locks first is a far more
> > > sensible step.
> > NACK by Anup
>=20
> Who's he when he's not sending NAKs ?

We had discussions in the RISC-V platforms group about this. Over there,
We had evaluated all spin lock approaches (ticket, qspinlock, etc) tried
in Linux till now. It was concluded in those discussions that eventually we
have to move to qspinlock (even if we moved to ticket spinlock temporarily)
because qspinlock avoids cache line bouncing. Also, moving to qspinlock
will be aligned with other major architectures supported in Linux (such as
x86, ARM64)

Some of the organizations working on high-end RISC-V systems (> 32
CPUs) are interested in having an optimized spinlock implementation
(just like other major architectures x86 and ARM64).

Based on above, Linux RISC-V should move to qspinlock.

Regards,
Anup