diff mbox series

Fix the 64-bit macro definition of mips architecture

Message ID 20210422072609.9938-1-sujiaxun@uniontech.com
State Changes Requested
Headers show
Series Fix the 64-bit macro definition of mips architecture | expand

Commit Message

sujiaxun April 22, 2021, 7:26 a.m. UTC 
https://github.com/torvalds/linux/blob/master/arch/mips/include/uapi/asm/shmbuf.h
The mips 64-bit macro definition in the kernel is "__mips64",
 and the mips 64-bit macro definition in the ltp is "__arch64__".

Signed-off-by: sujiaxun <sujiaxun@uniontech.com>
---
 include/lapi/msgbuf.h | 2 +-
 include/lapi/sembuf.h | 2 +-
 include/lapi/shmbuf.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

--
2.20.1

Comments

Petr Vorel April 26, 2021, 5:42 a.m. UTC | #1 
Hi sujiaxun,

[ Cc: Viresh, the original author ]

> https://github.com/torvalds/linux/blob/master/arch/mips/include/uapi/asm/shmbuf.h
> The mips 64-bit macro definition in the kernel is "__mips64",
>  and the mips 64-bit macro definition in the ltp is "__arch64__".

> Signed-off-by: sujiaxun <sujiaxun@uniontech.com>
> ---
>  include/lapi/msgbuf.h | 2 +-
>  include/lapi/sembuf.h | 2 +-
>  include/lapi/shmbuf.h | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

> diff --git a/include/lapi/msgbuf.h b/include/lapi/msgbuf.h
> index f3277270d..f010695f1 100644
> --- a/include/lapi/msgbuf.h
> +++ b/include/lapi/msgbuf.h
> @@ -17,7 +17,7 @@
>  #if defined(__mips__)
>  #define HAVE_MSQID64_DS

