Messages in this thread | | | Date | Fri, 7 Aug 2020 14:07:59 -0400 (EDT) | From | Mathieu Desnoyers <> | Subject | Re: [PATCH 1/2] membarrier: add MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU |
| |
----- On Aug 7, 2020, at 1:48 PM, Peter Oskolkov posk@posk.io wrote:
> On Thu, Aug 6, 2020 at 10:37 AM Mathieu Desnoyers > <mathieu.desnoyers@efficios.com> wrote: >> [...] >> Also, should this belong to the membarrier or the rseq system call ? It just >> looks like the membarrier happens to implement very similar things for barriers, >> but arguably this is really about rseq. I wonder if we should expose this >> through >> rseq instead, even if we end up using membarrier code. > > Yes, this is more about rseq; on the other hand, the high-level API/behavior > looks closer to that membarrier, and a lot of code will be shared. > > As you are the maintainer for both rseq and membarrier, this is for > you to decide, I guess... :)
Considering that membarrier has been made extensible with the cmd argument, and on the other hand rseq can be extended with "flags", but is currently only about registration/unregistration, I think adding a command to membarrier is indeed a natural approach.
I am not very fond on re-purposing the membarrier flags parameter into a cpu number though. Maybe we should tweak the membarrier system call so it can expect 3 arguments instead ?
int membarrier(int cmd, int flags, int cpu);
where cpu is only used for specific commands.
One thing I find weird about Peter's patch is that it adds a MEMBERRIER_CMD_PRIVATE_EXPEDITED_RSEQ without a corresponding MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_RSEQ. Considering that the SYNC_CORE variant already has its own register command, I find it weird that the RSEQ counterpart does not have one.
Also, do we want to allow a RSEQ | SYNC_CORE private expedited membarrier as well ? If that is the case, then we might want to investigate exposing RSEQ-membarrier as a new membarrier flag rather than as a stand-alone command.
Thanks,
Mathieu
-- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
| |