From mboxrd@z Thu Jan 1 00:00:00 1970 From: gang.chen@asianux.com (Chen Gang) Date: Fri, 27 Sep 2013 19:36:11 +0800 Subject: [PATCH v2] ARM: include: asm: use 'long long' instead of 'u64' within atomic.h In-Reply-To: <20130927110618.GB9520@mudshark.cambridge.arm.com> References: <523D7DC7.8030603@asianux.com> <20130924093041.GB15119@mudshark.cambridge.arm.com> <5242498F.8060407@asianux.com> <20130925160746.GC14502@mudshark.cambridge.arm.com> <5243953E.6010405@asianux.com> <20130926100422.GE2855@mudshark.cambridge.arm.com> <5244148F.6080405@asianux.com> <20130927110618.GB9520@mudshark.cambridge.arm.com> Message-ID: <52456DAB.8060900@asianux.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 09/27/2013 07:06 PM, Will Deacon wrote: > On Thu, Sep 26, 2013 at 12:03:43PM +0100, Chen Gang wrote: >> On 09/26/2013 06:04 PM, Will Deacon wrote: >>> On Thu, Sep 26, 2013 at 03:00:30AM +0100, Chen Gang wrote: >>>> On 09/26/2013 12:07 AM, Will Deacon wrote: >>>>> On Wed, Sep 25, 2013 at 03:25:19AM +0100, Chen Gang wrote: >>>>>> atomic* value is signed value, and atomic* functions need also process >>>>>> signed value (parameter value, and return value), so 32-bit arm need >>>>>> use 'long long' instead of 'u64'. >>>>>> >>>>>> After replacement, it will also fix a bug for atomic64_add_negative(): >>>>>> "u64 is never less than 0". >>>>>> >>>>>> The modifications are: >>>>>> >>>>>> in vim, use "1,% s/\/long long/g" command. >>>>>> remove '__aligned(8)' which is useless for 64-bit. >>>>>> be sure of 80 column limitation after replacement. >>>>>> >>>>>> >>>>>> Signed-off-by: Chen Gang >>>>> >>>>> Looks better to me, thanks. While you're here, we could also replace the use >>>>> of `unsigned long' with `int' for the 32-bit atomics, then the whole header >>>>> is consistent with the generic types. >>>>> >>>>> Will >>>>> >>>>> >>>> >>>> Hmm... at least, it seems we can let "use 'int' for 32-bit atomics" in >>>> another patch. >>> >>> Sure, it can be a follow-up to this one. >>> >> >> OK, if it is really a useful patch too, I will follow-up. > > It's a cosmetic change to bring the signedness of our 32-bit atomics in-line > with your proposal for 64-bit atomics. > >> 'unsigned long' can be used as a register related variable, it is always >> 32-bit under 32-bit machine, and always 64-bit under 64-bit machine. > > Not necessarily (c.f. ILP32). > >> So can use it for both arm and arm64, for arm, it can not cause issue, >> and for arm64, it is also OK (if changed to 'int' under arm64, may cause >> real issue). > > arm and arm64 have different instructions sets. There's no way the inline > assembly used to implement the atomic operations can be made portable > between them. > Hmm... if you like, I will send a follow-up patch for it, the reason is: It is belong to internal implementation, not belong to API, so if it is harmless, better to follow related maintainers' 'hobby' or 'tastes', it will let the related code more clearer. :-) I will wait 1-2 days to let another reviewers check, if no reply, I will send 2 patches for it. (if you already applied the "u64 -> long long" patch, please let me know, and I will send one patch enough). > Will > > Thanks. -- Chen Gang