Merge Question on asm-ppc*/rwsem.h

Merge Question on asm-ppc*/rwsem.h

[Jon Loeliger]

Paul (or anyone, really),

I am not too familiar with some of the low-level machine
details that might (do!) influence some of the semaphore
implementations.  I was hoping I might get you to give
me a quick nudge in the right direction here.

Here is a brief example of the differences found in
the ppc and pp64 versions of asm-ppc*/rwsem.h:

    (ppc is -, and ppc64 is + here)

    @@ -69,9 +74,7 @@
      */
     static inline void __down_read(struct rw_semaphore *sem)
     {
    -       if (atomic_inc_return((atomic_t *)(&sem->count)) > 0)
    -               smp_wmb();
    -       else
    +       if (unlikely(atomic_inc_return((atomic_t *)(&sem->count)) <= 0))
                    rwsem_down_read_failed(sem);
     }

If we add likely() to the PPC version, and then invert it,
we can get effectively this:

    static inline void __down_read(struct rw_semaphore *sem)
    {
            if (unlikely(atomic_inc_return((atomic_t *)(&sem->count)) <= 0))
                    rwsem_down_read_failed(sem);
    #ifndef __powerpc64__
            else
                    smp_wmb();
    #endif
    }

Which begs the question, what is the "else smp_wmb();" clause
really doing for us in the ppc32 case, and can we either
get rid of it, or is it safe to add to the ppc64 case?

Thanks,
jdl

Re: Merge Question on asm-ppc*/rwsem.h

[Paul Mackerras]

Jon Loeliger writes:

> Which begs the question, what is the "else smp_wmb();" clause
> really doing for us in the ppc32 case, and can we either
> get rid of it, or is it safe to add to the ppc64 case?

I think the difference is that the atomic ops that return a value also
have include a barrier on ppc64, but don't on ppc32 (although they
probably should).  IOW I think the merged case should follow ppc64
here.

Paul.

Re: Merge Question on asm-ppc*/rwsem.h

[Jon Loeliger]

So sprach Paul Mackerras:
> 
> I think the difference is that the atomic ops that return a value also
> have include a barrier on ppc64, but don't on ppc32 (although they
> probably should).  IOW I think the merged case should follow ppc64
> here.

Excellent!

I will submit that patch in the style of ppc64 early this week.

Thanks for the feedback,
jdl