> -#if defined(__arch64__)
> +#if defined(__mips64)
So __arch64__ is not defined for mips 64 bit? (as it's defined for sparc 64bit?)
__mips64 is obviously correct and better readable, but is it really required?
(you can check it with: echo | gcc -dM -E -).

Kind regards,
Petr
Viresh Kumar April 26, 2021, 5:55 a.m. UTC | #2 
On 26-04-21, 07:42, Petr Vorel wrote:
> Hi sujiaxun,
> 
> [ Cc: Viresh, the original author ]
> 
> > https://github.com/torvalds/linux/blob/master/arch/mips/include/uapi/asm/shmbuf.h
> > The mips 64-bit macro definition in the kernel is "__mips64",
> >  and the mips 64-bit macro definition in the ltp is "__arch64__".
> 
> > Signed-off-by: sujiaxun <sujiaxun@uniontech.com>
> > ---
> >  include/lapi/msgbuf.h | 2 +-
> >  include/lapi/sembuf.h | 2 +-
> >  include/lapi/shmbuf.h | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> > diff --git a/include/lapi/msgbuf.h b/include/lapi/msgbuf.h
> > index f3277270d..f010695f1 100644
> > --- a/include/lapi/msgbuf.h
> > +++ b/include/lapi/msgbuf.h
> > @@ -17,7 +17,7 @@
> >  #if defined(__mips__)
> >  #define HAVE_MSQID64_DS
> 
> > -#if defined(__arch64__)
> > +#if defined(__mips64)
> So __arch64__ is not defined for mips 64 bit? (as it's defined for sparc 64bit?)
> __mips64 is obviously correct and better readable, but is it really required?

I am not sure what you meant by "is it really required?" The #ifdef hackery here
? It is as can be seen in include/uapi/asm-generic/shmbuf.h in Linux source.

> (you can check it with: echo | gcc -dM -E -).
Petr Vorel April 26, 2021, 6:17 a.m. UTC | #3 
Hi Viresh,

...
> > > -#if defined(__arch64__)
> > > +#if defined(__mips64)
> > So __arch64__ is not defined for mips 64 bit? (as it's defined for sparc 64bit?)
> > __mips64 is obviously correct and better readable, but is it really required?

> I am not sure what you meant by "is it really required?" The #ifdef hackery here
> ? It is as can be seen in include/uapi/asm-generic/shmbuf.h in Linux source.
I mean if #if defined(__mips__) && defined(__arch64__) detect 64bit mips
the patch would not be needed (although IMHO __mips64 is more descriptive than
__arch64__, for which you need to search for which architecture it was defined).

But I'm not sure myself if __arch64__ is defined for mips 64bit.

Kind regards,
Petr
Viresh Kumar April 26, 2021, 6:23 a.m. UTC | #4 
On 26-04-21, 08:17, Petr Vorel wrote:
> Hi Viresh,
> 
> ...
> > > > -#if defined(__arch64__)
> > > > +#if defined(__mips64)
> > > So __arch64__ is not defined for mips 64 bit? (as it's defined for sparc 64bit?)
> > > __mips64 is obviously correct and better readable, but is it really required?
> 
> > I am not sure what you meant by "is it really required?" The #ifdef hackery here
> > ? It is as can be seen in include/uapi/asm-generic/shmbuf.h in Linux source.
> I mean if #if defined(__mips__) && defined(__arch64__) detect 64bit mips
> the patch would not be needed (although IMHO __mips64 is more descriptive than
> __arch64__, for which you need to search for which architecture it was defined).

Ahh, right.

Actually the kernel has this instead:

#if __BITS_PER_LONG == 64

> But I'm not sure myself if __arch64__ is defined for mips 64bit.

Neither do I.
sujiaxun April 26, 2021, 7 a.m. UTC | #5 
On 2021/4/26 下午2:23, Viresh Kumar wrote:
> On 26-04-21, 08:17, Petr Vorel wrote:
>> Hi Viresh,
>>
>> ...
>>>>> -#if defined(__arch64__)
>>>>> +#if defined(__mips64)
>>>> So __arch64__ is not defined for mips 64 bit? (as it's defined for sparc 64bit?)
>>>> __mips64 is obviously correct and better readable, but is it really required?
>>
>>> I am not sure what you meant by "is it really required?" The #ifdef hackery here
>>> ? It is as can be seen in include/uapi/asm-generic/shmbuf.h in Linux source.
>> I mean if #if defined(__mips__) && defined(__arch64__) detect 64bit mips
>> the patch would not be needed (although IMHO __mips64 is more descriptive than
>> __arch64__, for which you need to search for which architecture it was defined).
> 
> Ahh, right.
> 
> Actually the kernel has this instead:
> 
> #if __BITS_PER_LONG == 64
> 
>> But I'm not sure myself if __arch64__ is defined for mips 64bit.
> 
> Neither do I.
> 
uos@uos-PC:~$ echo |gcc -dM -E - | grep -i arch
#define _MIPS_ARCH "mips64r2"
#define _MIPS_ARCH_MIPS64R2 1
uos@uos-PC:~$ echo |gcc -dM -E - | grep -i mips64
#define _MIPS_ISA _MIPS_ISA_MIPS64
#define _MIPS_TUNE "mips64r2"
#define _MIPS_TUNE_MIPS64R2 1
#define _MIPS_ARCH "mips64r2"
#define _MIPS_ARCH_MIPS64R2 1
#define __mips64 1
uos@uos-PC:~$ uname  -m
mips64

The mips architecture gcc has no built-in __arch64__, only __mips64 
definitions. Of course, "__BITS_PER_LONG == 64" can also be used, but I 
think it is better to use __mips64 in the mips architecture.
Viresh Kumar April 26, 2021, 7:03 a.m. UTC | #6 
On 26-04-21, 15:00, sujiaxun wrote:
> uos@uos-PC:~$ echo |gcc -dM -E - | grep -i arch
> #define _MIPS_ARCH "mips64r2"
> #define _MIPS_ARCH_MIPS64R2 1
> uos@uos-PC:~$ echo |gcc -dM -E - | grep -i mips64
> #define _MIPS_ISA _MIPS_ISA_MIPS64
> #define _MIPS_TUNE "mips64r2"
> #define _MIPS_TUNE_MIPS64R2 1
> #define _MIPS_ARCH "mips64r2"
> #define _MIPS_ARCH_MIPS64R2 1
> #define __mips64 1
> uos@uos-PC:~$ uname  -m
> mips64
> 
> The mips architecture gcc has no built-in __arch64__, only __mips64
> definitions. Of course, "__BITS_PER_LONG == 64" can also be used, but I
> think it is better to use __mips64 in the mips architecture.

Hmm, I will rather try to do what the kernel source code does, i.e.
use __BITS_PER_LONG here instead.
sujiaxun April 26, 2021, 7:53 a.m. UTC | #7 
On 2021/4/26 下午3:03, Viresh Kumar wrote:
> On 26-04-21, 15:00, sujiaxun wrote:
>> uos@uos-PC:~$ echo |gcc -dM -E - | grep -i arch
>> #define _MIPS_ARCH "mips64r2"
>> #define _MIPS_ARCH_MIPS64R2 1
>> uos@uos-PC:~$ echo |gcc -dM -E - | grep -i mips64
>> #define _MIPS_ISA _MIPS_ISA_MIPS64
>> #define _MIPS_TUNE "mips64r2"
>> #define _MIPS_TUNE_MIPS64R2 1
>> #define _MIPS_ARCH "mips64r2"
>> #define _MIPS_ARCH_MIPS64R2 1
>> #define __mips64 1
>> uos@uos-PC:~$ uname  -m
>> mips64
>>
>> The mips architecture gcc has no built-in __arch64__, only __mips64
>> definitions. Of course, "__BITS_PER_LONG == 64" can also be used, but I
>> think it is better to use __mips64 in the mips architecture.
> 
> Hmm, I will rather try to do what the kernel source code does, i.e.
> use __BITS_PER_LONG here instead.
> 
I resubmitted a patch and changed "__arch64__" to "#if __BITS_PER_LONG 
== 64", the link is: 
https://patchwork.ozlabs.org/project/ltp/patch/20210426074812.27798-1-sujiaxun@uniontech.com 
/
Viresh Kumar April 26, 2021, 8:12 a.m. UTC | #8 
On 26-04-21, 15:53, sujiaxun wrote:
> 
> 
> On 2021/4/26 下午3:03, Viresh Kumar wrote:
> > On 26-04-21, 15:00, sujiaxun wrote:
> > > uos@uos-PC:~$ echo |gcc -dM -E - | grep -i arch
> > > #define _MIPS_ARCH "mips64r2"
> > > #define _MIPS_ARCH_MIPS64R2 1
> > > uos@uos-PC:~$ echo |gcc -dM -E - | grep -i mips64
> > > #define _MIPS_ISA _MIPS_ISA_MIPS64
> > > #define _MIPS_TUNE "mips64r2"
> > > #define _MIPS_TUNE_MIPS64R2 1
> > > #define _MIPS_ARCH "mips64r2"
> > > #define _MIPS_ARCH_MIPS64R2 1
> > > #define __mips64 1
> > > uos@uos-PC:~$ uname  -m
> > > mips64
> > > 
> > > The mips architecture gcc has no built-in __arch64__, only __mips64
> > > definitions. Of course, "__BITS_PER_LONG == 64" can also be used, but I
> > > think it is better to use __mips64 in the mips architecture.
> > 
> > Hmm, I will rather try to do what the kernel source code does, i.e.
> > use __BITS_PER_LONG here instead.
> > 
> I resubmitted a patch and changed "__arch64__" to "#if __BITS_PER_LONG ==
> 64", the link is: https://patchwork.ozlabs.org/project/ltp/patch/20210426074812.27798-1-sujiaxun@uniontech.com

You should have cc'd me directly :(

I don't have that patch in my inbox..

Though the patch looks fine.
sujiaxun April 26, 2021, 8:28 a.m. UTC | #9 
在 2021/4/26 下午4:12, Viresh Kumar 写道:
> On 26-04-21, 15:53, sujiaxun wrote:
>>
>>
>> On 2021/4/26 下午3:03, Viresh Kumar wrote:
>>> On 26-04-21, 15:00, sujiaxun wrote:
>>>> uos@uos-PC:~$ echo |gcc -dM -E - | grep -i arch
>>>> #define _MIPS_ARCH "mips64r2"
>>>> #define _MIPS_ARCH_MIPS64R2 1
>>>> uos@uos-PC:~$ echo |gcc -dM -E - | grep -i mips64
>>>> #define _MIPS_ISA _MIPS_ISA_MIPS64
>>>> #define _MIPS_TUNE "mips64r2"
>>>> #define _MIPS_TUNE_MIPS64R2 1
>>>> #define _MIPS_ARCH "mips64r2"
>>>> #define _MIPS_ARCH_MIPS64R2 1
>>>> #define __mips64 1
>>>> uos@uos-PC:~$ uname  -m
>>>> mips64
>>>>
>>>> The mips architecture gcc has no built-in __arch64__, only __mips64
>>>> definitions. Of course, "__BITS_PER_LONG == 64" can also be used, but I
>>>> think it is better to use __mips64 in the mips architecture.
>>>
>>> Hmm, I will rather try to do what the kernel source code does, i.e.
>>> use __BITS_PER_LONG here instead.
>>>
>> I resubmitted a patch and changed "__arch64__" to "#if __BITS_PER_LONG ==
>> 64", the link is: https://patchwork.ozlabs.org/project/ltp/patch/20210426074812.27798-1-sujiaxun@uniontech.com
> 
> You should have cc'd me directly :(
> 
> I don't have that patch in my inbox..
> 
> Though the patch looks fine.
> 
Sorry, what should I do now?
Viresh Kumar April 26, 2021, 8:44 a.m. UTC | #10 
On 26-04-21, 16:28, sujiaxun wrote:
> 
> 
> 在 2021/4/26 下午4:12, Viresh Kumar 写道:
> > On 26-04-21, 15:53, sujiaxun wrote:
> > > 
> > > 
> > > On 2021/4/26 下午3:03, Viresh Kumar wrote:
> > > > On 26-04-21, 15:00, sujiaxun wrote:
> > > > > uos@uos-PC:~$ echo |gcc -dM -E - | grep -i arch
> > > > > #define _MIPS_ARCH "mips64r2"
> > > > > #define _MIPS_ARCH_MIPS64R2 1
> > > > > uos@uos-PC:~$ echo |gcc -dM -E - | grep -i mips64
> > > > > #define _MIPS_ISA _MIPS_ISA_MIPS64
> > > > > #define _MIPS_TUNE "mips64r2"
> > > > > #define _MIPS_TUNE_MIPS64R2 1
> > > > > #define _MIPS_ARCH "mips64r2"
> > > > > #define _MIPS_ARCH_MIPS64R2 1
> > > > > #define __mips64 1
> > > > > uos@uos-PC:~$ uname  -m
> > > > > mips64
> > > > > 
> > > > > The mips architecture gcc has no built-in __arch64__, only __mips64
> > > > > definitions. Of course, "__BITS_PER_LONG == 64" can also be used, but I
> > > > > think it is better to use __mips64 in the mips architecture.
> > > > 
> > > > Hmm, I will rather try to do what the kernel source code does, i.e.
> > > > use __BITS_PER_LONG here instead.
> > > > 
> > > I resubmitted a patch and changed "__arch64__" to "#if __BITS_PER_LONG ==
> > > 64", the link is: https://patchwork.ozlabs.org/project/ltp/patch/20210426074812.27798-1-sujiaxun@uniontech.com
> > 
> > You should have cc'd me directly :(
> > 
> > I don't have that patch in my inbox..
> > 
> > Though the patch looks fine.
> > 
> Sorry, what should I do now?

Just reply to the email (without deleting any stuff), and add me in cc
and mention in the top of the email like.

+Viresh.
Petr Vorel April 26, 2021, 1:09 p.m. UTC | #11 
> On 26-04-21, 15:00, sujiaxun wrote:
> > uos@uos-PC:~$ echo |gcc -dM -E - | grep -i arch
> > #define _MIPS_ARCH "mips64r2"
> > #define _MIPS_ARCH_MIPS64R2 1
> > uos@uos-PC:~$ echo |gcc -dM -E - | grep -i mips64
> > #define _MIPS_ISA _MIPS_ISA_MIPS64
> > #define _MIPS_TUNE "mips64r2"
> > #define _MIPS_TUNE_MIPS64R2 1
> > #define _MIPS_ARCH "mips64r2"
> > #define _MIPS_ARCH_MIPS64R2 1
> > #define __mips64 1
> > uos@uos-PC:~$ uname  -m
> > mips64
Thanks for verification!

> > The mips architecture gcc has no built-in __arch64__, only __mips64
> > definitions. Of course, "__BITS_PER_LONG == 64" can also be used, but I
> > think it is better to use __mips64 in the mips architecture.

> Hmm, I will rather try to do what the kernel source code does, i.e.
> use __BITS_PER_LONG here instead.
+1

Kind regards,
Petr
diff mbox series

Patch

diff --git a/include/lapi/msgbuf.h b/include/lapi/msgbuf.h
index f3277270d..f010695f1 100644
--- a/include/lapi/msgbuf.h
+++ b/include/lapi/msgbuf.h
@@ -17,7 +17,7 @@ 
 #if defined(__mips__)
 #define HAVE_MSQID64_DS

-#if defined(__arch64__)
+#if defined(__mips64)
 /*
  * The msqid64_ds structure for the MIPS architecture.
  * Note extra padding because this structure is passed back and forth
diff --git a/include/lapi/sembuf.h b/include/lapi/sembuf.h
index 4ef0483a0..58ad9dff5 100644
--- a/include/lapi/sembuf.h
+++ b/include/lapi/sembuf.h
@@ -24,7 +24,7 @@ 
  * Pad space is left for 2 miscellaneous 64-bit values on mips64,
  * but used for the upper 32 bit of the time values on mips32.
  */
-#if defined(__arch64__)
+#if defined(__mips64)
 struct semid64_ds {
 	struct ipc64_perm sem_perm;		/* permissions .. see ipc.h */
 	long		 sem_otime;		/* last semop time */
diff --git a/include/lapi/shmbuf.h b/include/lapi/shmbuf.h
index 28ee33620..fe405ffe8 100644
--- a/include/lapi/shmbuf.h
+++ b/include/lapi/shmbuf.h
@@ -27,7 +27,7 @@ 
  * data structure when moving to 64-bit time_t.
  */

-#if defined(__arch64__)
+#if defined(__mips64)
 struct shmid64_ds {
 	struct ipc64_perm	shm_perm;	/* operation perms */
 	size_t			shm_segsz;	/* size of segment (bytes) */