All of
 help / color / mirror / Atom feed
From: Manfred Spraul <>
To: LKML <>,
	Davidlohr Bueso <>,
	Waiman Long <>
Cc:, Andrew Morton <>,
	Peter Zijlstra <>,
	Manfred Spraul <>
Subject: [PATCH 0/5] V3: Clarify/standardize memory barriers for ipc
Date: Sun, 20 Oct 2019 14:33:00 +0200	[thread overview]
Message-ID: <> (raw)


Updated series, based on input from Davidlohr and Peter Zijlstra:

- I've dropped the documentation update for wake_q_add, as what it
  states is normal: When you call a function and pass a parameter
  to a structure, you as caller are responsible to ensure that the 
  parameter is valid, and remains valid for the duration of the
  function call, including any tearing due to memory reordering.
  In addition, I've switched ipc to wake_q_add_safe().

- The patch to Documentation/memory_barriers.txt now as first change.
  @Davidlohr: You proposed to have 2 paragraphs: First, one for
  add/subtract, then one for failed cmpxchg. I didn't like that:
  We have one rule (can be combined with non-mb RMW ops), and then
  examples what are non-mb RMW ops. Listing special cases just ask
  for issues later.
  What I don't know is if there should be examples at all in
  Documentation/memory_barriers, or just
  "See Documentation/atomic_t.txt for examples of RMW ops that
  do not contain a memory barrier"

- For the memory barrier pairs in ipc/<whatever>, I have now added
  /* See ABC_BARRIER for purpose/pairing */ as standard comment,
  and then a block near the relevant structure where purpose, pairing
  races, ... are explained. I think this makes it easier to read,
  compared to adding it to both the _release and _acquire branches.


The memory barriers in ipc are not properly documented, and at least
for some architectures insufficient:
Reading the xyz->status is only a control barrier, thus
smp_acquire__after_ctrl_dep() was missing in mqueue.c and msg.c
sem.c contained a full smp_mb(), which is not required.

Patch 1: Documentation for smp_mb__{before,after}_atomic().

Patch 2: Remove code duplication inside ipc/mqueue.c

Patch 3-5: Update the ipc code, especially add missing
           smp_mb__after_ctrl_dep() and switch to wake_q_add_safe().

Clarify that smp_mb__{before,after}_atomic() are compatible with all
RMW atomic operations, not just the operations that do not return a value.

Open issues:
- More testing. I did some tests, but doubt that the tests would be
  sufficient to show issues with regards to incorrect memory barriers.

What do you think?


             reply	other threads:[~2019-10-20 12:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-20 12:33 Manfred Spraul [this message]
2019-10-20 12:33 ` [PATCH 1/5] smp_mb__{before,after}_atomic(): Update Documentation Manfred Spraul
2019-11-01 16:49   ` Will Deacon
2019-11-06 19:23     ` Manfred Spraul
2019-11-07 11:22       ` Will Deacon
2019-10-20 12:33 ` [PATCH 2/5] ipc/mqueue.c: Remove duplicated code Manfred Spraul
2019-10-22 22:43   ` Andrew Morton
2019-10-20 12:33 ` [PATCH 3/5] ipc/mqueue.c: Update/document memory barriers Manfred Spraul
2019-10-20 12:33 ` [PATCH 4/5] ipc/msg.c: Update and document " Manfred Spraul
2019-10-20 12:33 ` [PATCH 5/5] ipc/sem.c: Document and update " Manfred Spraul

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:

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

  git send-email \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.