| Message ID | 1468565287-32581-1-git-send-email-czuzu@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Corneliu, On 15/07/16 07:48, Corneliu ZUZU wrote: > Move duplicate macros between asm-arm/arm32/atomic.h and asm-arm/arm64/atomic.h > to asm-arm/atomic.h. Adjust README.LinuxPrimitives in the process. > Also empty line fixes. Why do you add empty lines? They are not necessary nor coding style requirement nor in Linux headers. Please don't introduce changes without a valid reason. > Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changed since v3: > * update README.LinuxPrimitives file > --- > xen/arch/arm/README.LinuxPrimitives | 20 ++++++++++++++++++++ > xen/include/asm-arm/arm32/atomic.h | 15 ++------------- > xen/include/asm-arm/arm64/atomic.h | 15 ++------------- > xen/include/asm-arm/atomic.h | 14 ++++++++++++++ > 4 files changed, 38 insertions(+), 26 deletions(-) > > diff --git a/xen/arch/arm/README.LinuxPrimitives b/xen/arch/arm/README.LinuxPrimitives > index 3115f51..4906593 100644 > --- a/xen/arch/arm/README.LinuxPrimitives > +++ b/xen/arch/arm/README.LinuxPrimitives > @@ -23,6 +23,16 @@ atomics: last sync @ v3.16-rc6 (last commit: 8715466b6027) > > linux/arch/arm64/include/asm/atomic.h xen/include/asm-arm/arm64/atomic.h > > +The following functions were taken from Linux: > + atomic_add(), atomic_add_return(), atomic_sub(), atomic_sub_return(), > + atomic_cmpxchg(), __atomic_add_unless() > + > +Also, the following macros which were in the meantime moved to asm-arm/atomic.h: > + atomic_xchg(v, new), atomic_inc(v), atomic_dec(v), > + atomic_inc_and_test(v), atomic_dec_and_test(v), > + atomic_inc_return(v), atomic_dec_return(v), > + atomic_sub_and_test(i, v), atomic_add_negative(i,v) I would drop the second paragraph because they are moved outside of arm/*atomic.h and you turn into inline. So IHMO they are not sync-able anymore. > + > --------------------------------------------------------------------- > > mem*: last sync @ v3.16-rc6 (last commit: d875c9b37240) > @@ -91,6 +101,16 @@ atomics: last sync @ v3.16-rc6 (last commit: 030d0178bdbd) > > linux/arch/arm/include/asm/atomic.h xen/include/asm-arm/arm32/atomic.h > > +The following functions were taken from Linux: > + atomic_add(), atomic_add_return(), atomic_sub(), atomic_sub_return(), > + atomic_cmpxchg(), __atomic_add_unless() > + > +Also, the following macros which were in the meantime moved to asm-arm/atomic.h: > + atomic_xchg(v, new), atomic_inc(v), atomic_dec(v), > + atomic_inc_and_test(v), atomic_dec_and_test(v), > + atomic_inc_return(v), atomic_dec_return(v), > + atomic_sub_and_test(i, v), atomic_add_negative(i,v) Ditto. Regards,
On 7/15/2016 12:28 PM, Julien Grall wrote: > Hi Corneliu, > > On 15/07/16 07:48, Corneliu ZUZU wrote: >> Move duplicate macros between asm-arm/arm32/atomic.h and >> asm-arm/arm64/atomic.h >> to asm-arm/atomic.h. Adjust README.LinuxPrimitives in the process. >> Also empty line fixes. > > Why do you add empty lines? A little picky today, aren't we? :-) I added empty lines: - before local variables block - between header comment and #ifndef guard because IMO -it looks better- and is consistent with the format of -most of- the rest of the files (thus I assumed that's the preferred format and that deviations from that are honest mistakes :-)). > They are not necessary nor coding style requirement nor in Linux > headers. Please don't introduce changes without a valid reason. I just peeked in the Linux source tree and I noticed there are also headers there with an empty line between the file-comment and #ifndef. Plus, this is the Xen code-base and I don't see why I'd look in the Linux source tree to determine rules that apply to the Xen source-tree. I didn't expect for a rule like this to -need- putting in CODING_STYLE. Anyway, they'll be dropped in v5 if it really matters. > > >> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> >> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> >> --- >> Changed since v3: >> * update README.LinuxPrimitives file >> --- >> xen/arch/arm/README.LinuxPrimitives | 20 ++++++++++++++++++++ >> xen/include/asm-arm/arm32/atomic.h | 15 ++------------- >> xen/include/asm-arm/arm64/atomic.h | 15 ++------------- >> xen/include/asm-arm/atomic.h | 14 ++++++++++++++ >> 4 files changed, 38 insertions(+), 26 deletions(-) >> >> diff --git a/xen/arch/arm/README.LinuxPrimitives >> b/xen/arch/arm/README.LinuxPrimitives >> index 3115f51..4906593 100644 >> --- a/xen/arch/arm/README.LinuxPrimitives >> +++ b/xen/arch/arm/README.LinuxPrimitives >> @@ -23,6 +23,16 @@ atomics: last sync @ v3.16-rc6 (last commit: >> 8715466b6027) >> >> linux/arch/arm64/include/asm/atomic.h >> xen/include/asm-arm/arm64/atomic.h >> >> +The following functions were taken from Linux: >> + atomic_add(), atomic_add_return(), atomic_sub(), >> atomic_sub_return(), >> + atomic_cmpxchg(), __atomic_add_unless() >> + >> +Also, the following macros which were in the meantime moved to >> asm-arm/atomic.h: >> + atomic_xchg(v, new), atomic_inc(v), atomic_dec(v), >> + atomic_inc_and_test(v), atomic_dec_and_test(v), >> + atomic_inc_return(v), atomic_dec_return(v), >> + atomic_sub_and_test(i, v), atomic_add_negative(i,v) > > I would drop the second paragraph because they are moved outside of > arm/*atomic.h and you turn into inline. So IHMO they are not sync-able > anymore. > Ok. > >> + >> --------------------------------------------------------------------- >> >> mem*: last sync @ v3.16-rc6 (last commit: d875c9b37240) >> @@ -91,6 +101,16 @@ atomics: last sync @ v3.16-rc6 (last commit: >> 030d0178bdbd) >> >> linux/arch/arm/include/asm/atomic.h xen/include/asm-arm/arm32/atomic.h >> >> +The following functions were taken from Linux: >> + atomic_add(), atomic_add_return(), atomic_sub(), >> atomic_sub_return(), >> + atomic_cmpxchg(), __atomic_add_unless() >> + >> +Also, the following macros which were in the meantime moved to >> asm-arm/atomic.h: >> + atomic_xchg(v, new), atomic_inc(v), atomic_dec(v), >> + atomic_inc_and_test(v), atomic_dec_and_test(v), >> + atomic_inc_return(v), atomic_dec_return(v), >> + atomic_sub_and_test(i, v), atomic_add_negative(i,v) > > Ditto. > > Regards, Ack. Corneliu.
On 15/07/16 10:55, Corneliu ZUZU wrote: > On 7/15/2016 12:28 PM, Julien Grall wrote: >> Hi Corneliu, >> >> On 15/07/16 07:48, Corneliu ZUZU wrote: >>> Move duplicate macros between asm-arm/arm32/atomic.h and >>> asm-arm/arm64/atomic.h >>> to asm-arm/atomic.h. Adjust README.LinuxPrimitives in the process. >>> Also empty line fixes. >> >> Why do you add empty lines? > > A little picky today, aren't we? :-) Same as usual ;). [...] >> They are not necessary nor coding style requirement nor in Linux >> headers. Please don't introduce changes without a valid reason. > > I just peeked in the Linux source tree and I noticed there are also > headers there with an empty line between the file-comment and #ifndef. > Plus, this is the Xen code-base and I don't see why I'd look in the > Linux source tree to determine rules that apply to the Xen source-tree. Because files taken from Linux respect Linux coding style. I did the mistake on other files to diverge (such as the SMMU code) and it was a pain to re-sync it later. So I prefer to have a strict rule on it, even for cosmetic changes. Regards,
On 7/15/2016 1:06 PM, Julien Grall wrote: > > > On 15/07/16 10:55, Corneliu ZUZU wrote: >> On 7/15/2016 12:28 PM, Julien Grall wrote: >>> Hi Corneliu, >>> >>> On 15/07/16 07:48, Corneliu ZUZU wrote: >>>> Move duplicate macros between asm-arm/arm32/atomic.h and >>>> asm-arm/arm64/atomic.h >>>> to asm-arm/atomic.h. Adjust README.LinuxPrimitives in the process. >>>> Also empty line fixes. >>> >>> Why do you add empty lines? >> >> A little picky today, aren't we? :-) > > Same as usual ;). > > [...] > >>> They are not necessary nor coding style requirement nor in Linux >>> headers. Please don't introduce changes without a valid reason. >> >> I just peeked in the Linux source tree and I noticed there are also >> headers there with an empty line between the file-comment and #ifndef. >> Plus, this is the Xen code-base and I don't see why I'd look in the >> Linux source tree to determine rules that apply to the Xen source-tree. > > Because files taken from Linux respect Linux coding style. > > I did the mistake on other files to diverge (such as the SMMU code) > and it was a pain to re-sync it later. So I prefer to have a strict > rule on it, even for cosmetic changes. > > Regards, Makes sense and much better than "Please don't introduce changes without a valid reason". ;-) Corneliu.
diff --git a/xen/arch/arm/README.LinuxPrimitives b/xen/arch/arm/README.LinuxPrimitives index 3115f51..4906593 100644 --- a/xen/arch/arm/README.LinuxPrimitives +++ b/xen/arch/arm/README.LinuxPrimitives @@ -23,6 +23,16 @@ atomics: last sync @ v3.16-rc6 (last commit: 8715466b6027) linux/arch/arm64/include/asm/atomic.h xen/include/asm-arm/arm64/atomic.h +The following functions were taken from Linux: + atomic_add(), atomic_add_return(), atomic_sub(), atomic_sub_return(), + atomic_cmpxchg(), __atomic_add_unless() + +Also, the following macros which were in the meantime moved to asm-arm/atomic.h: + atomic_xchg(v, new), atomic_inc(v), atomic_dec(v), + atomic_inc_and_test(v), atomic_dec_and_test(v), + atomic_inc_return(v), atomic_dec_return(v), + atomic_sub_and_test(i, v), atomic_add_negative(i,v) + --------------------------------------------------------------------- mem*: last sync @ v3.16-rc6 (last commit: d875c9b37240) @@ -91,6 +101,16 @@ atomics: last sync @ v3.16-rc6 (last commit: 030d0178bdbd) linux/arch/arm/include/asm/atomic.h xen/include/asm-arm/arm32/atomic.h +The following functions were taken from Linux: + atomic_add(), atomic_add_return(), atomic_sub(), atomic_sub_return(), + atomic_cmpxchg(), __atomic_add_unless() + +Also, the following macros which were in the meantime moved to asm-arm/atomic.h: + atomic_xchg(v, new), atomic_inc(v), atomic_dec(v), + atomic_inc_and_test(v), atomic_dec_and_test(v), + atomic_inc_return(v), atomic_dec_return(v), + atomic_sub_and_test(i, v), atomic_add_negative(i,v) + --------------------------------------------------------------------- mem*: last sync @ v3.16-rc6 (last commit: d98b90ea22b0) diff --git a/xen/include/asm-arm/arm32/atomic.h b/xen/include/asm-arm/arm32/atomic.h index 7ec712f..78de60f 100644 --- a/xen/include/asm-arm/arm32/atomic.h +++ b/xen/include/asm-arm/arm32/atomic.h @@ -8,6 +8,7 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ + #ifndef __ARCH_ARM_ARM32_ATOMIC__ #define __ARCH_ARM_ARM32_ATOMIC__ @@ -147,20 +148,8 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u) return oldval; } -#define atomic_xchg(v, new) (xchg(&((v)->counter), new)) - -#define atomic_inc(v) atomic_add(1, v) -#define atomic_dec(v) atomic_sub(1, v) - -#define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0) -#define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0) -#define atomic_inc_return(v) (atomic_add_return(1, v)) -#define atomic_dec_return(v) (atomic_sub_return(1, v)) -#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0) - -#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0) - #endif /* __ARCH_ARM_ARM32_ATOMIC__ */ + /* * Local variables: * mode: C diff --git a/xen/include/asm-arm/arm64/atomic.h b/xen/include/asm-arm/arm64/atomic.h index b49219e..d640bef 100644 --- a/xen/include/asm-arm/arm64/atomic.h +++ b/xen/include/asm-arm/arm64/atomic.h @@ -19,6 +19,7 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see <http://www.gnu.org/licenses/>. */ + #ifndef __ARCH_ARM_ARM64_ATOMIC #define __ARCH_ARM_ARM64_ATOMIC @@ -113,8 +114,6 @@ static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new) return oldval; } -#define atomic_xchg(v, new) (xchg(&((v)->counter), new)) - static inline int __atomic_add_unless(atomic_t *v, int a, int u) { int c, old; @@ -125,18 +124,8 @@ static inline int __atomic_add_unless(atomic_t *v, int a, int u) return c; } -#define atomic_inc(v) atomic_add(1, v) -#define atomic_dec(v) atomic_sub(1, v) - -#define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0) -#define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0) -#define atomic_inc_return(v) (atomic_add_return(1, v)) -#define atomic_dec_return(v) (atomic_sub_return(1, v)) -#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0) - -#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0) - #endif + /* * Local variables: * mode: C diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h index 29ab265..32771e9 100644 --- a/xen/include/asm-arm/atomic.h +++ b/xen/include/asm-arm/atomic.h @@ -138,7 +138,21 @@ static inline void _atomic_set(atomic_t *v, int i) # error "unknown ARM variant" #endif +#define atomic_inc(v) atomic_add(1, v) +#define atomic_dec(v) atomic_sub(1, v) + +#define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0) +#define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0) +#define atomic_inc_return(v) (atomic_add_return(1, v)) +#define atomic_dec_return(v) (atomic_sub_return(1, v)) +#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0) + +#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0) + +#define atomic_xchg(v, new) (xchg(&((v)->counter), new)) + #endif /* __ARCH_ARM_ATOMIC__ */ + /* * Local variables: * mode: C