Re: [PATCH 2/2] membarrier: riscv: Provide core serializing command

[Mathieu Desnoyers]

On 2023-11-29 16:25, Andrea Parri wrote:
> > > As regards the Fixes: tag, I guess it boils down to what we want or we
> > > need to take for commit "riscv: Support membarrier private cmd".  :-)
> >
> > I'm not seeing this commit in the Linux master branch, am I missing
> > something ?
>
> I don't think you're missing something: I was wondering "what/where is
> this commit"?  Sorry for the confusion.
>
>
> > > FWIW, a quick git-log search confirmed that MEMBARRIER has been around
> > > for quite some time in the RISC-V world (though I'm not familiar with
> > > any of its mainstream uses): commit 1464d00b27b2 says (at least) since
> > > 93917ad50972 ("RISC-V: Add support for restartable sequence").  I am
> > > currently inclined to pick the latter commit (and check it w/ Palmer),
> > > but other suggestions are welcome.
> >
> > Supporting membarrier private expedited is not optional since Linux 4.14:
> >
> > see kernel/sched/core.c:
> >
> >                  membarrier_switch_mm(rq, prev->active_mm, next->mm);
> >                  /*
> >                   * sys_membarrier() requires an smp_mb() between setting
> >                   * rq->curr / membarrier_switch_mm() and returning to userspace.
> >                   *
> >                   * The below provides this either through switch_mm(), or in
> >                   * case 'prev->active_mm == next->mm' through
> >                   * finish_task_switch()'s mmdrop().
> >                   */
> >                  switch_mm_irqs_off(prev->active_mm, next->mm, next);
> >
> > Failure to provide the required barrier is a bug in the architecture's
> > switch_mm implementation when CONFIG_MEMBARRIER=y.
> >
> > We should probably introduce a new
> > Documentation/features/sched/membarrier/arch-support.txt
> > to clarify this requirement.
> >
> > Userspace code such as liburcu [1] heavily relies on membarrier private
> > expedited (when available) to speed up RCU read-side critical sections.
> > Various DNS servers, including BIND 9, use liburcu.
>
> Thanks for the information.
>
> So I should probably stick to 93917ad50972, which apparently selected
> CONFIG_MEMBARRIER on RISC-V, for the Fixes: tag in question.

I think it goes further than that, because you can explicitly
CONFIG_MEMBARRIER=y, see init/Kconfig:

config MEMBARRIER
        bool "Enable membarrier() system call" if EXPERT
        default y
        help
          Enable the membarrier() system call that allows issuing memory
          barriers across all running threads, which can be used to distribute
          the cost of user-space memory barriers asymmetrically by transforming
          pairs of memory barriers into pairs consisting of membarrier() and a
          compiler barrier.

          If unsure, say Y.

Before 1464d00b27b2, riscv just happened to set it to =n in the defconfig.

I suspect the initial port of riscv merged after v4.14 was already broken.

> I'll look into adding the membarrier feature you mention (as a final/
> follow-up patch), unless you or someone else want to take care of it.

I'll be happy to review it :)

Thanks,

Mathieu

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com