* Re: [Xenomai] [Xenomai-git] Jan Kiszka : cobalt/kernel: Fix invalidation of cond shadow [not found] <E1aJQEe-0006t5-UO@sd-51317.xenomai.org> @ 2016-01-13 18:44 ` Philippe Gerum 2016-01-13 19:42 ` Jan Kiszka 0 siblings, 1 reply; 5+ messages in thread From: Philippe Gerum @ 2016-01-13 18:44 UTC (permalink / raw) To: Kiszka, Jan; +Cc: xenomai On 01/13/2016 07:33 PM, git repository hosting wrote: > Module: xenomai-jki > Branch: for-forge > Commit: b7d37dab571df0c0cd42aae094c3d5d1bc71db80 > URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=b7d37dab571df0c0cd42aae094c3d5d1bc71db80 > > Author: Jan Kiszka <jan.kiszka@siemens.com> > Date: Wed Jan 13 19:31:23 2016 +0100 > > cobalt/kernel: Fix invalidation of cond shadow > > pthread_cond_destroy fails to invalidate also the shadow object. This > can cause spurious -EBUSY errors on re-initialization of the very same > shadow as condition variable later on. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > --- > > kernel/cobalt/posix/cond.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/cobalt/posix/cond.c b/kernel/cobalt/posix/cond.c > index 7e115cf..84ae8fd 100644 > --- a/kernel/cobalt/posix/cond.c > +++ b/kernel/cobalt/posix/cond.c > @@ -117,6 +117,8 @@ static inline int pthread_cond_destroy(struct cobalt_cond_shadow *cnd) > > cobalt_cond_reclaim(&cond->resnode, s); /* drops lock */ > > + cobalt_mark_deleted(cnd); > + > return 0; > } > > cobalt_cond_reclaim() invalidates the shadow object already. Did you observe -EBUSY already? If so, maybe a rescheduling happens early when flushing the synchro object, before the invalidation takes place in that routine. i.e. diff --git a/kernel/cobalt/posix/cond.c b/kernel/cobalt/posix/cond.c index 7e115cf..3f81b47 100644 --- a/kernel/cobalt/posix/cond.c +++ b/kernel/cobalt/posix/cond.c @@ -414,8 +414,8 @@ void cobalt_cond_reclaim(struct cobalt_resnode *node, spl_t s) cond = container_of(node, struct cobalt_cond, resnode); xnregistry_remove(node->handle); cobalt_del_resource(node); - xnsynch_destroy(&cond->synchbase); cobalt_mark_deleted(cond); + xnsynch_destroy(&cond->synchbase); xnlock_put_irqrestore(&nklock, s); cobalt_umm_free(&cobalt_ppd_get(cond->attr.pshared)->umm, -- Philippe. ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Xenomai] [Xenomai-git] Jan Kiszka : cobalt/kernel: Fix invalidation of cond shadow 2016-01-13 18:44 ` [Xenomai] [Xenomai-git] Jan Kiszka : cobalt/kernel: Fix invalidation of cond shadow Philippe Gerum @ 2016-01-13 19:42 ` Jan Kiszka 2016-01-14 8:46 ` Philippe Gerum 0 siblings, 1 reply; 5+ messages in thread From: Jan Kiszka @ 2016-01-13 19:42 UTC (permalink / raw) To: Philippe Gerum; +Cc: xenomai On 2016-01-13 19:44, Philippe Gerum wrote: > On 01/13/2016 07:33 PM, git repository hosting wrote: >> Module: xenomai-jki >> Branch: for-forge >> Commit: b7d37dab571df0c0cd42aae094c3d5d1bc71db80 >> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=b7d37dab571df0c0cd42aae094c3d5d1bc71db80 >> >> Author: Jan Kiszka <jan.kiszka@siemens.com> >> Date: Wed Jan 13 19:31:23 2016 +0100 >> >> cobalt/kernel: Fix invalidation of cond shadow >> >> pthread_cond_destroy fails to invalidate also the shadow object. This >> can cause spurious -EBUSY errors on re-initialization of the very same >> shadow as condition variable later on. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> >> --- >> >> kernel/cobalt/posix/cond.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/kernel/cobalt/posix/cond.c b/kernel/cobalt/posix/cond.c >> index 7e115cf..84ae8fd 100644 >> --- a/kernel/cobalt/posix/cond.c >> +++ b/kernel/cobalt/posix/cond.c >> @@ -117,6 +117,8 @@ static inline int pthread_cond_destroy(struct cobalt_cond_shadow *cnd) >> >> cobalt_cond_reclaim(&cond->resnode, s); /* drops lock */ >> >> + cobalt_mark_deleted(cnd); >> + >> return 0; >> } >> >> > > cobalt_cond_reclaim() invalidates the shadow object already. Did you Nope, it only invalidates cond->magic, i.e. the object itself. We also need to invalidate cnd->magic here. See also mutex.c, it has the same pattern. > observe -EBUSY already? If so, maybe a rescheduling happens early when > flushing the synchro object, before the invalidation takes place in that > routine. Yes, we saw -EBUSY in the field. I wasn't able to create a reproduction case, but I debugged that pthread_mutex_destroy failed to change the shadow object. The patch still needs validation with our field scenario, but the issue seems to be solved under the microscope (debugger) at least. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Xenomai] [Xenomai-git] Jan Kiszka : cobalt/kernel: Fix invalidation of cond shadow 2016-01-13 19:42 ` Jan Kiszka @ 2016-01-14 8:46 ` Philippe Gerum 2016-01-15 13:59 ` Jan Kiszka 0 siblings, 1 reply; 5+ messages in thread From: Philippe Gerum @ 2016-01-14 8:46 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai On 01/13/2016 08:42 PM, Jan Kiszka wrote: > On 2016-01-13 19:44, Philippe Gerum wrote: >> On 01/13/2016 07:33 PM, git repository hosting wrote: >>> Module: xenomai-jki >>> Branch: for-forge >>> Commit: b7d37dab571df0c0cd42aae094c3d5d1bc71db80 >>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=b7d37dab571df0c0cd42aae094c3d5d1bc71db80 >>> >>> Author: Jan Kiszka <jan.kiszka@siemens.com> >>> Date: Wed Jan 13 19:31:23 2016 +0100 >>> >>> cobalt/kernel: Fix invalidation of cond shadow >>> >>> pthread_cond_destroy fails to invalidate also the shadow object. This >>> can cause spurious -EBUSY errors on re-initialization of the very same >>> shadow as condition variable later on. >>> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>> >>> --- >>> >>> kernel/cobalt/posix/cond.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/kernel/cobalt/posix/cond.c b/kernel/cobalt/posix/cond.c >>> index 7e115cf..84ae8fd 100644 >>> --- a/kernel/cobalt/posix/cond.c >>> +++ b/kernel/cobalt/posix/cond.c >>> @@ -117,6 +117,8 @@ static inline int pthread_cond_destroy(struct cobalt_cond_shadow *cnd) >>> >>> cobalt_cond_reclaim(&cond->resnode, s); /* drops lock */ >>> >>> + cobalt_mark_deleted(cnd); >>> + >>> return 0; >>> } >>> >>> >> >> cobalt_cond_reclaim() invalidates the shadow object already. Did you > > Nope, it only invalidates cond->magic, i.e. the object itself. We also > need to invalidate cnd->magic here. See also mutex.c, it has the same > pattern. > Ack, I misread the patch. This is a regression compared to 2.x, we need both descriptors to be invalidated indeed. -- Philippe. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Xenomai] [Xenomai-git] Jan Kiszka : cobalt/kernel: Fix invalidation of cond shadow 2016-01-14 8:46 ` Philippe Gerum @ 2016-01-15 13:59 ` Jan Kiszka 2016-01-16 17:00 ` Philippe Gerum 0 siblings, 1 reply; 5+ messages in thread From: Jan Kiszka @ 2016-01-15 13:59 UTC (permalink / raw) To: Philippe Gerum; +Cc: xenomai On 2016-01-14 09:46, Philippe Gerum wrote: > On 01/13/2016 08:42 PM, Jan Kiszka wrote: >> On 2016-01-13 19:44, Philippe Gerum wrote: >>> On 01/13/2016 07:33 PM, git repository hosting wrote: >>>> Module: xenomai-jki >>>> Branch: for-forge >>>> Commit: b7d37dab571df0c0cd42aae094c3d5d1bc71db80 >>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=b7d37dab571df0c0cd42aae094c3d5d1bc71db80 >>>> >>>> Author: Jan Kiszka <jan.kiszka@siemens.com> >>>> Date: Wed Jan 13 19:31:23 2016 +0100 >>>> >>>> cobalt/kernel: Fix invalidation of cond shadow >>>> >>>> pthread_cond_destroy fails to invalidate also the shadow object. This >>>> can cause spurious -EBUSY errors on re-initialization of the very same >>>> shadow as condition variable later on. >>>> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> --- >>>> >>>> kernel/cobalt/posix/cond.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/kernel/cobalt/posix/cond.c b/kernel/cobalt/posix/cond.c >>>> index 7e115cf..84ae8fd 100644 >>>> --- a/kernel/cobalt/posix/cond.c >>>> +++ b/kernel/cobalt/posix/cond.c >>>> @@ -117,6 +117,8 @@ static inline int pthread_cond_destroy(struct cobalt_cond_shadow *cnd) >>>> >>>> cobalt_cond_reclaim(&cond->resnode, s); /* drops lock */ >>>> >>>> + cobalt_mark_deleted(cnd); >>>> + >>>> return 0; >>>> } >>>> >>>> >>> >>> cobalt_cond_reclaim() invalidates the shadow object already. Did you >> >> Nope, it only invalidates cond->magic, i.e. the object itself. We also >> need to invalidate cnd->magic here. See also mutex.c, it has the same >> pattern. >> > > Ack, I misread the patch. This is a regression compared to 2.x, we need > both descriptors to be invalidated indeed. > OK, will you pull into into next and stable? BTW, it's now successfully validated by our test scenario. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Xenomai] [Xenomai-git] Jan Kiszka : cobalt/kernel: Fix invalidation of cond shadow 2016-01-15 13:59 ` Jan Kiszka @ 2016-01-16 17:00 ` Philippe Gerum 0 siblings, 0 replies; 5+ messages in thread From: Philippe Gerum @ 2016-01-16 17:00 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai On 01/15/2016 02:59 PM, Jan Kiszka wrote: > On 2016-01-14 09:46, Philippe Gerum wrote: >> On 01/13/2016 08:42 PM, Jan Kiszka wrote: >>> On 2016-01-13 19:44, Philippe Gerum wrote: >>>> On 01/13/2016 07:33 PM, git repository hosting wrote: >>>>> Module: xenomai-jki >>>>> Branch: for-forge >>>>> Commit: b7d37dab571df0c0cd42aae094c3d5d1bc71db80 >>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=b7d37dab571df0c0cd42aae094c3d5d1bc71db80 >>>>> >>>>> Author: Jan Kiszka <jan.kiszka@siemens.com> >>>>> Date: Wed Jan 13 19:31:23 2016 +0100 >>>>> >>>>> cobalt/kernel: Fix invalidation of cond shadow >>>>> >>>>> pthread_cond_destroy fails to invalidate also the shadow object. This >>>>> can cause spurious -EBUSY errors on re-initialization of the very same >>>>> shadow as condition variable later on. >>>>> >>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>>>> >>>>> --- >>>>> >>>>> kernel/cobalt/posix/cond.c | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/kernel/cobalt/posix/cond.c b/kernel/cobalt/posix/cond.c >>>>> index 7e115cf..84ae8fd 100644 >>>>> --- a/kernel/cobalt/posix/cond.c >>>>> +++ b/kernel/cobalt/posix/cond.c >>>>> @@ -117,6 +117,8 @@ static inline int pthread_cond_destroy(struct cobalt_cond_shadow *cnd) >>>>> >>>>> cobalt_cond_reclaim(&cond->resnode, s); /* drops lock */ >>>>> >>>>> + cobalt_mark_deleted(cnd); >>>>> + >>>>> return 0; >>>>> } >>>>> >>>>> >>>> >>>> cobalt_cond_reclaim() invalidates the shadow object already. Did you >>> >>> Nope, it only invalidates cond->magic, i.e. the object itself. We also >>> need to invalidate cnd->magic here. See also mutex.c, it has the same >>> pattern. >>> >> >> Ack, I misread the patch. This is a regression compared to 2.x, we need >> both descriptors to be invalidated indeed. >> > > OK, will you pull into into next and stable? BTW, it's now successfully > validated by our test scenario. > Done, thanks. -- Philippe. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-01-16 17:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <E1aJQEe-0006t5-UO@sd-51317.xenomai.org>
2016-01-13 18:44 ` [Xenomai] [Xenomai-git] Jan Kiszka : cobalt/kernel: Fix invalidation of cond shadow Philippe Gerum
2016-01-13 19:42 ` Jan Kiszka
2016-01-14 8:46 ` Philippe Gerum
2016-01-15 13:59 ` Jan Kiszka
2016-01-16 17:00 ` 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.