* [PATCH 1/2] cobalt/vfile: seq_file seek index must progress
@ 2021-05-24 7:32 Philippe Gerum
2021-05-24 7:32 ` [PATCH 2/2] cobalt/arm: document the syscall convention Philippe Gerum
2021-05-25 6:00 ` [PATCH 1/2] cobalt/vfile: seq_file seek index must progress Jan Kiszka
0 siblings, 2 replies; 6+ messages in thread
From: Philippe Gerum @ 2021-05-24 7:32 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai, Philippe Gerum
From: Philippe Gerum <rpm@xenomai.org>
The offset field we receive from the kernel in a vfile next() handler
must progress in order for the loop to stop properly, independently
from our own tracking of the end-of-list condition.
Bug is reproducible by running two loops in parallel:
- one continuously spawning an application which creates a few tenths
of threads (10-20 would suffice) before exiting shortly after.
- another one continuously reading from /proc/xenomai/sched/{threads,
stat, acct}.
At some point, the vfile handler should cause a kernel crash.
Signed-off-by: Philippe Gerum <rpm@xenomai.org>
---
kernel/cobalt/vfile.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/kernel/cobalt/vfile.c b/kernel/cobalt/vfile.c
index c7b81a704..fae0cc377 100644
--- a/kernel/cobalt/vfile.c
+++ b/kernel/cobalt/vfile.c
@@ -109,11 +109,11 @@ static void *vfile_snapshot_next(struct seq_file *seq, void *v, loff_t *offp)
struct xnvfile_snapshot_iterator *it = seq->private;
loff_t pos = *offp;
+ ++*offp;
+
if (pos >= it->nrdata)
return NULL;
- ++*offp;
-
return it->databuf + pos * it->vfile->datasz;
}
@@ -452,17 +452,15 @@ static void *vfile_regular_next(struct seq_file *seq, void *v, loff_t *offp)
struct xnvfile_regular *vfile = it->vfile;
void *data;
+ it->pos = ++(*offp);
+
if (vfile->ops->next == NULL)
return NULL;
- it->pos = *offp + 1;
-
data = vfile->ops->next(it);
if (data == NULL)
return NULL;
- *offp = it->pos;
-
return data;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] cobalt/arm: document the syscall convention
2021-05-24 7:32 [PATCH 1/2] cobalt/vfile: seq_file seek index must progress Philippe Gerum
@ 2021-05-24 7:32 ` Philippe Gerum
2021-05-25 5:58 ` Jan Kiszka
2021-05-25 6:00 ` [PATCH 1/2] cobalt/vfile: seq_file seek index must progress Jan Kiszka
1 sibling, 1 reply; 6+ messages in thread
From: Philippe Gerum @ 2021-05-24 7:32 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai, Philippe Gerum
From: Philippe Gerum <rpm@xenomai.org>
Signed-off-by: Philippe Gerum <rpm@xenomai.org>
---
.../arm/include/asm/xenomai/uapi/syscall.h | 31 ++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h b/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
index c079a358c..9146b732a 100644
--- a/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
+++ b/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
@@ -21,9 +21,38 @@
#ifndef _COBALT_ARM_ASM_UAPI_SYSCALL_H
#define _COBALT_ARM_ASM_UAPI_SYSCALL_H
+/*
+ * Argument marshalling for Cobalt syscalls:
+ *
+ * r7 := XENO_ARM_SYSCALL
+ * r0 := Cobalt syscall code (sc_cobalt_xx | __COBALT_SYSCALL_BIT)
+ * r1 := first argument
+ * r2 := second argument
+ * ...
+ * r5 := fifth argument
+ *
+ * r0 := <return value>
+ *
+ * This marshalling is common to the I-pipe and Dovetail-based
+ * configurations so that we have a single ABI convention for both,
+ * enabling us to have a single user build which does not depend on
+ * the pipeline type. With Dovetail, CONFIG_IPIPE_COMPAT is enabled
+ * to allow this.
+ *
+ * __COBALT_SYSCALL_BIT is a generic marker for Cobalt system calls we
+ * use with all CPU architectures in common code, to distinguish them
+ * from native Linux syscalls. For this reason, it must be present in
+ * the Cobalt syscall code register (r0) as well.
+ */
+
#define __xn_syscode(__nr) (__COBALT_SYSCALL_BIT | (__nr))
-#define XENO_ARM_SYSCALL 0x000F0042 /* carefully chosen... */
+/*
+ * A carefully choosen ARM-specific local syscall code denoting a
+ * Cobalt syscall, which the interrupt pipeline looks for in order to
+ * route the request to the real-time core via a dedicated handler.
+ */
+#define XENO_ARM_SYSCALL 0x000F0042
#define XENOMAI_SYSARCH_TSCINFO 4
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] cobalt/arm: document the syscall convention
2021-05-24 7:32 ` [PATCH 2/2] cobalt/arm: document the syscall convention Philippe Gerum
@ 2021-05-25 5:58 ` Jan Kiszka
2021-05-25 7:30 ` Philippe Gerum
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2021-05-25 5:58 UTC (permalink / raw)
To: Philippe Gerum; +Cc: xenomai
On 24.05.21 09:32, Philippe Gerum wrote:
> From: Philippe Gerum <rpm@xenomai.org>
>
> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
> ---
> .../arm/include/asm/xenomai/uapi/syscall.h | 31 ++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h b/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
> index c079a358c..9146b732a 100644
> --- a/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
> +++ b/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
> @@ -21,9 +21,38 @@
> #ifndef _COBALT_ARM_ASM_UAPI_SYSCALL_H
> #define _COBALT_ARM_ASM_UAPI_SYSCALL_H
>
> +/*
> + * Argument marshalling for Cobalt syscalls:
> + *
> + * r7 := XENO_ARM_SYSCALL
> + * r0 := Cobalt syscall code (sc_cobalt_xx | __COBALT_SYSCALL_BIT)
> + * r1 := first argument
> + * r2 := second argument
> + * ...
> + * r5 := fifth argument
> + *
> + * r0 := <return value>
> + *
> + * This marshalling is common to the I-pipe and Dovetail-based
> + * configurations so that we have a single ABI convention for both,
> + * enabling us to have a single user build which does not depend on
> + * the pipeline type. With Dovetail, CONFIG_IPIPE_COMPAT is enabled
> + * to allow this.
> + *
> + * __COBALT_SYSCALL_BIT is a generic marker for Cobalt system calls we
> + * use with all CPU architectures in common code, to distinguish them
> + * from native Linux syscalls. For this reason, it must be present in
> + * the Cobalt syscall code register (r0) as well.
Could we please add the reason for why we have this deviation from the
kernel's ABI on ARM? One sentence is likely enough ("it makes things
much faster" or "needed because of limited free bits" or whatever), and
it would be a perfect place to drop it here.
Thanks,
Jan
> + */
> +
> #define __xn_syscode(__nr) (__COBALT_SYSCALL_BIT | (__nr))
>
> -#define XENO_ARM_SYSCALL 0x000F0042 /* carefully chosen... */
> +/*
> + * A carefully choosen ARM-specific local syscall code denoting a
> + * Cobalt syscall, which the interrupt pipeline looks for in order to
> + * route the request to the real-time core via a dedicated handler.
> + */
> +#define XENO_ARM_SYSCALL 0x000F0042
>
> #define XENOMAI_SYSARCH_TSCINFO 4
>
>
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] cobalt/arm: document the syscall convention
2021-05-25 5:58 ` Jan Kiszka
@ 2021-05-25 7:30 ` Philippe Gerum
2021-05-25 7:41 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Gerum @ 2021-05-25 7:30 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai
Jan Kiszka <jan.kiszka@siemens.com> writes:
> On 24.05.21 09:32, Philippe Gerum wrote:
>> From: Philippe Gerum <rpm@xenomai.org>
>>
>> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
>> ---
>> .../arm/include/asm/xenomai/uapi/syscall.h | 31 ++++++++++++++++++-
>> 1 file changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h b/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
>> index c079a358c..9146b732a 100644
>> --- a/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
>> +++ b/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
>> @@ -21,9 +21,38 @@
>> #ifndef _COBALT_ARM_ASM_UAPI_SYSCALL_H
>> #define _COBALT_ARM_ASM_UAPI_SYSCALL_H
>>
>> +/*
>> + * Argument marshalling for Cobalt syscalls:
>> + *
>> + * r7 := XENO_ARM_SYSCALL
>> + * r0 := Cobalt syscall code (sc_cobalt_xx | __COBALT_SYSCALL_BIT)
>> + * r1 := first argument
>> + * r2 := second argument
>> + * ...
>> + * r5 := fifth argument
>> + *
>> + * r0 := <return value>
>> + *
>> + * This marshalling is common to the I-pipe and Dovetail-based
>> + * configurations so that we have a single ABI convention for both,
>> + * enabling us to have a single user build which does not depend on
>> + * the pipeline type. With Dovetail, CONFIG_IPIPE_COMPAT is enabled
>> + * to allow this.
>> + *
>> + * __COBALT_SYSCALL_BIT is a generic marker for Cobalt system calls we
>> + * use with all CPU architectures in common code, to distinguish them
>> + * from native Linux syscalls. For this reason, it must be present in
>> + * the Cobalt syscall code register (r0) as well.
>
> Could we please add the reason for why we have this deviation from the
> kernel's ABI on ARM? One sentence is likely enough ("it makes things
> much faster" or "needed because of limited free bits" or whatever), and
> it would be a perfect place to drop it here.
>
There is no reason. Just a fact of history.
> Thanks,
> Jan
>
>> + */
>> +
>> #define __xn_syscode(__nr) (__COBALT_SYSCALL_BIT | (__nr))
>>
>> -#define XENO_ARM_SYSCALL 0x000F0042 /* carefully chosen... */
>> +/*
>> + * A carefully choosen ARM-specific local syscall code denoting a
>> + * Cobalt syscall, which the interrupt pipeline looks for in order to
>> + * route the request to the real-time core via a dedicated handler.
>> + */
>> +#define XENO_ARM_SYSCALL 0x000F0042
>>
>> #define XENOMAI_SYSARCH_TSCINFO 4
>>
>>
--
Philippe.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] cobalt/arm: document the syscall convention
2021-05-25 7:30 ` Philippe Gerum
@ 2021-05-25 7:41 ` Jan Kiszka
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2021-05-25 7:41 UTC (permalink / raw)
To: Philippe Gerum; +Cc: xenomai
On 25.05.21 09:30, Philippe Gerum wrote:
>
> Jan Kiszka <jan.kiszka@siemens.com> writes:
>
>> On 24.05.21 09:32, Philippe Gerum wrote:
>>> From: Philippe Gerum <rpm@xenomai.org>
>>>
>>> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
>>> ---
>>> .../arm/include/asm/xenomai/uapi/syscall.h | 31 ++++++++++++++++++-
>>> 1 file changed, 30 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h b/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
>>> index c079a358c..9146b732a 100644
>>> --- a/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
>>> +++ b/kernel/cobalt/arch/arm/include/asm/xenomai/uapi/syscall.h
>>> @@ -21,9 +21,38 @@
>>> #ifndef _COBALT_ARM_ASM_UAPI_SYSCALL_H
>>> #define _COBALT_ARM_ASM_UAPI_SYSCALL_H
>>>
>>> +/*
>>> + * Argument marshalling for Cobalt syscalls:
>>> + *
>>> + * r7 := XENO_ARM_SYSCALL
>>> + * r0 := Cobalt syscall code (sc_cobalt_xx | __COBALT_SYSCALL_BIT)
>>> + * r1 := first argument
>>> + * r2 := second argument
>>> + * ...
>>> + * r5 := fifth argument
>>> + *
>>> + * r0 := <return value>
>>> + *
>>> + * This marshalling is common to the I-pipe and Dovetail-based
>>> + * configurations so that we have a single ABI convention for both,
>>> + * enabling us to have a single user build which does not depend on
>>> + * the pipeline type. With Dovetail, CONFIG_IPIPE_COMPAT is enabled
>>> + * to allow this.
>>> + *
>>> + * __COBALT_SYSCALL_BIT is a generic marker for Cobalt system calls we
>>> + * use with all CPU architectures in common code, to distinguish them
>>> + * from native Linux syscalls. For this reason, it must be present in
>>> + * the Cobalt syscall code register (r0) as well.
>>
>> Could we please add the reason for why we have this deviation from the
>> kernel's ABI on ARM? One sentence is likely enough ("it makes things
>> much faster" or "needed because of limited free bits" or whatever), and
>> it would be a perfect place to drop it here.
>>
>
> There is no reason. Just a fact of history.
That's good to hear: Time to clean up for 3.2. We are fine break ABIs on
major updates.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] cobalt/vfile: seq_file seek index must progress
2021-05-24 7:32 [PATCH 1/2] cobalt/vfile: seq_file seek index must progress Philippe Gerum
2021-05-24 7:32 ` [PATCH 2/2] cobalt/arm: document the syscall convention Philippe Gerum
@ 2021-05-25 6:00 ` Jan Kiszka
1 sibling, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2021-05-25 6:00 UTC (permalink / raw)
To: Philippe Gerum; +Cc: xenomai
On 24.05.21 09:32, Philippe Gerum wrote:
> From: Philippe Gerum <rpm@xenomai.org>
>
> The offset field we receive from the kernel in a vfile next() handler
> must progress in order for the loop to stop properly, independently
> from our own tracking of the end-of-list condition.
>
> Bug is reproducible by running two loops in parallel:
>
> - one continuously spawning an application which creates a few tenths
> of threads (10-20 would suffice) before exiting shortly after.
>
> - another one continuously reading from /proc/xenomai/sched/{threads,
> stat, acct}.
>
> At some point, the vfile handler should cause a kernel crash.
>
> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
> ---
> kernel/cobalt/vfile.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/cobalt/vfile.c b/kernel/cobalt/vfile.c
> index c7b81a704..fae0cc377 100644
> --- a/kernel/cobalt/vfile.c
> +++ b/kernel/cobalt/vfile.c
> @@ -109,11 +109,11 @@ static void *vfile_snapshot_next(struct seq_file *seq, void *v, loff_t *offp)
> struct xnvfile_snapshot_iterator *it = seq->private;
> loff_t pos = *offp;
>
> + ++*offp;
> +
> if (pos >= it->nrdata)
> return NULL;
>
> - ++*offp;
> -
> return it->databuf + pos * it->vfile->datasz;
> }
>
> @@ -452,17 +452,15 @@ static void *vfile_regular_next(struct seq_file *seq, void *v, loff_t *offp)
> struct xnvfile_regular *vfile = it->vfile;
> void *data;
>
> + it->pos = ++(*offp);
> +
> if (vfile->ops->next == NULL)
> return NULL;
>
> - it->pos = *offp + 1;
> -
> data = vfile->ops->next(it);
> if (data == NULL)
> return NULL;
>
> - *offp = it->pos;
> -
> return data;
> }
>
>
Thanks, applied to next and all stable branches.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-25 7:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-24 7:32 [PATCH 1/2] cobalt/vfile: seq_file seek index must progress Philippe Gerum
2021-05-24 7:32 ` [PATCH 2/2] cobalt/arm: document the syscall convention Philippe Gerum
2021-05-25 5:58 ` Jan Kiszka
2021-05-25 7:30 ` Philippe Gerum
2021-05-25 7:41 ` Jan Kiszka
2021-05-25 6:00 ` [PATCH 1/2] cobalt/vfile: seq_file seek index must progress Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.