From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2] Xen/atomic: use static inlines instead of macros Date: Fri, 28 Mar 2014 11:23:54 +0000 Message-ID: <53355BCA.7000807@citrix.com> References: <1396004108-27981-1-git-send-email-andrew.cooper3@citrix.com> <53356575020000780000350B@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53356575020000780000350B@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , Tim Deegan , StefanoStabellini , Ian Campbell , Xen-devel List-Id: xen-devel@lists.xenproject.org On 28/03/14 11:05, Jan Beulich wrote: >>>> On 28.03.14 at 11:55, wrote: >> This is some coverity-inspired tidying. >> >> Coverity has some grief analysing the call sites of atomic_read(). This is >> believed to be a bug in Coverity itself when expanding the nested macros, >> but >> there is no legitimate reason for it to be a macro in the first place. >> >> This patch changes {,_}atomic_{read,set}() from being macros to being static >> inline functions, thus gaining some type safety. >> >> One issue which is not immediately obvious is that the non-atomic variants >> take >> their atomic_t at a different level of indirection to the atomic variants. >> >> This is not suitable for _atomic_set() (when used to initialise an atomic_t) >> which is converted to take its parameter as a pointer. One callsite of >> _atomic_set() is updated, while the other two callsites are updated to >> ATOMIC_INIT(). >> >> Signed-off-by: Andrew Cooper >> CC: Keir Fraser >> CC: Jan Beulich >> CC: Ian Campbell >> CC: Stefano Stabellini >> CC: Tim Deegan >> >> --- >> >> This is compile-tested on arm32 and 64, and functionally tested on x86_64 >> >> v2: spelling fixes in commit message, and remove some redundant brackets > I see you left even the "non-atomic atomic ops" as inline functions, > other than suggested on v1. I'll leave it to Keir to decide whether > the resulting larger change is worthwhile the little benefit. > > Jan > I did explain why I disagreed with leaving the non-atomic as macros. Any optimising compiler will generate the same code from either option, but the static inline functions provide much more informative error messages. ~Andrew