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

Message ID 20180716113017.3909-2-mark.rutland@arm.com
State New
Headers show
Series
  • atomics: generate atomic headers / instrument arm64
Related show

Commit Message

Mark Rutland July 16, 2018, 11:30 a.m.
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 <peter@hurleysoftware.com>
Acked-by: Will Deacon <will.deacon@arm.com>

Reported-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

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

Mark.

-- 
2.11.0

Comments

Ingo Molnar July 24, 2018, 7:15 a.m. | #1
* Mark Rutland <mark.rutland@arm.com> wrote:

> 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 <peter@hurleysoftware.com>

> Acked-by: Will Deacon <will.deacon@arm.com>

> Reported-by: Mark Rutland <mark.rutland@arm.com>

> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> 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?

Thanks,

	Ingo
Peter Zijlstra July 24, 2018, 9:20 a.m. | #2
On Tue, Jul 24, 2018 at 09:15:18AM +0200, Ingo Molnar wrote:
> 

> * Mark Rutland <mark.rutland@arm.com> wrote:

> 

> > 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 <peter@hurleysoftware.com>

> > Acked-by: Will Deacon <will.deacon@arm.com>

> > Reported-by: Mark Rutland <mark.rutland@arm.com>

> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> > 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).
Mark Rutland July 24, 2018, 9:23 a.m. | #3
On Tue, Jul 24, 2018 at 11:20:36AM +0200, Peter Zijlstra wrote:
> On Tue, Jul 24, 2018 at 09:15:18AM +0200, Ingo Molnar wrote:

> > 

> > * Mark Rutland <mark.rutland@arm.com> wrote:

> > 

> > > 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 <peter@hurleysoftware.com>

> > > Acked-by: Will Deacon <will.deacon@arm.com>

> > > Reported-by: Mark Rutland <mark.rutland@arm.com>

> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> > > 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.

Thanks,
Mark.
Ingo Molnar July 24, 2018, 11:13 a.m. | #4
* Mark Rutland <mark.rutland@arm.com> wrote:

> On Tue, Jul 24, 2018 at 11:20:36AM +0200, Peter Zijlstra wrote:

> > On Tue, Jul 24, 2018 at 09:15:18AM +0200, Ingo Molnar wrote:

> > > 

> > > * Mark Rutland <mark.rutland@arm.com> wrote:

> > > 

> > > > 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 <peter@hurleysoftware.com>

> > > > Acked-by: Will Deacon <will.deacon@arm.com>

> > > > Reported-by: Mark Rutland <mark.rutland@arm.com>

> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> > > > 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.

Which patches can I apply as a preparatory step?

Thanks,

	Ingo
Mark Rutland July 24, 2018, 1:05 p.m. | #5
On Tue, Jul 24, 2018 at 01:13:13PM +0200, Ingo Molnar wrote:
> 

> * Mark Rutland <mark.rutland@arm.com> wrote:

> 

> > On Tue, Jul 24, 2018 at 11:20:36AM +0200, Peter Zijlstra wrote:

> > > On Tue, Jul 24, 2018 at 09:15:18AM +0200, Ingo Molnar wrote:

> > > > 

> > > > * Mark Rutland <mark.rutland@arm.com> wrote:

> > > > 

> > > > > 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 <peter@hurleysoftware.com>

> > > > > Acked-by: Will Deacon <will.deacon@arm.com>

> > > > > Reported-by: Mark Rutland <mark.rutland@arm.com>

> > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> > > > > 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.
Ingo Molnar July 24, 2018, 1:40 p.m. | #6
* Mark Rutland <mark.rutland@arm.com> wrote:

> > 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.


Ok, will apply those.

> I guess I should rebase and resend the remaining bits once Greg's tree

> goes upstream?


Yeah, easiest would be to do that once Greg's and tip:locking/core changes (with 
patches 2-6) both got merged by Linus.

Thanks,

	Ingo
Mark Rutland July 24, 2018, 3:06 p.m. | #7
On Tue, Jul 24, 2018 at 03:40:48PM +0200, Ingo Molnar wrote:
> 

> * Mark Rutland <mark.rutland@arm.com> wrote:

> 

> > > 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.

> 

> Ok, will apply those.


Cheers!

> > I guess I should rebase and resend the remaining bits once Greg's tree

> > goes upstream?

> 

> Yeah, easiest would be to do that once Greg's and tip:locking/core changes (with 

> patches 2-6) both got merged by Linus.


I'll keep an eye out for both of these.

Thanks,
Mark.

Patch

diff --git a/drivers/tty/tty_ldsem.c b/drivers/tty/tty_ldsem.c
index 37a91b3df980..0c98d88f795a 100644
--- a/drivers/tty/tty_ldsem.c
+++ b/drivers/tty/tty_ldsem.c
@@ -74,28 +74,6 @@  struct ldsem_waiter {
 	struct task_struct *task;
 };
 
