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

Reply via email to