Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
On Tue, Jul 24, 2018 at 03:40:48PM +0200, Ingo Molnar wrote: > > * Mark Rutland 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.
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
On Tue, Jul 24, 2018 at 03:40:48PM +0200, Ingo Molnar wrote: > > * Mark Rutland 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.
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
* Mark Rutland 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
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
* Mark Rutland 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
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
On Tue, Jul 24, 2018 at 01:13:13PM +0200, Ingo Molnar wrote: > > * Mark Rutland 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 wrote: > > > > > > > > > From: Peter Zijlstra > > > > > > > > > > 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 > > > > > Acked-by: Will Deacon > > > > > Reported-by: Mark Rutland > > > > > Reviewed-by: Andy Shevchenko > > > > > Signed-off-by: Peter Zijlstra (Intel) > > > > > Signed-off-by: Mark Rutland > > > > > Cc: Ingo Molnar > > > > > --- > > > > > 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=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.
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
On Tue, Jul 24, 2018 at 01:13:13PM +0200, Ingo Molnar wrote: > > * Mark Rutland 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 wrote: > > > > > > > > > From: Peter Zijlstra > > > > > > > > > > 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 > > > > > Acked-by: Will Deacon > > > > > Reported-by: Mark Rutland > > > > > Reviewed-by: Andy Shevchenko > > > > > Signed-off-by: Peter Zijlstra (Intel) > > > > > Signed-off-by: Mark Rutland > > > > > Cc: Ingo Molnar > > > > > --- > > > > > 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=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.
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
* Mark Rutland 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 wrote: > > > > > > > From: Peter Zijlstra > > > > > > > > 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 > > > > Acked-by: Will Deacon > > > > Reported-by: Mark Rutland > > > > Reviewed-by: Andy Shevchenko > > > > Signed-off-by: Peter Zijlstra (Intel) > > > > Signed-off-by: Mark Rutland > > > > Cc: Ingo Molnar > > > > --- > > > > 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=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
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
* Mark Rutland 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 wrote: > > > > > > > From: Peter Zijlstra > > > > > > > > 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 > > > > Acked-by: Will Deacon > > > > Reported-by: Mark Rutland > > > > Reviewed-by: Andy Shevchenko > > > > Signed-off-by: Peter Zijlstra (Intel) > > > > Signed-off-by: Mark Rutland > > > > Cc: Ingo Molnar > > > > --- > > > > 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=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
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
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 wrote: > > > > > From: Peter Zijlstra > > > > > > 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 > > > Acked-by: Will Deacon > > > Reported-by: Mark Rutland > > > Reviewed-by: Andy Shevchenko > > > Signed-off-by: Peter Zijlstra (Intel) > > > Signed-off-by: Mark Rutland > > > Cc: Ingo Molnar > > > --- > > > 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=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.
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
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 wrote: > > > > > From: Peter Zijlstra > > > > > > 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 > > > Acked-by: Will Deacon > > > Reported-by: Mark Rutland > > > Reviewed-by: Andy Shevchenko > > > Signed-off-by: Peter Zijlstra (Intel) > > > Signed-off-by: Mark Rutland > > > Cc: Ingo Molnar > > > --- > > > 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=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.
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
On Tue, Jul 24, 2018 at 09:15:18AM +0200, Ingo Molnar wrote: > > * Mark Rutland wrote: > > > From: Peter Zijlstra > > > > 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 > > Acked-by: Will Deacon > > Reported-by: Mark Rutland > > Reviewed-by: Andy Shevchenko > > Signed-off-by: Peter Zijlstra (Intel) > > Signed-off-by: Mark Rutland > > Cc: Ingo Molnar > > --- > > 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=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).
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
On Tue, Jul 24, 2018 at 09:15:18AM +0200, Ingo Molnar wrote: > > * Mark Rutland wrote: > > > From: Peter Zijlstra > > > > 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 > > Acked-by: Will Deacon > > Reported-by: Mark Rutland > > Reviewed-by: Andy Shevchenko > > Signed-off-by: Peter Zijlstra (Intel) > > Signed-off-by: Mark Rutland > > Cc: Ingo Molnar > > --- > > 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=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).
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
* Mark Rutland wrote: > From: Peter Zijlstra > > 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 > Acked-by: Will Deacon > Reported-by: Mark Rutland > Reviewed-by: Andy Shevchenko > Signed-off-by: Peter Zijlstra (Intel) > Signed-off-by: Mark Rutland > Cc: Ingo Molnar > --- > 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=5fd691afdf929061c391d897fa627822c3b2fd5a Can this patch be skipped, or do the others depend on it? Thanks, Ingo
Re: [PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
* Mark Rutland wrote: > From: Peter Zijlstra > > 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 > Acked-by: Will Deacon > Reported-by: Mark Rutland > Reviewed-by: Andy Shevchenko > Signed-off-by: Peter Zijlstra (Intel) > Signed-off-by: Mark Rutland > Cc: Ingo Molnar > --- > 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=5fd691afdf929061c391d897fa627822c3b2fd5a Can this patch be skipped, or do the others depend on it? Thanks, Ingo
[PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
From: Peter Zijlstra
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
Acked-by: Will Deacon
Reported-by: Mark Rutland
Reviewed-by: Andy Shevchenko
Signed-off-by: Peter Zijlstra (Intel)
Signed-off-by: Mark Rutland
Cc: Ingo Molnar
---
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=5fd691afdf929061c391d897fa627822c3b2fd5a
Mark.
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 *)>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(>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(>dep_map, name, key, 0);
#endif
- sem->count = LDSEM_UNLOCKED;
+ atomic_long_set(>count, LDSEM_UNLOCKED);
sem->wait_readers = 0;
raw_spin_lock_init(>wait_lock);
INIT_LIST_HEAD(>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, >count);
do {
if (count > 0)
break;
- if (ldsem_cmpxchg(, count - adjust, sem))
+ if (atomic_long_try_cmpxchg(>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, >count);
do {
if ((count & LDSEM_ACTIVE_MASK) == LDSEM_ACTIVE_BIAS)
return 1;
- if (ldsem_cmpxchg(, count - LDSEM_ACTIVE_BIAS, sem))
+ if (atomic_long_try_cmpxchg(>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(>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 {
-
[PATCHv4 01/12] atomic/tty: Fix up atomic abuse in ldsem
From: Peter Zijlstra
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
Acked-by: Will Deacon
Reported-by: Mark Rutland
Reviewed-by: Andy Shevchenko
Signed-off-by: Peter Zijlstra (Intel)
Signed-off-by: Mark Rutland
Cc: Ingo Molnar
---
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=5fd691afdf929061c391d897fa627822c3b2fd5a
Mark.
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 *)>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(>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(>dep_map, name, key, 0);
#endif
- sem->count = LDSEM_UNLOCKED;
+ atomic_long_set(>count, LDSEM_UNLOCKED);
sem->wait_readers = 0;
raw_spin_lock_init(>wait_lock);
INIT_LIST_HEAD(>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, >count);
do {
if (count > 0)
break;
- if (ldsem_cmpxchg(, count - adjust, sem))
+ if (atomic_long_try_cmpxchg(>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, >count);
do {
if ((count & LDSEM_ACTIVE_MASK) == LDSEM_ACTIVE_BIAS)
return 1;
- if (ldsem_cmpxchg(, count - LDSEM_ACTIVE_BIAS, sem))
+ if (atomic_long_try_cmpxchg(>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(>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 {
-
