[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <843af7b5-8917-e9e3-de27-cb328f53fb70@efficios.com>
Date: Wed, 7 Dec 2022 12:07:46 -0500
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Michal Clapinski <mclapinski@...gle.com>,
"Paul E. McKenney" <paulmck@...nel.org>
Cc: Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Andrei Vagin <avagin@...il.com>, Shuah Khan <shuah@...nel.org>,
linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 1/2] sched/membarrier: Introduce
MEMBARRIER_CMD_GET_REGISTRATIONS
On 2022-12-07 11:43, Michal Clapinski wrote:
> Provide a method to query previously issued registrations.
>
> Signed-off-by: Michal Clapinski <mclapinski@...gle.com>
> ---
> include/uapi/linux/membarrier.h | 4 ++++
> kernel/sched/membarrier.c | 39 ++++++++++++++++++++++++++++++++-
> 2 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/membarrier.h b/include/uapi/linux/membarrier.h
> index 737605897f36..5f3ad6d5be6f 100644
> --- a/include/uapi/linux/membarrier.h
> +++ b/include/uapi/linux/membarrier.h
> @@ -137,6 +137,9 @@
> * @MEMBARRIER_CMD_SHARED:
> * Alias to MEMBARRIER_CMD_GLOBAL. Provided for
> * header backward compatibility.
> + * @MEMBARRIER_CMD_GET_REGISTRATIONS:
> + * Returns a bitmask of previously issued
> + * registration commands.
> *
> * Command to be passed to the membarrier system call. The commands need to
> * be a single bit each, except for MEMBARRIER_CMD_QUERY which is assigned to
> @@ -153,6 +156,7 @@ enum membarrier_cmd {
> MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE = (1 << 6),
> MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ = (1 << 7),
> MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_RSEQ = (1 << 8),
> + MEMBARRIER_CMD_GET_REGISTRATIONS = (1 << 9),
>
> /* Alias for header backward compatibility. */
> MEMBARRIER_CMD_SHARED = MEMBARRIER_CMD_GLOBAL,
> diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c
> index 0c5be7ebb1dc..2ad881d07752 100644
> --- a/kernel/sched/membarrier.c
> +++ b/kernel/sched/membarrier.c
> @@ -159,7 +159,8 @@
> | MEMBARRIER_CMD_PRIVATE_EXPEDITED \
> | MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED \
> | MEMBARRIER_PRIVATE_EXPEDITED_SYNC_CORE_BITMASK \
> - | MEMBARRIER_PRIVATE_EXPEDITED_RSEQ_BITMASK)
> + | MEMBARRIER_PRIVATE_EXPEDITED_RSEQ_BITMASK \
> + | MEMBARRIER_CMD_GET_REGISTRATIONS)
>
> static void ipi_mb(void *info)
> {
> @@ -540,6 +541,40 @@ static int membarrier_register_private_expedited(int flags)
> return 0;
> }
>
> +static int membarrier_get_registrations(void)
> +{
> + struct task_struct *p = current;
> + struct mm_struct *mm = p->mm;
> + int registrations_mask = 0, membarrier_state, i;
> + static const int states[] = {
> + MEMBARRIER_STATE_GLOBAL_EXPEDITED |
> + MEMBARRIER_STATE_GLOBAL_EXPEDITED_READY,
What is the purpose of checking for the _READY state flag as well here ?
> + MEMBARRIER_STATE_PRIVATE_EXPEDITED |
> + MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY,
> + MEMBARRIER_STATE_PRIVATE_EXPEDITED_SYNC_CORE |
> + MEMBARRIER_STATE_PRIVATE_EXPEDITED_SYNC_CORE_READY,
> + MEMBARRIER_STATE_PRIVATE_EXPEDITED_RSEQ |
> + MEMBARRIER_STATE_PRIVATE_EXPEDITED_RSEQ_READY
> + };
> + static const int registration_cmds[] = {
> + MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED,
> + MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED,
> + MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE,
> + MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_RSEQ
> + };
> + BUILD_BUG_ON(ARRAY_SIZE(states) != ARRAY_SIZE(registration_cmds));
> +
> + membarrier_state = atomic_read(&mm->membarrier_state);
> + for (i = 0; i < ARRAY_SIZE(states); ++i) {
> + if (membarrier_state & states[i]) {
The mask will match if either of the flags to match are set. Is that
your intent ?
> + registrations_mask |= registration_cmds[i];
> + membarrier_state &= ~states[i];
So I understand that those _READY flags are there purely for making sure
we clear the membarrier_state for validation that they have all been
checked with the following WARN_ON_ONCE(). Am I on the right track ?
> + }
> + }
> + WARN_ON_ONCE(membarrier_state != 0);
Thanks,
Mathieu
> + return registrations_mask;
> +}
> +
> /**
> * sys_membarrier - issue memory barriers on a set of threads
> * @cmd: Takes command values defined in enum membarrier_cmd.
> @@ -623,6 +658,8 @@ SYSCALL_DEFINE3(membarrier, int, cmd, unsigned int, flags, int, cpu_id)
> return membarrier_private_expedited(MEMBARRIER_FLAG_RSEQ, cpu_id);
> case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_RSEQ:
> return membarrier_register_private_expedited(MEMBARRIER_FLAG_RSEQ);
> + case MEMBARRIER_CMD_GET_REGISTRATIONS:
> + return membarrier_get_registrations();
> default:
> return -EINVAL;
> }
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
Powered by blists - more mailing lists