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@google.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; }