-static inline long ldsem_atomic_update(long delta, struct ld_semaphore *sem)
-{
-	return atomic_long_add_return(delta, (atomic_long_t *)&sem->count);
-}
-
-/*
- * ldsem_cmpxchg() updates @*old with the last-known sem->count value.
- * Returns 1 if count was successfully changed; @*old will have @new value.
- * Returns 0 if count was not changed; @*old will have most recent sem->count
- */
-static inline int ldsem_cmpxchg(long *old, long new, struct ld_semaphore *sem)
-{
-	long tmp = atomic_long_cmpxchg(&sem->count, *old, new);
-	if (tmp == *old) {
-		*old = new;
-		return 1;
-	} else {
-		*old = tmp;
-		return 0;
-	}
-}
-
 /*
  * Initialize an ldsem:
  */
@@ -109,7 +87,7 @@  void __init_ldsem(struct ld_semaphore *sem, const char *name,
 	debug_check_no_locks_freed((void *)sem, sizeof(*sem));
 	lockdep_init_map(&sem->dep_map, name, key, 0);
 #endif
-	sem->count = LDSEM_UNLOCKED;
+	atomic_long_set(&sem->count, LDSEM_UNLOCKED);
 	sem->wait_readers = 0;
 	raw_spin_lock_init(&sem->wait_lock);
 	INIT_LIST_HEAD(&sem->read_wait);
@@ -122,16 +100,17 @@  static void __ldsem_wake_readers(struct ld_semaphore *sem)
 	struct task_struct *tsk;
 	long adjust, count;
 
-	/* Try to grant read locks to all readers on the read wait list.
+	/*
+	 * Try to grant read locks to all readers on the read wait list.
 	 * Note the 'active part' of the count is incremented by
 	 * the number of readers before waking any processes up.
 	 */
 	adjust = sem->wait_readers * (LDSEM_ACTIVE_BIAS - LDSEM_WAIT_BIAS);
-	count = ldsem_atomic_update(adjust, sem);
+	count = atomic_long_add_return(adjust, &sem->count);
 	do {
 		if (count > 0)
 			break;
-		if (ldsem_cmpxchg(&count, count - adjust, sem))
+		if (atomic_long_try_cmpxchg(&sem->count, &count, count - adjust))
 			return;
 	} while (1);
 
@@ -148,14 +127,15 @@  static void __ldsem_wake_readers(struct ld_semaphore *sem)
 
 static inline int writer_trylock(struct ld_semaphore *sem)
 {
-	/* only wake this writer if the active part of the count can be
+	/*
+	 * Only wake this writer if the active part of the count can be
 	 * transitioned from 0 -> 1
 	 */
-	long count = ldsem_atomic_update(LDSEM_ACTIVE_BIAS, sem);
+	long count = atomic_long_add_return(LDSEM_ACTIVE_BIAS, &sem->count);
 	do {
 		if ((count & LDSEM_ACTIVE_MASK) == LDSEM_ACTIVE_BIAS)
 			return 1;
-		if (ldsem_cmpxchg(&count, count - LDSEM_ACTIVE_BIAS, sem))
+		if (atomic_long_try_cmpxchg(&sem->count, &count, count - LDSEM_ACTIVE_BIAS))
 			return 0;
 	} while (1);
 }
@@ -205,12 +185,16 @@  down_read_failed(struct ld_semaphore *sem, long count, long timeout)
 	/* set up my own style of waitqueue */
 	raw_spin_lock_irq(&sem->wait_lock);
 
-	/* Try to reverse the lock attempt but if the count has changed
+	/*
+	 * Try to reverse the lock attempt but if the count has changed
 	 * so that reversing fails, check if there are are no waiters,
-	 * and early-out if not */
+	 * and early-out if not
+	 */
 	do {
-		if (ldsem_cmpxchg(&count, count + adjust, sem))
+		if (atomic_long_try_cmpxchg(&sem->count, &count, count + adjust)) {
+			count += adjust;
 			break;
+		}
 		if (count > 0) {
 			raw_spin_unlock_irq(&sem->wait_lock);
 			return sem;
@@ -243,12 +227,14 @@  down_read_failed(struct ld_semaphore *sem, long count, long timeout)
 	__set_current_state(TASK_RUNNING);
 
 	if (!timeout) {
-		/* lock timed out but check if this task was just
+		/*
+		 * Lock timed out but check if this task was just
 		 * granted lock ownership - if so, pretend there
-		 * was no timeout; otherwise, cleanup lock wait */
+		 * was no timeout; otherwise, cleanup lock wait.
+		 */
 		raw_spin_lock_irq(&sem->wait_lock);
 		if (waiter.task) {
-			ldsem_atomic_update(-LDSEM_WAIT_BIAS, sem);
+			atomic_long_add_return(-LDSEM_WAIT_BIAS, &sem->count);
 			list_del(&waiter.list);
 			raw_spin_unlock_irq(&sem->wait_lock);
 			put_task_struct(waiter.task);
@@ -273,11 +259,13 @@  down_write_failed(struct ld_semaphore *sem, long count, long timeout)
 	/* set up my own style of waitqueue */
 	raw_spin_lock_irq(&sem->wait_lock);
 
-	/* Try to reverse the lock attempt but if the count has changed
+	/*
+	 * Try to reverse the lock attempt but if the count has changed
 	 * so that reversing fails, check if the lock is now owned,
-	 * and early-out if so */
+	 * and early-out if so.
+	 */
 	do {
-		if (ldsem_cmpxchg(&count, count + adjust, sem))
+		if (atomic_long_try_cmpxchg(&sem->count, &count, count + adjust))
 			break;
 		if ((count & LDSEM_ACTIVE_MASK) == LDSEM_ACTIVE_BIAS) {
 			raw_spin_unlock_irq(&sem->wait_lock);
@@ -303,7 +291,7 @@  down_write_failed(struct ld_semaphore *sem, long count, long timeout)
 	}
 
 	if (!locked)
-		ldsem_atomic_update(-LDSEM_WAIT_BIAS, sem);
+		atomic_long_add_return(-LDSEM_WAIT_BIAS, &sem->count);
 	list_del(&waiter.list);
 	raw_spin_unlock_irq(&sem->wait_lock);
 
@@ -324,7 +312,7 @@  static int __ldsem_down_read_nested(struct ld_semaphore *sem,
 
 	lockdep_acquire_read(sem, subclass, 0, _RET_IP_);
 
-	count = ldsem_atomic_update(LDSEM_READ_BIAS, sem);
+	count = atomic_long_add_return(LDSEM_READ_BIAS, &sem->count);
 	if (count <= 0) {
 		lock_stat(sem, contended);
 		if (!down_read_failed(sem, count, timeout)) {
@@ -343,7 +331,7 @@  static int __ldsem_down_write_nested(struct ld_semaphore *sem,
 
 	lockdep_acquire(sem, subclass, 0, _RET_IP_);
 
-	count = ldsem_atomic_update(LDSEM_WRITE_BIAS, sem);
+	count = atomic_long_add_return(LDSEM_WRITE_BIAS, &sem->count);
 	if ((count & LDSEM_ACTIVE_MASK) != LDSEM_ACTIVE_BIAS) {
 		lock_stat(sem, contended);
 		if (!down_write_failed(sem, count, timeout)) {
@@ -370,10 +358,10 @@  int __sched ldsem_down_read(struct ld_semaphore *sem, long timeout)
  */
 int ldsem_down_read_trylock(struct ld_semaphore *sem)
 {
-	long count = sem->count;
+	long count = atomic_long_read(&sem->count);
 
 	while (count >= 0) {
-		if (ldsem_cmpxchg(&count, count + LDSEM_READ_BIAS, sem)) {
+		if (atomic_long_try_cmpxchg(&sem->count, &count, count + LDSEM_READ_BIAS)) {
 			lockdep_acquire_read(sem, 0, 1, _RET_IP_);
 			lock_stat(sem, acquired);
 			return 1;
@@ -396,10 +384,10 @@  int __sched ldsem_down_write(struct ld_semaphore *sem, long timeout)
  */
 int ldsem_down_write_trylock(struct ld_semaphore *sem)
 {
-	long count = sem->count;
+	long count = atomic_long_read(&sem->count);
 
 	while ((count & LDSEM_ACTIVE_MASK) == 0) {
-		if (ldsem_cmpxchg(&count, count + LDSEM_WRITE_BIAS, sem)) {
+		if (atomic_long_try_cmpxchg(&sem->count, &count, count + LDSEM_WRITE_BIAS)) {
 			lockdep_acquire(sem, 0, 1, _RET_IP_);
 			lock_stat(sem, acquired);
 			return 1;
@@ -417,7 +405,7 @@  void ldsem_up_read(struct ld_semaphore *sem)
 
 	lockdep_release(sem, 1, _RET_IP_);
 
-	count = ldsem_atomic_update(-LDSEM_READ_BIAS, sem);
+	count = atomic_long_add_return(-LDSEM_READ_BIAS, &sem->count);
 	if (count < 0 && (count & LDSEM_ACTIVE_MASK) == 0)
 		ldsem_wake(sem);
 }
@@ -431,7 +419,7 @@  void ldsem_up_write(struct ld_semaphore *sem)
 
 	lockdep_release(sem, 1, _RET_IP_);
 
-	count = ldsem_atomic_update(-LDSEM_WRITE_BIAS, sem);
+	count = atomic_long_add_return(-LDSEM_WRITE_BIAS, &sem->count);
 	if (count < 0)
 		ldsem_wake(sem);
 }
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 1ef64d4ad887..840894ca3fc0 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -119,13 +119,13 @@ 
 
 #include <linux/fs.h>
 #include <linux/wait.h>
-
+#include <linux/atomic.h>
 
 /*
  * the semaphore definition
  */
 struct ld_semaphore {
-	long			count;
+	atomic_long_t		count;
 	raw_spinlock_t		wait_lock;
 	unsigned int		wait_readers;
 	struct list_head	read_wait;