Thread (22 messages) 22 messages, 4 authors, 2018-07-24

[PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem

From: mark.rutland@arm.com (Mark Rutland)
Date: 2018-07-24 13:05:24
Also in: lkml

On Tue, Jul 24, 2018 at 01:13:13PM +0200, Ingo Molnar wrote:
* Mark Rutland [off-list ref] wrote:
quoted
On Tue, Jul 24, 2018 at 11:20:36AM +0200, Peter Zijlstra wrote:
quoted
On Tue, Jul 24, 2018 at 09:15:18AM +0200, Ingo Molnar wrote:
quoted
* Mark Rutland [off-list ref] wrote:
quoted
From: Peter Zijlstra <peterz@infradead.org>

Mark found ldsem_cmpxchg() needed an (atomic_long_t *) cast to keep
working after making the atomic_long interface type safe.

Needing casts is bad form, which made me look at the code. There are no
ld_semaphore::count users outside of these functions so there is no
reason why it can not be an atomic_long_t in the first place, obviating
the need for this cast.

That also ensures the loads use atomic_long_read(), which implies (at
least) READ_ONCE() in order to guarantee single-copy-atomic loads.

When using atomic_long_try_cmpxchg() the ldsem_cmpxchg() wrapper gets
very thin (the only difference is not changing *old on success, which
most callers don't seem to care about).

So rework the whole thing to use atomic_long_t and its accessors
directly.

While there, fixup all the horrible comment styles.

Cc: Peter Hurley <redacted>
Acked-by: Will Deacon <redacted>
Reported-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Andy Shevchenko <redacted>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ingo Molnar <mingo@kernel.org>
---
 drivers/tty/tty_ldsem.c   | 82 ++++++++++++++++++++---------------------------
 include/linux/tty_ldisc.h |  4 +--
 2 files changed, 37 insertions(+), 49 deletions(-)

Note: Greg has queued this via the in the tty tree for v4.19, which can be seen at: 

https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-next&id=5fd691afdf929061c391d897fa627822c3b2fd5a
Can this patch be skipped, or do the others depend on it?
IIRC it depends on it, without this patch you get build issues due to
atomic_long_cmpxchg() getting picky about it's arguments (type safety
improved).
Yup. Without this patch, there will be a build regression at patch 9,
when we move to generated atomic_long_*() wrappers.
Ok, then these bits will have to wait until Greg's tree goes upstream
in about two weeks.
Ok.
Which patches can I apply as a preparatory step?
Patches 2-6 can be applied now.

I guess I should rebase and resend the remaining bits once Greg's tree
goes upstream?

Thanks,
Mark.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help