Re: [patch] (resend) hwpmc [7/13]

[Aggelos Economopoulos <aoiko@xxxxxxxxxxxxxx>]

On Tuesday 27 November 2007, Matthew Dillon wrote:
>     This is a pretty big patch set, and it makes modifications around
>     all over the kernel source.  It's going to take a while to digest
>     this.

Careful reading from other people is what I'm hoping for. Just because I'm 
submitting it doesn't mean I believe the code (even the parts I've tested) is 
bug-free!

>     I don't think I like those atomic_*() instructions... they aren't 
>     well documented and they aren't even close to being portable.

Well, as for the 'well documented' part, I couldn't agree more. Would
something like this explanation (from the Oct/2000 commit message) be 
sufficient?

"- Expand the set of atomic operations to optionally include memory barriers
  in most of the atomic operations.  Now for these operations, you can
  use the normal atomic operation, you can use the operation with a read
  barrier, or you can use the operation with a write barrier.  The function
  names follow the same semantics used in the ia64 instruction set.  An
  atomic operation with a read barrier has the extra suffix 'acq', due to
  it having "acquire" semantics.  An atomic operation with a write barrier
  has the extra suffix 'rel'.  These suffixes are inserted between the
  name of the operation to perform and the typename.  For example, the
  atomic_add_int() function now has 3 variants:
  - atomic_add_int() - this is the same as the previous function
  - atomic_add_acq_int() - this function combines the add operation with a
    read memory barrier
  - atomic_add_rel_int() - this function combines the add operation with a
    write memory barrier
- Add 'ptr' to the list of types that we can perform atomic operations
  on.  This allows one to do atomic operations on uintptr_t's.  This is
  useful in the mutex code, for example, because the actual mutex lock is
  a pointer.
- Add two new operations for doing loads and stores with memory barriers.
  The new load operations use a read barrier before the load, and the
  new store operations use a write barrier after the load.  For example,
  atomic_load_acq_int() will atomically load an integer as well as
  enforcing a read barrier."

Now, about them not being portable, I simply don't get what you mean. If 
you're refering to memory-ordering semantics on other architectures, I think 
they are adequate; indeed they seem to be written with ia64 in mind, which 
according to Paul McKenney (see Table 1 in this article 
http://www.linuxjournal.com/article/8212) are very relaxed. If anything, one 
could argue that they are *too* portable and you're better of just 
open-coding the memory barriers (although these macros, by emphasizing the 
connection with the atomic op, may make the code clearer for some people).

That said, the names are not, IMHO, very intuitive.

Can you clarify?

Thanks,
Aggelos