* [Xenomai-core] [PATCH] Fix gatekeeper affinity @ 2009-02-24 13:35 Jan Kiszka 2009-02-24 13:51 ` Gilles Chanteperdrix 2009-02-24 14:57 ` Philippe Gerum 0 siblings, 2 replies; 7+ messages in thread From: Jan Kiszka @ 2009-02-24 13:35 UTC (permalink / raw) To: xenomai-core As the xnsched structures get initialized later, during xnpod_init, xnsched_cpu always returned 0 in the gatekeeper_thread prologue. That caused binding of all gatekeepers to CPU 0. Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid> --- ksrc/nucleus/shadow.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c index 1dedd85..2243c0e 100644 --- a/ksrc/nucleus/shadow.c +++ b/ksrc/nucleus/shadow.c @@ -823,11 +823,14 @@ static int gatekeeper_thread(void *data) struct task_struct *this_task = current; DECLARE_WAITQUEUE(wait, this_task); struct xnsched *sched = data; - int cpu = xnsched_cpu(sched); struct xnthread *target; cpumask_t cpumask; + int cpu; spl_t s; + /* sched not fully initialized, xnsched_cpu does not work yet */ + cpu = sched - nkpod_struct.sched; + this_task->flags |= PF_NOFREEZE; sigfillset(&this_task->blocked); cpumask = cpumask_of_cpu(cpu); ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Xenomai-core] [PATCH] Fix gatekeeper affinity 2009-02-24 13:35 [Xenomai-core] [PATCH] Fix gatekeeper affinity Jan Kiszka @ 2009-02-24 13:51 ` Gilles Chanteperdrix 2009-02-24 13:57 ` Jan Kiszka 2009-02-24 15:02 ` Philippe Gerum 2009-02-24 14:57 ` Philippe Gerum 1 sibling, 2 replies; 7+ messages in thread From: Gilles Chanteperdrix @ 2009-02-24 13:51 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core Jan Kiszka wrote: > As the xnsched structures get initialized later, during xnpod_init, > xnsched_cpu always returned 0 in the gatekeeper_thread prologue. That > caused binding of all gatekeepers to CPU 0. > > Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid> > --- > > ksrc/nucleus/shadow.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c > index 1dedd85..2243c0e 100644 > --- a/ksrc/nucleus/shadow.c > +++ b/ksrc/nucleus/shadow.c > @@ -823,11 +823,14 @@ static int gatekeeper_thread(void *data) > struct task_struct *this_task = current; > DECLARE_WAITQUEUE(wait, this_task); > struct xnsched *sched = data; > - int cpu = xnsched_cpu(sched); > struct xnthread *target; > cpumask_t cpumask; > + int cpu; > spl_t s; > > + /* sched not fully initialized, xnsched_cpu does not work yet */ > + cpu = sched - nkpod_struct.sched; > + > this_task->flags |= PF_NOFREEZE; > sigfillset(&this_task->blocked); > cpumask = cpumask_of_cpu(cpu); This does not look good, it means that the gatekeeper accesses the sched structure before it is initialized. So, IMO, the proper fix would be to start the gatekeepers only after the sched structure has been initialized. -- Gilles. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Xenomai-core] [PATCH] Fix gatekeeper affinity 2009-02-24 13:51 ` Gilles Chanteperdrix @ 2009-02-24 13:57 ` Jan Kiszka 2009-02-24 14:09 ` Gilles Chanteperdrix 2009-02-24 15:02 ` Philippe Gerum 1 sibling, 1 reply; 7+ messages in thread From: Jan Kiszka @ 2009-02-24 13:57 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> As the xnsched structures get initialized later, during xnpod_init, >> xnsched_cpu always returned 0 in the gatekeeper_thread prologue. That >> caused binding of all gatekeepers to CPU 0. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid> >> --- >> >> ksrc/nucleus/shadow.c | 5 ++++- >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c >> index 1dedd85..2243c0e 100644 >> --- a/ksrc/nucleus/shadow.c >> +++ b/ksrc/nucleus/shadow.c >> @@ -823,11 +823,14 @@ static int gatekeeper_thread(void *data) >> struct task_struct *this_task = current; >> DECLARE_WAITQUEUE(wait, this_task); >> struct xnsched *sched = data; >> - int cpu = xnsched_cpu(sched); >> struct xnthread *target; >> cpumask_t cpumask; >> + int cpu; >> spl_t s; >> >> + /* sched not fully initialized, xnsched_cpu does not work yet */ >> + cpu = sched - nkpod_struct.sched; >> + >> this_task->flags |= PF_NOFREEZE; >> sigfillset(&this_task->blocked); >> cpumask = cpumask_of_cpu(cpu); > > This does not look good, it means that the gatekeeper accesses the sched > structure before it is initialized. So, IMO, the proper fix would be to > start the gatekeepers only after the sched structure has been initialized. > I briefly thought about moving xnshadow_mount into xnpod_init. But given the fact that it worked like this before and that I was not able to quickly exclude new regressions when reordering things, I decided to restore the old pattern. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Xenomai-core] [PATCH] Fix gatekeeper affinity 2009-02-24 13:57 ` Jan Kiszka @ 2009-02-24 14:09 ` Gilles Chanteperdrix 2009-02-24 14:20 ` Jan Kiszka 0 siblings, 1 reply; 7+ messages in thread From: Gilles Chanteperdrix @ 2009-02-24 14:09 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> As the xnsched structures get initialized later, during xnpod_init, >>> xnsched_cpu always returned 0 in the gatekeeper_thread prologue. That >>> caused binding of all gatekeepers to CPU 0. >>> >>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid> >>> --- >>> >>> ksrc/nucleus/shadow.c | 5 ++++- >>> 1 files changed, 4 insertions(+), 1 deletions(-) >>> >>> diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c >>> index 1dedd85..2243c0e 100644 >>> --- a/ksrc/nucleus/shadow.c >>> +++ b/ksrc/nucleus/shadow.c >>> @@ -823,11 +823,14 @@ static int gatekeeper_thread(void *data) >>> struct task_struct *this_task = current; >>> DECLARE_WAITQUEUE(wait, this_task); >>> struct xnsched *sched = data; >>> - int cpu = xnsched_cpu(sched); >>> struct xnthread *target; >>> cpumask_t cpumask; >>> + int cpu; >>> spl_t s; >>> >>> + /* sched not fully initialized, xnsched_cpu does not work yet */ >>> + cpu = sched - nkpod_struct.sched; >>> + >>> this_task->flags |= PF_NOFREEZE; >>> sigfillset(&this_task->blocked); >>> cpumask = cpumask_of_cpu(cpu); >> This does not look good, it means that the gatekeeper accesses the sched >> structure before it is initialized. So, IMO, the proper fix would be to >> start the gatekeepers only after the sched structure has been initialized. >> > > I briefly thought about moving xnshadow_mount into xnpod_init. But given > the fact that it worked like this before and that I was not able to > quickly exclude new regressions when reordering things, I decided to > restore the old pattern. Ok. What about passing cpu as the gatekeeper argument, and using xnpod_sched_slot(cpu) to find the sched pointer ? -- Gilles. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Xenomai-core] [PATCH] Fix gatekeeper affinity 2009-02-24 14:09 ` Gilles Chanteperdrix @ 2009-02-24 14:20 ` Jan Kiszka 0 siblings, 0 replies; 7+ messages in thread From: Jan Kiszka @ 2009-02-24 14:20 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: xenomai-core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> As the xnsched structures get initialized later, during xnpod_init, >>>> xnsched_cpu always returned 0 in the gatekeeper_thread prologue. That >>>> caused binding of all gatekeepers to CPU 0. >>>> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid> >>>> --- >>>> >>>> ksrc/nucleus/shadow.c | 5 ++++- >>>> 1 files changed, 4 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c >>>> index 1dedd85..2243c0e 100644 >>>> --- a/ksrc/nucleus/shadow.c >>>> +++ b/ksrc/nucleus/shadow.c >>>> @@ -823,11 +823,14 @@ static int gatekeeper_thread(void *data) >>>> struct task_struct *this_task = current; >>>> DECLARE_WAITQUEUE(wait, this_task); >>>> struct xnsched *sched = data; >>>> - int cpu = xnsched_cpu(sched); >>>> struct xnthread *target; >>>> cpumask_t cpumask; >>>> + int cpu; >>>> spl_t s; >>>> >>>> + /* sched not fully initialized, xnsched_cpu does not work yet */ >>>> + cpu = sched - nkpod_struct.sched; >>>> + >>>> this_task->flags |= PF_NOFREEZE; >>>> sigfillset(&this_task->blocked); >>>> cpumask = cpumask_of_cpu(cpu); >>> This does not look good, it means that the gatekeeper accesses the sched >>> structure before it is initialized. So, IMO, the proper fix would be to >>> start the gatekeepers only after the sched structure has been initialized. >>> >> I briefly thought about moving xnshadow_mount into xnpod_init. But given >> the fact that it worked like this before and that I was not able to >> quickly exclude new regressions when reordering things, I decided to >> restore the old pattern. > > Ok. What about passing cpu as the gatekeeper argument, and using > xnpod_sched_slot(cpu) to find the sched pointer ? > Something like this? Same result, but looks indeed a bit nicer. -----------> As the xnsched structures get initialized later, during xnpod_init, xnsched_cpu always returned 0 in the gatekeeper_thread prologue. That caused binding of all gatekeepers to CPU 0. Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid> --- ksrc/nucleus/shadow.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c index 1dedd85..6822fcb 100644 --- a/ksrc/nucleus/shadow.c +++ b/ksrc/nucleus/shadow.c @@ -822,8 +822,8 @@ static int gatekeeper_thread(void *data) { struct task_struct *this_task = current; DECLARE_WAITQUEUE(wait, this_task); - struct xnsched *sched = data; - int cpu = xnsched_cpu(sched); + int cpu = (long)data; + struct xnsched *sched = xnpod_sched_slot(cpu); struct xnthread *target; cpumask_t cpumask; spl_t s; @@ -2600,8 +2600,8 @@ int xnshadow_mount(void) sema_init(&sched->gksync, 0); xnarch_memory_barrier(); sched->gatekeeper = - kthread_create(&gatekeeper_thread, sched, "gatekeeper/%d", - cpu); + kthread_create(&gatekeeper_thread, (void *)(long)cpu, + "gatekeeper/%d", cpu); wake_up_process(sched->gatekeeper); down(&sched->gksync); } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Xenomai-core] [PATCH] Fix gatekeeper affinity 2009-02-24 13:51 ` Gilles Chanteperdrix 2009-02-24 13:57 ` Jan Kiszka @ 2009-02-24 15:02 ` Philippe Gerum 1 sibling, 0 replies; 7+ messages in thread From: Philippe Gerum @ 2009-02-24 15:02 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Jan Kiszka, xenomai-core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> As the xnsched structures get initialized later, during xnpod_init, >> xnsched_cpu always returned 0 in the gatekeeper_thread prologue. That >> caused binding of all gatekeepers to CPU 0. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid> >> --- >> >> ksrc/nucleus/shadow.c | 5 ++++- >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c >> index 1dedd85..2243c0e 100644 >> --- a/ksrc/nucleus/shadow.c >> +++ b/ksrc/nucleus/shadow.c >> @@ -823,11 +823,14 @@ static int gatekeeper_thread(void *data) >> struct task_struct *this_task = current; >> DECLARE_WAITQUEUE(wait, this_task); >> struct xnsched *sched = data; >> - int cpu = xnsched_cpu(sched); >> struct xnthread *target; >> cpumask_t cpumask; >> + int cpu; >> spl_t s; >> >> + /* sched not fully initialized, xnsched_cpu does not work yet */ >> + cpu = sched - nkpod_struct.sched; >> + >> this_task->flags |= PF_NOFREEZE; >> sigfillset(&this_task->blocked); >> cpumask = cpumask_of_cpu(cpu); > > This does not look good, it means that the gatekeeper accesses the sched > structure before it is initialized. Fortunately, no, this can't be. The gatekeeper only refers to the semaphore and the sync barrier and only that. Since the semaphore is initialized when the gk starts, the gk sets up the barrier on entry, and the barrier can't be signaled until the nucleus has fully initialized in xnpod_init(), we used to be safe. So, IMO, the proper fix would be to > start the gatekeepers only after the sched structure has been initialized. > -- Philippe. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Xenomai-core] [PATCH] Fix gatekeeper affinity 2009-02-24 13:35 [Xenomai-core] [PATCH] Fix gatekeeper affinity Jan Kiszka 2009-02-24 13:51 ` Gilles Chanteperdrix @ 2009-02-24 14:57 ` Philippe Gerum 1 sibling, 0 replies; 7+ messages in thread From: Philippe Gerum @ 2009-02-24 14:57 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai-core Jan Kiszka wrote: > As the xnsched structures get initialized later, during xnpod_init, > xnsched_cpu always returned 0 in the gatekeeper_thread prologue. That > caused binding of all gatekeepers to CPU 0. > Hey, nice. This is why v2.4.x used pointer arithmetics to determine the cpu # in the first place. > Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid> > --- > > ksrc/nucleus/shadow.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c > index 1dedd85..2243c0e 100644 > --- a/ksrc/nucleus/shadow.c > +++ b/ksrc/nucleus/shadow.c > @@ -823,11 +823,14 @@ static int gatekeeper_thread(void *data) > struct task_struct *this_task = current; > DECLARE_WAITQUEUE(wait, this_task); > struct xnsched *sched = data; > - int cpu = xnsched_cpu(sched); > struct xnthread *target; > cpumask_t cpumask; > + int cpu; > spl_t s; > > + /* sched not fully initialized, xnsched_cpu does not work yet */ > + cpu = sched - nkpod_struct.sched; > + > this_task->flags |= PF_NOFREEZE; > sigfillset(&this_task->blocked); > cpumask = cpumask_of_cpu(cpu); > > _______________________________________________ > Xenomai-core mailing list > Xenomai-core@domain.hid > https://mail.gna.org/listinfo/xenomai-core > -- Philippe. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-02-24 15:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-02-24 13:35 [Xenomai-core] [PATCH] Fix gatekeeper affinity Jan Kiszka 2009-02-24 13:51 ` Gilles Chanteperdrix 2009-02-24 13:57 ` Jan Kiszka 2009-02-24 14:09 ` Gilles Chanteperdrix 2009-02-24 14:20 ` Jan Kiszka 2009-02-24 15:02 ` Philippe Gerum 2009-02-24 14:57 ` Philippe Gerum
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.