'COUNTER' and other same kind macros are too common to use, and easy to
get conflict with other modules. So add prefix for them.
And it is UAPI, so only change it within linux kernel.
The related warning (allmodconfig with score):
CC [M] drivers/md/raid1.o
In file included from drivers/md/raid1.c:42:0:
drivers/md/bitmap.h:93:0: warning: "COUNTER" redefined
#define COUNTER(x) (((bitmap_counter_t) x) & COUNTER_MAX)
^
In file included from ./arch/score/include/asm/ptrace.h:4:0,
from include/linux/sched.h:31,
from include/linux/blkdev.h:4,
from drivers/md/raid1.c:36:
./arch/score/include/uapi/asm/ptrace.h:13:0: note: this is the location of the previous definition
#define COUNTER 38
Signed-off-by: Chen Gang <[email protected]>
---
arch/score/include/uapi/asm/ptrace.h | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/arch/score/include/uapi/asm/ptrace.h b/arch/score/include/uapi/asm/ptrace.h
index f59771a..56e2de0 100644
--- a/arch/score/include/uapi/asm/ptrace.h
+++ b/arch/score/include/uapi/asm/ptrace.h
@@ -4,16 +4,29 @@
#define PTRACE_GETREGS 12
#define PTRACE_SETREGS 13
-#define PC 32
-#define CONDITION 33
-#define ECR 34
-#define EMA 35
-#define CEH 36
-#define CEL 37
-#define COUNTER 38
-#define LDCR 39
-#define STCR 40
-#define PSR 41
+#if !defined(__KERNEL__) || !defined(__linux__)
+#define PC 32
+#define CONDITION 33
+#define ECR 34
+#define EMA 35
+#define CEH 36
+#define CEL 37
+#define COUNTER 38
+#define LDCR 39
+#define STCR 40
+#define PSR 41
+#else
+#define SCORE_PC 32
+#define SCORE_CONDITION 33
+#define SCORE_ECR 34
+#define SCORE_EMA 35
+#define SCORE_CEH 36
+#define SCORE_CEL 37
+#define SCORE_COUNTER 38
+#define SCORE_LDCR 39
+#define SCORE_STCR 40
+#define SCORE_PSR 41
+#endif
#define SINGLESTEP16_INSN 0x7006
#define SINGLESTEP32_INSN 0x840C8000
--
1.9.2.459.g68773ac
On 06/24/2014 06:08 AM, Chen Gang wrote:
> 'COUNTER' and other same kind macros are too common to use, and easy to
> get conflict with other modules. So add prefix for them.
>
> And it is UAPI, so only change it within linux kernel.
>
> The related warning (allmodconfig with score):
>
> CC [M] drivers/md/raid1.o
> In file included from drivers/md/raid1.c:42:0:
> drivers/md/bitmap.h:93:0: warning: "COUNTER" redefined
> #define COUNTER(x) (((bitmap_counter_t) x) & COUNTER_MAX)
> ^
> In file included from ./arch/score/include/asm/ptrace.h:4:0,
> from include/linux/sched.h:31,
> from include/linux/blkdev.h:4,
> from drivers/md/raid1.c:36:
> ./arch/score/include/uapi/asm/ptrace.h:13:0: note: this is the location of the previous definition
> #define COUNTER 38
>
>
> Signed-off-by: Chen Gang <[email protected]>
> ---
> arch/score/include/uapi/asm/ptrace.h | 33 +++++++++++++++++++++++----------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/arch/score/include/uapi/asm/ptrace.h b/arch/score/include/uapi/asm/ptrace.h
> index f59771a..56e2de0 100644
> --- a/arch/score/include/uapi/asm/ptrace.h
> +++ b/arch/score/include/uapi/asm/ptrace.h
> @@ -4,16 +4,29 @@
> #define PTRACE_GETREGS 12
> #define PTRACE_SETREGS 13
>
> -#define PC 32
> -#define CONDITION 33
> -#define ECR 34
> -#define EMA 35
> -#define CEH 36
> -#define CEL 37
> -#define COUNTER 38
> -#define LDCR 39
> -#define STCR 40
> -#define PSR 41
> +#if !defined(__KERNEL__) || !defined(__linux__)
> +#define PC 32
> +#define CONDITION 33
> +#define ECR 34
> +#define EMA 35
> +#define CEH 36
> +#define CEL 37
> +#define COUNTER 38
> +#define LDCR 39
> +#define STCR 40
> +#define PSR 41
> +#else
> +#define SCORE_PC 32
> +#define SCORE_CONDITION 33
> +#define SCORE_ECR 34
> +#define SCORE_EMA 35
> +#define SCORE_CEH 36
> +#define SCORE_CEL 37
> +#define SCORE_COUNTER 38
> +#define SCORE_LDCR 39
> +#define SCORE_STCR 40
> +#define SCORE_PSR 41
> +#endif
>
> #define SINGLESTEP16_INSN 0x7006
> #define SINGLESTEP32_INSN 0x840C8000
>
That looks weird ... not sure if that is a good solution either.
Are those defines actually used anywhere ? They don't seem to be used in the kernel.
Side note - an 'a' got missing in your headline.
And I'd suggest to abbreviate it to something like
score/uapi: ptrace.h:
Thanks,
Guenter
On Tue, 24 Jun 2014, Guenter Roeck wrote:
> > diff --git a/arch/score/include/uapi/asm/ptrace.h
> > b/arch/score/include/uapi/asm/ptrace.h
> > index f59771a..56e2de0 100644
> > --- a/arch/score/include/uapi/asm/ptrace.h
> > +++ b/arch/score/include/uapi/asm/ptrace.h
> > @@ -4,16 +4,29 @@
> > #define PTRACE_GETREGS 12
> > #define PTRACE_SETREGS 13
> >
> > -#define PC 32
> > -#define CONDITION 33
> > -#define ECR 34
> > -#define EMA 35
> > -#define CEH 36
> > -#define CEL 37
> > -#define COUNTER 38
> > -#define LDCR 39
> > -#define STCR 40
> > -#define PSR 41
> > +#if !defined(__KERNEL__) || !defined(__linux__)
> > +#define PC 32
> > +#define CONDITION 33
> > +#define ECR 34
> > +#define EMA 35
> > +#define CEH 36
> > +#define CEL 37
> > +#define COUNTER 38
> > +#define LDCR 39
> > +#define STCR 40
> > +#define PSR 41
> > +#else
> > +#define SCORE_PC 32
> > +#define SCORE_CONDITION 33
> > +#define SCORE_ECR 34
> > +#define SCORE_EMA 35
> > +#define SCORE_CEH 36
> > +#define SCORE_CEL 37
> > +#define SCORE_COUNTER 38
> > +#define SCORE_LDCR 39
> > +#define SCORE_STCR 40
> > +#define SCORE_PSR 41
> > +#endif
> >
> > #define SINGLESTEP16_INSN 0x7006
> > #define SINGLESTEP32_INSN 0x840C8000
> >
> That looks weird ... not sure if that is a good solution either.
>
> Are those defines actually used anywhere ? They don't seem to be used in the
> kernel.
>
> Side note - an 'a' got missing in your headline.
> And I'd suggest to abbreviate it to something like
> score/uapi: ptrace.h:
>
Besides checking whether they are used in the kernel or not, I presume we
would need to be fairly confident there are actual userspace usecases for
these that require the change. It begs the question of why these
constants are too generic to be in the kernel but are acceptable in the
set of userspace applications in the world.
On 06/24/2014 03:51 PM, David Rientjes wrote:
> On Tue, 24 Jun 2014, Guenter Roeck wrote:
>
>>> diff --git a/arch/score/include/uapi/asm/ptrace.h
>>> b/arch/score/include/uapi/asm/ptrace.h
>>> index f59771a..56e2de0 100644
>>> --- a/arch/score/include/uapi/asm/ptrace.h
>>> +++ b/arch/score/include/uapi/asm/ptrace.h
>>> @@ -4,16 +4,29 @@
>>> #define PTRACE_GETREGS 12
>>> #define PTRACE_SETREGS 13
>>>
>>> -#define PC 32
>>> -#define CONDITION 33
>>> -#define ECR 34
>>> -#define EMA 35
>>> -#define CEH 36
>>> -#define CEL 37
>>> -#define COUNTER 38
>>> -#define LDCR 39
>>> -#define STCR 40
>>> -#define PSR 41
>>> +#if !defined(__KERNEL__) || !defined(__linux__)
>>> +#define PC 32
>>> +#define CONDITION 33
>>> +#define ECR 34
>>> +#define EMA 35
>>> +#define CEH 36
>>> +#define CEL 37
>>> +#define COUNTER 38
>>> +#define LDCR 39
>>> +#define STCR 40
>>> +#define PSR 41
>>> +#else
>>> +#define SCORE_PC 32
>>> +#define SCORE_CONDITION 33
>>> +#define SCORE_ECR 34
>>> +#define SCORE_EMA 35
>>> +#define SCORE_CEH 36
>>> +#define SCORE_CEL 37
>>> +#define SCORE_COUNTER 38
>>> +#define SCORE_LDCR 39
>>> +#define SCORE_STCR 40
>>> +#define SCORE_PSR 41
>>> +#endif
>>>
>>> #define SINGLESTEP16_INSN 0x7006
>>> #define SINGLESTEP32_INSN 0x840C8000
>>>
>> That looks weird ... not sure if that is a good solution either.
>>
>> Are those defines actually used anywhere ? They don't seem to be used in the
>> kernel.
>>
>> Side note - an 'a' got missing in your headline.
>> And I'd suggest to abbreviate it to something like
>> score/uapi: ptrace.h:
>>
>
> Besides checking whether they are used in the kernel or not, I presume we
> would need to be fairly confident there are actual userspace usecases for
> these that require the change. It begs the question of why these
> constants are too generic to be in the kernel but are acceptable in the
> set of userspace applications in the world.
>
That is another question. But for the time being it might be sufficient to
surround the defines with #if !defined(__KERNEL__) without introducing new
(and unused) kernel defines.
Guenter
On Tue, 24 Jun 2014, Guenter Roeck wrote:
> That is another question. But for the time being it might be sufficient to
> surround the defines with #if !defined(__KERNEL__) without introducing new
> (and unused) kernel defines.
>
On the other hand, if no userspace is referencing these, then you could
just rename them and move them out of the uapi header to
include/asm/ptrace.h and be done with it.
On 06/25/2014 07:40 AM, David Rientjes wrote:
> On Tue, 24 Jun 2014, Guenter Roeck wrote:
>
>> That is another question. But for the time being it might be sufficient to
>> surround the defines with #if !defined(__KERNEL__) without introducing new
>> (and unused) kernel defines.
>>
>
> On the other hand, if no userspace is referencing these, then you could
> just rename them and move them out of the uapi header to
> include/asm/ptrace.h and be done with it.
>
OK, thanks. And sorry for the subject lost 'a', firstly. And it is
related with UAPI, so it is really important enough to let related
maintainer to notice about it.
I guess, only the related maintainer know whether can remove these
macros out of "include/uapi/asm/ptrace.h" (although in current upstream
kernel source, they are really not used).
It is abnormal in uapi headers for same values with different macros
switched by "__KERNEL__", but if we have to remain them still in uapi,
we have to use "__KERNEL__ && __linux__" to carm down kernel and user.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
These marcos are listed in advance.
Indeed, they have not been used, and the functions use them have not
be released.
I would like to keep the status even if the configure should not be used.
However, if guys think it is dirty, maybe we can remove or comment them first.
Best,
Lennox
2014-06-25 9:25 GMT+08:00 Chen Gang <[email protected]>:
>
>
> On 06/25/2014 07:40 AM, David Rientjes wrote:
>> On Tue, 24 Jun 2014, Guenter Roeck wrote:
>>
>>> That is another question. But for the time being it might be sufficient to
>>> surround the defines with #if !defined(__KERNEL__) without introducing new
>>> (and unused) kernel defines.
>>>
>>
>> On the other hand, if no userspace is referencing these, then you could
>> just rename them and move them out of the uapi header to
>> include/asm/ptrace.h and be done with it.
>>
>
> OK, thanks. And sorry for the subject lost 'a', firstly. And it is
> related with UAPI, so it is really important enough to let related
> maintainer to notice about it.
>
> I guess, only the related maintainer know whether can remove these
> macros out of "include/uapi/asm/ptrace.h" (although in current upstream
> kernel source, they are really not used).
>
> It is abnormal in uapi headers for same values with different macros
> switched by "__KERNEL__", but if we have to remain them still in uapi,
> we have to use "__KERNEL__ && __linux__" to carm down kernel and user.
>
>
> Thanks.
> --
> Chen Gang
>
> Open, share, and attitude like air, water, and life which God blessed
On 06/25/2014 11:16 PM, Lennox Wu wrote:
> These marcos are listed in advance.
> Indeed, they have not been used, and the functions use them have not
> be released.
UAPI is the main interface for contribution to outside, that is our
main goal, and has the highest priority.
After new contents are added into UAPI, it is very hard to remove them
again: it is protocol, can only be complement, but can not be changed
(or will be a 'cheat').
> I would like to keep the status even if the configure should not be used.
> However, if guys think it is dirty, maybe we can remove or comment them first.
>
So, for me, if we find the new contents in UAPI may have negative effect to
outside, we need stop it firstly (at least, not let it merged into main line).
If no any additional reply for it within this week, I shall send patch
v3 for it (simply remove them).
Thanks.
--
Chen Gang
Open share and attitude like air water and life which God blessed
On Thu, Jun 26, 2014 at 12:05:33AM +0000, Wu Yu-Chen wrote:
> As I said, these macros are useless currently. I don’t think users will use these macros since the kernel doesn’t respond these macros. However, you are welcome to provide your idea ????
>
Just drop them entirely.
Guenter