Linux-csky Archive on lore.kernel.org
 help / color / Atom feed
From: Guo Ren <guoren@kernel.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Peter Zijlstra <peterz@infradead.org>,
	linux-riscv <linux-riscv@lists.infradead.org>,
	Linux Kernel Mailing List <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>,
	Anup Patel <anup@brainfault.org>,
	Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Subject: Re: [PATCH v4 3/4] locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
Date: Wed, 31 Mar 2021 14:44:39 +0800
Message-ID: <CAJF2gTSXY1fCBSZ-Z=8_AcRxoiCOoaNu-5A_JximGJxZY18RzQ@mail.gmail.com> (raw)
In-Reply-To: <CAJF2gTSGLn7katm6YAtkKWJcQRqw36_yqn+aK1pKUSRM5V1zUg@mail.gmail.com>

Hi Arnd

On Wed, Mar 31, 2021 at 12:18 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Tue, Mar 30, 2021 at 3:12 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, Mar 30, 2021 at 4:26 AM Guo Ren <guoren@kernel.org> wrote:
> > > On Mon, Mar 29, 2021 at 9:56 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Mon, Mar 29, 2021 at 2:52 PM Guo Ren <guoren@kernel.org> wrote:
> > > > > On Mon, Mar 29, 2021 at 7:31 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > > > > >
> > > > > > What's the architectural guarantee on LL/SC progress for RISC-V ?
> > > >
> > > >    "When LR/SC is used for memory locations marked RsrvNonEventual,
> > > >      software should provide alternative fall-back mechanisms used when
> > > >      lack of progress is detected."
> > > >
> > > > My reading of this is that if the example you tried stalls, then either
> > > > the PMA is not RsrvEventual, and it is wrong to rely on ll/sc on this,
> > > > or that the PMA is marked RsrvEventual but the implementation is
> > > > buggy.
> > >
> > > Yes, PMA just defines physical memory region attributes, But in our
> > > processor, when MMU is enabled (satp's value register > 2) in s-mode,
> > > it will look at our custom PTE's attributes BIT(63) ref [1]:
> > >
> > >    PTE format:
> > >    | 63 | 62 | 61 | 60 | 59 | 58-8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> > >      SO   C    B    SH   SE    RSW   D   A   G   U   X   W   R   V
> > >      ^    ^    ^    ^    ^
> > >    BIT(63): SO - Strong Order
> > >    BIT(62): C  - Cacheable
> > >    BIT(61): B  - Bufferable
> > >    BIT(60): SH - Shareable
> > >    BIT(59): SE - Security
> > >
> > > So the memory also could be RsrvNone/RsrvEventual.
> >
> > I was not talking about RsrvNone, which would clearly mean that
> > you cannot use lr/sc at all (trap would trap, right?), but "RsrvNonEventual",
> > which would explain the behavior you described in an earlier reply:
> >
> > | u32 a = 0x55aa66bb;
> > | u16 *ptr = &a;
> > |
> > | CPU0                       CPU1
> > | =========             =========
> > | xchg16(ptr, new)     while(1)
> > |                                     WRITE_ONCE(*(ptr + 1), x);
> > |
> > | When we use lr.w/sc.w implement xchg16, it'll cause CPU0 deadlock.
> >
> > As I understand, this example must not cause a deadlock on
> > a compliant hardware implementation when the underlying memory
> > has RsrvEventual behavior, but could deadlock in case of
> > RsrvNonEventual
> Thx for the nice explanation:
>  - RsrvNonEventual - depends on software fall-back mechanisms, and
> just I'm worried about.
>  - RsrvEventual - HW would provide the eventual success guarantee.
In riscv-spec 8.3 Eventual Success of Store-Conditional Instructions

I found:
"As a consequence of the eventuality guarantee, if some harts in an
execution environment are
executing constrained LR/SC loops, and no other harts or devices in
the execution environment
execute an unconditional store or AMO to that reservation set, then at
least one hart will
eventually exit its constrained LR/SC loop. *** By contrast, if other
harts or devices continue to
write to that reservation set, it ***is not guaranteed*** that any
hart will exit its LR/SC loop.*** "

Seems RsrvEventual couldn't solve the code's problem I've mentioned.

>
> >
> > > [1] https://github.com/c-sky/csky-linux/commit/e837aad23148542771794d8a2fcc52afd0fcbf88
> > >
> > > >
> > > > It also seems that the current "amoswap" based implementation
> > > > would be reliable independent of RsrvEventual/RsrvNonEventual.
> > >
> > > Yes, the hardware implementation of AMO could be different from LR/SC.
> > > AMO could use ACE snoop holding to lock the bus in hw coherency
> > > design, but LR/SC uses an exclusive monitor without locking the bus.
> > >
> > > RISC-V hasn't CAS instructions, and it uses LR/SC for cmpxchg. I don't
> > > think LR/SC would be slower than CAS, and CAS is just good for code
> > > size.
> >
> > What I meant here is that the current spinlock uses a simple amoswap,
> > which presumably does not suffer from the lack of forward process you
> > described.
> Does that mean we should prevent using LR/SC (if RsrvNonEventual)?
>
> --
> Best Regards
>  Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

  parent reply index

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-27 18:06 [PATCH v4 0/4] riscv: Add qspinlock/qrwlock guoren
2021-03-27 18:06 ` [PATCH v4 1/4] riscv: cmpxchg.h: Cleanup unused code guoren
2021-03-27 18:06 ` [PATCH v4 2/4] riscv: cmpxchg.h: Merge macros guoren
2021-03-27 21:25   ` Arnd Bergmann
2021-03-28  1:50     ` Guo Ren
2021-03-27 18:06 ` [PATCH v4 3/4] locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32 guoren
2021-03-27 18:43   ` Waiman Long
2021-03-28  1:48     ` Guo Ren
2021-03-29  7:50   ` Peter Zijlstra
2021-03-29  9:41     ` Arnd Bergmann
2021-03-29 11:16       ` Peter Zijlstra
2021-03-29 11:29         ` Peter Zijlstra
2021-03-29 12:52           ` Guo Ren
2021-03-29 13:56             ` Arnd Bergmann
2021-03-30  2:26               ` Guo Ren
2021-03-30  5:51                 ` Anup Patel
2021-03-30  6:26                   ` Guo Ren
2021-03-30  7:11                 ` Arnd Bergmann
2021-03-31  4:18                   ` Guo Ren
2021-03-31  5:33                     ` Paul Campbell
2021-04-05 16:12                       ` Guo Ren
2021-03-31  6:44                     ` Guo Ren [this message]
2021-03-31  7:12                       ` Arnd Bergmann
2021-03-29 11:19     ` Guo Ren
2021-03-29 11:26       ` Peter Zijlstra
2021-03-29 12:01         ` Guo Ren
2021-03-29 12:49           ` Peter Zijlstra
2021-03-30  3:13             ` Guo Ren
2021-03-30  4:54               ` Anup Patel
2021-03-30  6:27                 ` Guo Ren
2021-03-30  8:31               ` David Laight
2021-03-30 14:09               ` Waiman Long
2021-03-31 14:47                 ` Guo Ren
2021-04-05 16:45                 ` Guo Ren
2021-03-30 16:08               ` Peter Zijlstra
2021-03-30 22:35                 ` Stafford Horne
2021-03-31  7:23                   ` Arnd Bergmann
2021-03-31 12:31                     ` Stafford Horne
2021-03-31 15:10                       ` Guo Ren
2021-04-06  8:51                         ` Stafford Horne
2021-04-06  3:50                     ` Guo Ren
2021-04-06  8:56                       ` Stafford Horne
2021-04-07  8:42                         ` Arnd Bergmann
2021-04-07 11:36                           ` Peter Zijlstra
2021-04-07 11:57                             ` Arnd Bergmann
2021-04-07 12:02                             ` Peter Zijlstra
2021-04-05 16:40                 ` Guo Ren
2021-03-31 15:22             ` Guo Ren
2021-04-06  7:15               ` Peter Zijlstra
2021-04-07  9:42                 ` Christoph Hellwig
2021-04-07 14:29                   ` Christoph Müllner
2021-04-07 14:34                     ` Christoph Hellwig
2021-04-07 15:51                     ` Peter Zijlstra
2021-04-07 16:44                       ` Peter Zijlstra
2021-04-07 15:52                     ` Peter Zijlstra
2021-04-07 16:54                       ` Peter Zijlstra
2021-04-07 16:00                     ` Peter Zijlstra
2021-04-07 19:50                       ` Christoph Müllner
2021-04-06 17:24               ` Boqun Feng
2021-04-07  9:26                 ` Peter Zijlstra
2021-03-29 12:13         ` Anup Patel
2021-03-29 12:54           ` Peter Zijlstra
2021-03-27 18:06 ` [PATCH v4 4/4] riscv: Convert custom spinlock/rwlock to generic qspinlock/qrwlock guoren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJF2gTSXY1fCBSZ-Z=8_AcRxoiCOoaNu-5A_JximGJxZY18RzQ@mail.gmail.com' \
    --to=guoren@kernel.org \
    --cc=anup@brainfault.org \
    --cc=arnd@arndb.de \
    --cc=guoren@linux.alibaba.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-csky@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sebastian@breakpoint.cc \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-csky Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-csky/0 linux-csky/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-csky linux-csky/ https://lore.kernel.org/linux-csky \
		linux-csky@vger.kernel.org
	public-inbox-index linux-csky

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-csky


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git