On Fri, Jul 17, 2015 at 06:19:54PM +0100, Waiman Long wrote: > On 07/17/2015 05:40 AM, Will Deacon wrote: > > On Fri, Jul 17, 2015 at 01:07:28AM +0100, Waiman Long wrote: > >> I have a minor nit. The atomic_add_return block is repeated with > >> "s/atomic_add_return/.../". Perhaps some more comments to delineate the > >> blocks more visibly will make this patch easier to read. > > Yeah, I agree that it's pretty hard going, but I don't have any great > > suggestions to solve that. I could add an extra blank line + comment > > before the start of each section, if you like? Example snippet below. > > > > Will > > > > --->8 > > > > [...] > > #endif /* atomic_sub_return_relaxed */ > > > > > > /* atomic_xchg_relaxed */ > > #ifndef atomic_xchg_relaxed > > #define atomic_xchg_relaxed atomic_xchg > > #define atomic_xchg_acquire atomic_xchg > > #define atomic_xchg_release atomic_xchg > > > > #else /* atomic_xchg_relaxed */ > > > > #ifndef atomic_xchg_acquire > > #define atomic_xchg_acquire(...) \ > > __atomic_op_acquire(int, atomic_xchg, __VA_ARGS__) > > #endif > > > > #ifndef atomic_xchg_release > > #define atomic_xchg_release(...) \ > > __atomic_op_release(int, atomic_xchg, __VA_ARGS__) > > #endif > > > > #ifndef atomic_xchg > > #define atomic_xchg(...) \ > > __atomic_op_fence(int, atomic_xchg, __VA_ARGS__) > > #endif > > #endif /* atomic_xchg_relaxed */ > > > > > > /* atomic_cmpxchg_relaxed */ > > [...] > Something like > > /* BEGIN atomc_xchg_relax */ > ... > /* END atomic_xchg_relax */ > > may help. > > Alternatively, I sometimes add a line separator like > > /*===================[ atomic_xchg_relax ]====================*/ Hmm, I think we're straying into cosmetic preferences here and I can imagine some people taking objection to the above, especially as they don't appear to be in common use in mainline. Will -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html