Re: [PATCH 1/7] atomics/tty: add missing atomic_long_t * cast

From: Greg Kroah-Hartman
Date: Tue Jun 05 2018 - 10:21:23 EST


On Tue, Jun 05, 2018 at 02:00:22PM +0200, Peter Zijlstra wrote:
> On Tue, May 29, 2018 at 07:07:40PM +0100, Mark Rutland wrote:
> > In ldsem_cmpxchg a pointer to unsigned long is passed to
> > atomic_long_cmpxchg(), which expects a pointer to atomic_long_t.
>
> > - long tmp = atomic_long_cmpxchg(&sem->count, *old, new);
> > + long tmp = atomic_long_cmpxchg((atomic_long_t *)&sem->count, *old, new);
>
> Needing to cast anything to atomic types is dodgy at best, so I had me a
> look at this code.
>
> Would not the following patch be much better?
>
> ---
> drivers/tty/tty_ldsem.c | 82 ++++++++++++++++++++---------------------------
> include/linux/tty_ldisc.h | 4 +--
> 2 files changed, 37 insertions(+), 49 deletions(-)

At first glance, yes, this does look a lot better, it's less lines at
the least :)

I think Peter Hurley was just trying to hid the atomic mess behind
function calls, which is why he didn't open-code it like you did here.
But this makes it a bit more obvious as to what "magic" is happening, so
I like it.

Care to turn it into a real patch so I can queue it up after 4.18-rc1 is
out? Or do you want me to do that?

thanks,

greg k-h