* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function [not found] <E1NfBdX-0001ga-4b@domain.hid> @ 2010-02-10 12:34 ` Gilles Chanteperdrix 2010-02-10 12:43 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-10 12:34 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core GIT version control wrote: > Module: xenomai-jki > Branch: for-upstream > Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 > URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 > > Author: Jan Kiszka <jan.kiszka@domain.hid> > Date: Wed Feb 10 13:24:29 2010 +0100 > > Make xnarch_init_timeconv an uninlined weak function > > Otherwise the wrong set of time conversion variables might get > initialized when using > 1 skin libraries. If that would be possible, then it is the conversion variables which should made be weak, not the function. The way I see it, the posix and native skins currently get a different set of variables and functions, which works, but with your change, since there is only one function, only one set of variable gets initialized by the two function calls. And one skin just broke. Or am I missing something? Does the patch fix a problem you really had? -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 12:34 ` [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function Gilles Chanteperdrix @ 2010-02-10 12:43 ` Jan Kiszka 2010-02-10 14:01 ` Gilles Chanteperdrix 2010-02-10 19:33 ` Gilles Chanteperdrix 0 siblings, 2 replies; 28+ messages in thread From: Jan Kiszka @ 2010-02-10 12:43 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core Gilles Chanteperdrix wrote: > GIT version control wrote: >> Module: xenomai-jki >> Branch: for-upstream >> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >> >> Author: Jan Kiszka <jan.kiszka@domain.hid> >> Date: Wed Feb 10 13:24:29 2010 +0100 >> >> Make xnarch_init_timeconv an uninlined weak function >> >> Otherwise the wrong set of time conversion variables might get >> initialized when using > 1 skin libraries. > > If that would be possible, then it is the conversion variables which > should made be weak, not the function. > > The way I see it, the posix and native skins currently get a different > set of variables and functions, which works, but with your change, since > there is only one function, only one set of variable gets initialized by > the two function calls. And one skin just broke. > > Or am I missing something? Does the patch fix a problem you really had? Frankly, I wasn't able to test in the field yet as replacing the libs there is non-trivial. But I was able to observe that only one set of functions is used - which is logical considering the weak marks. And this breaks due to the static inline initialization. However, let's mark both functions and variables weak to fix the issue and avoid leaving unused variables around. Will update my patch in a minute. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 12:43 ` Jan Kiszka @ 2010-02-10 14:01 ` Gilles Chanteperdrix 2010-02-10 14:48 ` Philippe Gerum 2010-02-10 19:33 ` Gilles Chanteperdrix 1 sibling, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-10 14:01 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> GIT version control wrote: >>> Module: xenomai-jki >>> Branch: for-upstream >>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>> >>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>> Date: Wed Feb 10 13:24:29 2010 +0100 >>> >>> Make xnarch_init_timeconv an uninlined weak function >>> >>> Otherwise the wrong set of time conversion variables might get >>> initialized when using > 1 skin libraries. >> If that would be possible, then it is the conversion variables which >> should made be weak, not the function. >> >> The way I see it, the posix and native skins currently get a different >> set of variables and functions, which works, but with your change, since >> there is only one function, only one set of variable gets initialized by >> the two function calls. And one skin just broke. >> >> Or am I missing something? Does the patch fix a problem you really had? > > Frankly, I wasn't able to test in the field yet as replacing the libs > there is non-trivial. But I was able to observe that only one set of > functions is used - which is logical considering the weak marks. And > this breaks due to the static inline initialization. > > However, let's mark both functions and variables weak to fix the issue > and avoid leaving unused variables around. Will update my patch in a minute. Ok. Merged, thanks. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 14:01 ` Gilles Chanteperdrix @ 2010-02-10 14:48 ` Philippe Gerum 0 siblings, 0 replies; 28+ messages in thread From: Philippe Gerum @ 2010-02-10 14:48 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Jan Kiszka, Xenomai core On Wed, 2010-02-10 at 15:01 +0100, Gilles Chanteperdrix wrote: > Jan Kiszka wrote: > > Gilles Chanteperdrix wrote: > >> GIT version control wrote: > >>> Module: xenomai-jki > >>> Branch: for-upstream > >>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 > >>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 > >>> > >>> Author: Jan Kiszka <jan.kiszka@domain.hid> > >>> Date: Wed Feb 10 13:24:29 2010 +0100 > >>> > >>> Make xnarch_init_timeconv an uninlined weak function > >>> > >>> Otherwise the wrong set of time conversion variables might get > >>> initialized when using > 1 skin libraries. > >> If that would be possible, then it is the conversion variables which > >> should made be weak, not the function. > >> > >> The way I see it, the posix and native skins currently get a different > >> set of variables and functions, which works, but with your change, since > >> there is only one function, only one set of variable gets initialized by > >> the two function calls. And one skin just broke. > >> > >> Or am I missing something? Does the patch fix a problem you really had? > > > > Frankly, I wasn't able to test in the field yet as replacing the libs > > there is non-trivial. But I was able to observe that only one set of > > functions is used - which is logical considering the weak marks. And > > this breaks due to the static inline initialization. > > > > However, let's mark both functions and variables weak to fix the issue > > and avoid leaving unused variables around. Will update my patch in a minute. > > Ok. Merged, thanks. > The proper way to get rid of any ambiguous case would be to move that init code out of line, to the skins/common lib you already had the good idea to create. -- Philippe. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 12:43 ` Jan Kiszka 2010-02-10 14:01 ` Gilles Chanteperdrix @ 2010-02-10 19:33 ` Gilles Chanteperdrix 2010-02-10 20:24 ` Jan Kiszka 1 sibling, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-10 19:33 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> GIT version control wrote: >>> Module: xenomai-jki >>> Branch: for-upstream >>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>> >>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>> Date: Wed Feb 10 13:24:29 2010 +0100 >>> >>> Make xnarch_init_timeconv an uninlined weak function >>> >>> Otherwise the wrong set of time conversion variables might get >>> initialized when using > 1 skin libraries. >> If that would be possible, then it is the conversion variables which >> should made be weak, not the function. >> >> The way I see it, the posix and native skins currently get a different >> set of variables and functions, which works, but with your change, since >> there is only one function, only one set of variable gets initialized by >> the two function calls. And one skin just broke. >> >> Or am I missing something? Does the patch fix a problem you really had? > > Frankly, I wasn't able to test in the field yet as replacing the libs > there is non-trivial. But I was able to observe that only one set of > functions is used - which is logical considering the weak marks. And > this breaks due to the static inline initialization. > > However, let's mark both functions and variables weak to fix the issue > and avoid leaving unused variables around. Will update my patch in a minute. Ok. I am reverting this patch until you provide me with another solution. It causes latency to segfault purely and simply at startup on my dual PIII. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 19:33 ` Gilles Chanteperdrix @ 2010-02-10 20:24 ` Jan Kiszka 2010-02-10 20:33 ` Gilles Chanteperdrix 0 siblings, 1 reply; 28+ messages in thread From: Jan Kiszka @ 2010-02-10 20:24 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core [-- Attachment #1: Type: text/plain, Size: 1773 bytes --] Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> GIT version control wrote: >>>> Module: xenomai-jki >>>> Branch: for-upstream >>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>> >>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>> >>>> Make xnarch_init_timeconv an uninlined weak function >>>> >>>> Otherwise the wrong set of time conversion variables might get >>>> initialized when using > 1 skin libraries. >>> If that would be possible, then it is the conversion variables which >>> should made be weak, not the function. >>> >>> The way I see it, the posix and native skins currently get a different >>> set of variables and functions, which works, but with your change, since >>> there is only one function, only one set of variable gets initialized by >>> the two function calls. And one skin just broke. >>> >>> Or am I missing something? Does the patch fix a problem you really had? >> Frankly, I wasn't able to test in the field yet as replacing the libs >> there is non-trivial. But I was able to observe that only one set of >> functions is used - which is logical considering the weak marks. And >> this breaks due to the static inline initialization. >> >> However, let's mark both functions and variables weak to fix the issue >> and avoid leaving unused variables around. Will update my patch in a minute. > > Ok. I am reverting this patch until you provide me with another > solution. It causes latency to segfault purely and simply at startup on > my dual PIII. > Cannot reproduce yet. Do you have a backtrace? Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 20:24 ` Jan Kiszka @ 2010-02-10 20:33 ` Gilles Chanteperdrix 2010-02-10 21:09 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-10 20:33 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: >>>> GIT version control wrote: >>>>> Module: xenomai-jki >>>>> Branch: for-upstream >>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>> >>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>> >>>>> Make xnarch_init_timeconv an uninlined weak function >>>>> >>>>> Otherwise the wrong set of time conversion variables might get >>>>> initialized when using > 1 skin libraries. >>>> If that would be possible, then it is the conversion variables which >>>> should made be weak, not the function. >>>> >>>> The way I see it, the posix and native skins currently get a different >>>> set of variables and functions, which works, but with your change, since >>>> there is only one function, only one set of variable gets initialized by >>>> the two function calls. And one skin just broke. >>>> >>>> Or am I missing something? Does the patch fix a problem you really had? >>> Frankly, I wasn't able to test in the field yet as replacing the libs >>> there is non-trivial. But I was able to observe that only one set of >>> functions is used - which is logical considering the weak marks. And >>> this breaks due to the static inline initialization. >>> >>> However, let's mark both functions and variables weak to fix the issue >>> and avoid leaving unused variables around. Will update my patch in a minute. >> Ok. I am reverting this patch until you provide me with another >> solution. It causes latency to segfault purely and simply at startup on >> my dual PIII. >> > > Cannot reproduce yet. Do you have a backtrace? No. But the problem is probably the same as the one signaled by Henri, a misplaced weak directive ending up in a symbol with no address at all. Since the current situation works, I am going to wait for the "clean" fix which puts some code/data in the src/skins/common directory. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 20:33 ` Gilles Chanteperdrix @ 2010-02-10 21:09 ` Jan Kiszka 2010-02-10 21:13 ` Gilles Chanteperdrix 0 siblings, 1 reply; 28+ messages in thread From: Jan Kiszka @ 2010-02-10 21:09 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core [-- Attachment #1: Type: text/plain, Size: 2244 bytes --] Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> Gilles Chanteperdrix wrote: >>>>> GIT version control wrote: >>>>>> Module: xenomai-jki >>>>>> Branch: for-upstream >>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>> >>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>> >>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>> >>>>>> Otherwise the wrong set of time conversion variables might get >>>>>> initialized when using > 1 skin libraries. >>>>> If that would be possible, then it is the conversion variables which >>>>> should made be weak, not the function. >>>>> >>>>> The way I see it, the posix and native skins currently get a different >>>>> set of variables and functions, which works, but with your change, since >>>>> there is only one function, only one set of variable gets initialized by >>>>> the two function calls. And one skin just broke. >>>>> >>>>> Or am I missing something? Does the patch fix a problem you really had? >>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>> there is non-trivial. But I was able to observe that only one set of >>>> functions is used - which is logical considering the weak marks. And >>>> this breaks due to the static inline initialization. >>>> >>>> However, let's mark both functions and variables weak to fix the issue >>>> and avoid leaving unused variables around. Will update my patch in a minute. >>> Ok. I am reverting this patch until you provide me with another >>> solution. It causes latency to segfault purely and simply at startup on >>> my dual PIII. >>> >> Cannot reproduce yet. Do you have a backtrace? > > No. But the problem is probably the same as the one signaled by Henri, > a misplaced weak directive ending up in a symbol with no address at all. > Since the current situation works, I am going to wait for the "clean" > fix which puts some code/data in the src/skins/common directory. > Find it in my tree. But it's not yet well tested. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 21:09 ` Jan Kiszka @ 2010-02-10 21:13 ` Gilles Chanteperdrix 2010-02-10 21:23 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-10 21:13 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: >>>> Jan Kiszka wrote: >>>>> Gilles Chanteperdrix wrote: >>>>>> GIT version control wrote: >>>>>>> Module: xenomai-jki >>>>>>> Branch: for-upstream >>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>> >>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>> >>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>> >>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>> initialized when using > 1 skin libraries. >>>>>> If that would be possible, then it is the conversion variables which >>>>>> should made be weak, not the function. >>>>>> >>>>>> The way I see it, the posix and native skins currently get a different >>>>>> set of variables and functions, which works, but with your change, since >>>>>> there is only one function, only one set of variable gets initialized by >>>>>> the two function calls. And one skin just broke. >>>>>> >>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>> there is non-trivial. But I was able to observe that only one set of >>>>> functions is used - which is logical considering the weak marks. And >>>>> this breaks due to the static inline initialization. >>>>> >>>>> However, let's mark both functions and variables weak to fix the issue >>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>> Ok. I am reverting this patch until you provide me with another >>>> solution. It causes latency to segfault purely and simply at startup on >>>> my dual PIII. >>>> >>> Cannot reproduce yet. Do you have a backtrace? >> No. But the problem is probably the same as the one signaled by Henri, >> a misplaced weak directive ending up in a symbol with no address at all. >> Since the current situation works, I am going to wait for the "clean" >> fix which puts some code/data in the src/skins/common directory. >> > > Find it in my tree. But it's not yet well tested. I do not like it either. Functions which are in src/skins/common should still be weak, since this lib is included in all the skins libraries. xnarch_init_timeconv does not need to be exported, it may only be called in common (bind.c, or timeconv.c, or something like that). xnarch_tsc_to_ns/ns_to_tsc have no reason to be put in arith.h, timeconv.h was about just right. Maybe create asm-generic/timeconv.h? -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 21:13 ` Gilles Chanteperdrix @ 2010-02-10 21:23 ` Jan Kiszka 2010-02-10 21:26 ` Gilles Chanteperdrix 2010-02-10 21:28 ` Gilles Chanteperdrix 0 siblings, 2 replies; 28+ messages in thread From: Jan Kiszka @ 2010-02-10 21:23 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core [-- Attachment #1: Type: text/plain, Size: 3137 bytes --] Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> Gilles Chanteperdrix wrote: >>>>> Jan Kiszka wrote: >>>>>> Gilles Chanteperdrix wrote: >>>>>>> GIT version control wrote: >>>>>>>> Module: xenomai-jki >>>>>>>> Branch: for-upstream >>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>> >>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>> >>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>> >>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>> initialized when using > 1 skin libraries. >>>>>>> If that would be possible, then it is the conversion variables which >>>>>>> should made be weak, not the function. >>>>>>> >>>>>>> The way I see it, the posix and native skins currently get a different >>>>>>> set of variables and functions, which works, but with your change, since >>>>>>> there is only one function, only one set of variable gets initialized by >>>>>>> the two function calls. And one skin just broke. >>>>>>> >>>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>>> there is non-trivial. But I was able to observe that only one set of >>>>>> functions is used - which is logical considering the weak marks. And >>>>>> this breaks due to the static inline initialization. >>>>>> >>>>>> However, let's mark both functions and variables weak to fix the issue >>>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>>> Ok. I am reverting this patch until you provide me with another >>>>> solution. It causes latency to segfault purely and simply at startup on >>>>> my dual PIII. >>>>> >>>> Cannot reproduce yet. Do you have a backtrace? >>> No. But the problem is probably the same as the one signaled by Henri, >>> a misplaced weak directive ending up in a symbol with no address at all. >>> Since the current situation works, I am going to wait for the "clean" >>> fix which puts some code/data in the src/skins/common directory. >>> >> Find it in my tree. But it's not yet well tested. > > I do not like it either. Functions which are in src/skins/common should > still be weak, since this lib is included in all the skins libraries. Those functions are now in libxeno_common only, so I see no point in allowing them to be overloaded. > > xnarch_init_timeconv does not need to be exported, it may only be called > in common (bind.c, or timeconv.c, or something like that). If xnarch_init_timeconv shall be executed unconditionally, I will happily make it static inline again and call it from xeno_bind_skin_opt (which would imply retrieving sysinfo as well). > > xnarch_tsc_to_ns/ns_to_tsc have no reason to be put in arith.h, > timeconv.h was about just right. Maybe create asm-generic/timeconv.h? > Makes sense. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 21:23 ` Jan Kiszka @ 2010-02-10 21:26 ` Gilles Chanteperdrix 2010-02-10 21:34 ` Jan Kiszka 2010-02-10 21:28 ` Gilles Chanteperdrix 1 sibling, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-10 21:26 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: >>>> Jan Kiszka wrote: >>>>> Gilles Chanteperdrix wrote: >>>>>> Jan Kiszka wrote: >>>>>>> Gilles Chanteperdrix wrote: >>>>>>>> GIT version control wrote: >>>>>>>>> Module: xenomai-jki >>>>>>>>> Branch: for-upstream >>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>> >>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>>> >>>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>>> >>>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>>> initialized when using > 1 skin libraries. >>>>>>>> If that would be possible, then it is the conversion variables which >>>>>>>> should made be weak, not the function. >>>>>>>> >>>>>>>> The way I see it, the posix and native skins currently get a different >>>>>>>> set of variables and functions, which works, but with your change, since >>>>>>>> there is only one function, only one set of variable gets initialized by >>>>>>>> the two function calls. And one skin just broke. >>>>>>>> >>>>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>>>> there is non-trivial. But I was able to observe that only one set of >>>>>>> functions is used - which is logical considering the weak marks. And >>>>>>> this breaks due to the static inline initialization. >>>>>>> >>>>>>> However, let's mark both functions and variables weak to fix the issue >>>>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>>>> Ok. I am reverting this patch until you provide me with another >>>>>> solution. It causes latency to segfault purely and simply at startup on >>>>>> my dual PIII. >>>>>> >>>>> Cannot reproduce yet. Do you have a backtrace? >>>> No. But the problem is probably the same as the one signaled by Henri, >>>> a misplaced weak directive ending up in a symbol with no address at all. >>>> Since the current situation works, I am going to wait for the "clean" >>>> fix which puts some code/data in the src/skins/common directory. >>>> >>> Find it in my tree. But it's not yet well tested. >> I do not like it either. Functions which are in src/skins/common should >> still be weak, since this lib is included in all the skins libraries. > > Those functions are now in libxeno_common only, so I see no point in > allowing them to be overloaded. Yes, but libxeno_common is included in libpthread_rt.so and libnative.so. So, if you link with both libraries, you get libxeno_common twice. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 21:26 ` Gilles Chanteperdrix @ 2010-02-10 21:34 ` Jan Kiszka 2010-02-10 21:39 ` Gilles Chanteperdrix 0 siblings, 1 reply; 28+ messages in thread From: Jan Kiszka @ 2010-02-10 21:34 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core [-- Attachment #1: Type: text/plain, Size: 3091 bytes --] Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> Gilles Chanteperdrix wrote: >>>>> Jan Kiszka wrote: >>>>>> Gilles Chanteperdrix wrote: >>>>>>> Jan Kiszka wrote: >>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>> GIT version control wrote: >>>>>>>>>> Module: xenomai-jki >>>>>>>>>> Branch: for-upstream >>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>> >>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>>>> >>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>>>> >>>>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>>>> initialized when using > 1 skin libraries. >>>>>>>>> If that would be possible, then it is the conversion variables which >>>>>>>>> should made be weak, not the function. >>>>>>>>> >>>>>>>>> The way I see it, the posix and native skins currently get a different >>>>>>>>> set of variables and functions, which works, but with your change, since >>>>>>>>> there is only one function, only one set of variable gets initialized by >>>>>>>>> the two function calls. And one skin just broke. >>>>>>>>> >>>>>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>>>>> there is non-trivial. But I was able to observe that only one set of >>>>>>>> functions is used - which is logical considering the weak marks. And >>>>>>>> this breaks due to the static inline initialization. >>>>>>>> >>>>>>>> However, let's mark both functions and variables weak to fix the issue >>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>>>>> Ok. I am reverting this patch until you provide me with another >>>>>>> solution. It causes latency to segfault purely and simply at startup on >>>>>>> my dual PIII. >>>>>>> >>>>>> Cannot reproduce yet. Do you have a backtrace? >>>>> No. But the problem is probably the same as the one signaled by Henri, >>>>> a misplaced weak directive ending up in a symbol with no address at all. >>>>> Since the current situation works, I am going to wait for the "clean" >>>>> fix which puts some code/data in the src/skins/common directory. >>>>> >>>> Find it in my tree. But it's not yet well tested. >>> I do not like it either. Functions which are in src/skins/common should >>> still be weak, since this lib is included in all the skins libraries. >> Those functions are now in libxeno_common only, so I see no point in >> allowing them to be overloaded. > > Yes, but libxeno_common is included in libpthread_rt.so and > libnative.so. So, if you link with both libraries, you get > libxeno_common twice. > Do we link libxeno_common statically? Otherwise, this conflict is not logical to me. Also, there are other symbols in bind.c that are non-weak. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 21:34 ` Jan Kiszka @ 2010-02-10 21:39 ` Gilles Chanteperdrix 2010-02-11 8:38 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-10 21:39 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: >>>> Jan Kiszka wrote: >>>>> Gilles Chanteperdrix wrote: >>>>>> Jan Kiszka wrote: >>>>>>> Gilles Chanteperdrix wrote: >>>>>>>> Jan Kiszka wrote: >>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>> GIT version control wrote: >>>>>>>>>>> Module: xenomai-jki >>>>>>>>>>> Branch: for-upstream >>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>> >>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>>>>> >>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>>>>> >>>>>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>>>>> initialized when using > 1 skin libraries. >>>>>>>>>> If that would be possible, then it is the conversion variables which >>>>>>>>>> should made be weak, not the function. >>>>>>>>>> >>>>>>>>>> The way I see it, the posix and native skins currently get a different >>>>>>>>>> set of variables and functions, which works, but with your change, since >>>>>>>>>> there is only one function, only one set of variable gets initialized by >>>>>>>>>> the two function calls. And one skin just broke. >>>>>>>>>> >>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>>>>>> there is non-trivial. But I was able to observe that only one set of >>>>>>>>> functions is used - which is logical considering the weak marks. And >>>>>>>>> this breaks due to the static inline initialization. >>>>>>>>> >>>>>>>>> However, let's mark both functions and variables weak to fix the issue >>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>>>>>> Ok. I am reverting this patch until you provide me with another >>>>>>>> solution. It causes latency to segfault purely and simply at startup on >>>>>>>> my dual PIII. >>>>>>>> >>>>>>> Cannot reproduce yet. Do you have a backtrace? >>>>>> No. But the problem is probably the same as the one signaled by Henri, >>>>>> a misplaced weak directive ending up in a symbol with no address at all. >>>>>> Since the current situation works, I am going to wait for the "clean" >>>>>> fix which puts some code/data in the src/skins/common directory. >>>>>> >>>>> Find it in my tree. But it's not yet well tested. >>>> I do not like it either. Functions which are in src/skins/common should >>>> still be weak, since this lib is included in all the skins libraries. >>> Those functions are now in libxeno_common only, so I see no point in >>> allowing them to be overloaded. >> Yes, but libxeno_common is included in libpthread_rt.so and >> libnative.so. So, if you link with both libraries, you get >> libxeno_common twice. >> > > Do we link libxeno_common statically? Otherwise, this conflict is not > logical to me. Also, there are other symbols in bind.c that are non-weak. libxeno_common is a "convenience library", which means that when libpthread_rt.so is assembled, the object files from libxeno_common are included. I guess the loader eliminates the duplicates. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 21:39 ` Gilles Chanteperdrix @ 2010-02-11 8:38 ` Jan Kiszka 2010-02-11 8:47 ` Gilles Chanteperdrix 0 siblings, 1 reply; 28+ messages in thread From: Jan Kiszka @ 2010-02-11 8:38 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> Gilles Chanteperdrix wrote: >>>>> Jan Kiszka wrote: >>>>>> Gilles Chanteperdrix wrote: >>>>>>> Jan Kiszka wrote: >>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>> Jan Kiszka wrote: >>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>> GIT version control wrote: >>>>>>>>>>>> Module: xenomai-jki >>>>>>>>>>>> Branch: for-upstream >>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>> >>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>>>>>> >>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>>>>>> >>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>>>>>> initialized when using > 1 skin libraries. >>>>>>>>>>> If that would be possible, then it is the conversion variables which >>>>>>>>>>> should made be weak, not the function. >>>>>>>>>>> >>>>>>>>>>> The way I see it, the posix and native skins currently get a different >>>>>>>>>>> set of variables and functions, which works, but with your change, since >>>>>>>>>>> there is only one function, only one set of variable gets initialized by >>>>>>>>>>> the two function calls. And one skin just broke. >>>>>>>>>>> >>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>>>>>>> there is non-trivial. But I was able to observe that only one set of >>>>>>>>>> functions is used - which is logical considering the weak marks. And >>>>>>>>>> this breaks due to the static inline initialization. >>>>>>>>>> >>>>>>>>>> However, let's mark both functions and variables weak to fix the issue >>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>>>>>>> Ok. I am reverting this patch until you provide me with another >>>>>>>>> solution. It causes latency to segfault purely and simply at startup on >>>>>>>>> my dual PIII. >>>>>>>>> >>>>>>>> Cannot reproduce yet. Do you have a backtrace? >>>>>>> No. But the problem is probably the same as the one signaled by Henri, >>>>>>> a misplaced weak directive ending up in a symbol with no address at all. >>>>>>> Since the current situation works, I am going to wait for the "clean" >>>>>>> fix which puts some code/data in the src/skins/common directory. >>>>>>> >>>>>> Find it in my tree. But it's not yet well tested. >>>>> I do not like it either. Functions which are in src/skins/common should >>>>> still be weak, since this lib is included in all the skins libraries. >>>> Those functions are now in libxeno_common only, so I see no point in >>>> allowing them to be overloaded. >>> Yes, but libxeno_common is included in libpthread_rt.so and >>> libnative.so. So, if you link with both libraries, you get >>> libxeno_common twice. >>> >> Do we link libxeno_common statically? Otherwise, this conflict is not >> logical to me. Also, there are other symbols in bind.c that are non-weak. > > libxeno_common is a "convenience library", which means that when > libpthread_rt.so is assembled, the object files from libxeno_common are > included. BTW, what speaks against making it a dynamic library? > > I guess the loader eliminates the duplicates. > It has to. For the same reason, we should be able to clean up skins/common/current.c now. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 8:38 ` Jan Kiszka @ 2010-02-11 8:47 ` Gilles Chanteperdrix 2010-02-11 9:37 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-11 8:47 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: >>>> Jan Kiszka wrote: >>>>> Gilles Chanteperdrix wrote: >>>>>> Jan Kiszka wrote: >>>>>>> Gilles Chanteperdrix wrote: >>>>>>>> Jan Kiszka wrote: >>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>> Jan Kiszka wrote: >>>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>>> GIT version control wrote: >>>>>>>>>>>>> Module: xenomai-jki >>>>>>>>>>>>> Branch: for-upstream >>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>> >>>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>>>>>>> >>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>>>>>>> >>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>>>>>>> initialized when using > 1 skin libraries. >>>>>>>>>>>> If that would be possible, then it is the conversion variables which >>>>>>>>>>>> should made be weak, not the function. >>>>>>>>>>>> >>>>>>>>>>>> The way I see it, the posix and native skins currently get a different >>>>>>>>>>>> set of variables and functions, which works, but with your change, since >>>>>>>>>>>> there is only one function, only one set of variable gets initialized by >>>>>>>>>>>> the two function calls. And one skin just broke. >>>>>>>>>>>> >>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>>>>>>>> there is non-trivial. But I was able to observe that only one set of >>>>>>>>>>> functions is used - which is logical considering the weak marks. And >>>>>>>>>>> this breaks due to the static inline initialization. >>>>>>>>>>> >>>>>>>>>>> However, let's mark both functions and variables weak to fix the issue >>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>>>>>>>> Ok. I am reverting this patch until you provide me with another >>>>>>>>>> solution. It causes latency to segfault purely and simply at startup on >>>>>>>>>> my dual PIII. >>>>>>>>>> >>>>>>>>> Cannot reproduce yet. Do you have a backtrace? >>>>>>>> No. But the problem is probably the same as the one signaled by Henri, >>>>>>>> a misplaced weak directive ending up in a symbol with no address at all. >>>>>>>> Since the current situation works, I am going to wait for the "clean" >>>>>>>> fix which puts some code/data in the src/skins/common directory. >>>>>>>> >>>>>>> Find it in my tree. But it's not yet well tested. >>>>>> I do not like it either. Functions which are in src/skins/common should >>>>>> still be weak, since this lib is included in all the skins libraries. >>>>> Those functions are now in libxeno_common only, so I see no point in >>>>> allowing them to be overloaded. >>>> Yes, but libxeno_common is included in libpthread_rt.so and >>>> libnative.so. So, if you link with both libraries, you get >>>> libxeno_common twice. >>>> >>> Do we link libxeno_common statically? Otherwise, this conflict is not >>> logical to me. Also, there are other symbols in bind.c that are non-weak. >> libxeno_common is a "convenience library", which means that when >> libpthread_rt.so is assembled, the object files from libxeno_common are >> included. > > BTW, what speaks against making it a dynamic library? Complications. Currently the way of linking a native application is to do: `xeno-config --ldflags` -lnative Since when linking, the order matter, we can not hide this new library in xeno-config --ldflags, so we would have to turn this into: `xeno-config --ldflags` -lnative -lxeno-common i.e. breaking user way of building applications. I do not think the gain is worth the trouble: linking against only one xenomai library looks to me like the most common case. > >> I guess the loader eliminates the duplicates. >> > > It has to. For the same reason, we should be able to clean up > skins/common/current.c now. I am not that confident about that change. By using the weak directive, we make sure that the same __thread variable is used for instance. If we do not do that, what prevents the code from using the local symbols in each library ? IOW, removing the duplicate could work for libxeno_common symbols when invoked externally (which probably almost never happens), but not inside the skin libraries. I do not know about that, which is why I kept the weak declarations. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 8:47 ` Gilles Chanteperdrix @ 2010-02-11 9:37 ` Jan Kiszka 2010-02-11 9:46 ` Gilles Chanteperdrix 0 siblings, 1 reply; 28+ messages in thread From: Jan Kiszka @ 2010-02-11 9:37 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> Gilles Chanteperdrix wrote: >>>>> Jan Kiszka wrote: >>>>>> Gilles Chanteperdrix wrote: >>>>>>> Jan Kiszka wrote: >>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>> Jan Kiszka wrote: >>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>> Jan Kiszka wrote: >>>>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>>>> GIT version control wrote: >>>>>>>>>>>>>> Module: xenomai-jki >>>>>>>>>>>>>> Branch: for-upstream >>>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>>> >>>>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>>>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>>>>>>>> >>>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>>>>>>>> >>>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>>>>>>>> initialized when using > 1 skin libraries. >>>>>>>>>>>>> If that would be possible, then it is the conversion variables which >>>>>>>>>>>>> should made be weak, not the function. >>>>>>>>>>>>> >>>>>>>>>>>>> The way I see it, the posix and native skins currently get a different >>>>>>>>>>>>> set of variables and functions, which works, but with your change, since >>>>>>>>>>>>> there is only one function, only one set of variable gets initialized by >>>>>>>>>>>>> the two function calls. And one skin just broke. >>>>>>>>>>>>> >>>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>>>>>>>>> there is non-trivial. But I was able to observe that only one set of >>>>>>>>>>>> functions is used - which is logical considering the weak marks. And >>>>>>>>>>>> this breaks due to the static inline initialization. >>>>>>>>>>>> >>>>>>>>>>>> However, let's mark both functions and variables weak to fix the issue >>>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>>>>>>>>> Ok. I am reverting this patch until you provide me with another >>>>>>>>>>> solution. It causes latency to segfault purely and simply at startup on >>>>>>>>>>> my dual PIII. >>>>>>>>>>> >>>>>>>>>> Cannot reproduce yet. Do you have a backtrace? >>>>>>>>> No. But the problem is probably the same as the one signaled by Henri, >>>>>>>>> a misplaced weak directive ending up in a symbol with no address at all. >>>>>>>>> Since the current situation works, I am going to wait for the "clean" >>>>>>>>> fix which puts some code/data in the src/skins/common directory. >>>>>>>>> >>>>>>>> Find it in my tree. But it's not yet well tested. >>>>>>> I do not like it either. Functions which are in src/skins/common should >>>>>>> still be weak, since this lib is included in all the skins libraries. >>>>>> Those functions are now in libxeno_common only, so I see no point in >>>>>> allowing them to be overloaded. >>>>> Yes, but libxeno_common is included in libpthread_rt.so and >>>>> libnative.so. So, if you link with both libraries, you get >>>>> libxeno_common twice. >>>>> >>>> Do we link libxeno_common statically? Otherwise, this conflict is not >>>> logical to me. Also, there are other symbols in bind.c that are non-weak. >>> libxeno_common is a "convenience library", which means that when >>> libpthread_rt.so is assembled, the object files from libxeno_common are >>> included. >> BTW, what speaks against making it a dynamic library? > > Complications. Currently the way of linking a native application is to do: > > `xeno-config --ldflags` -lnative > > Since when linking, the order matter, we can not hide this new library > in xeno-config --ldflags, so we would have to turn this into: > > `xeno-config --ldflags` -lnative -lxeno-common > > i.e. breaking user way of building applications. I do not think the gain > is worth the trouble: linking against only one xenomai library looks to > me like the most common case. What would break when swapping -lnative and -xeno-common arbitrarily? There is a _lot_ of cleanup potential. E.g. all that redundant __real wrappers are good candidates for libxeno-common. > > >>> I guess the loader eliminates the duplicates. >>> >> It has to. For the same reason, we should be able to clean up >> skins/common/current.c now. > > I am not that confident about that change. By using the weak directive, > we make sure that the same __thread variable is used for instance. If we > do not do that, what prevents the code from using the local symbols in > each library ? IOW, removing the duplicate could work for libxeno_common > symbols when invoked externally (which probably almost never happens), > but not inside the skin libraries. I do not know about that, which is > why I kept the weak declarations. Mixing weak functions with non-weak variables can cause troubles as well. Sticking our head into the sand is no good approach, understanding the requirements for weak or not is better - and eliminating weak would be best. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 9:37 ` Jan Kiszka @ 2010-02-11 9:46 ` Gilles Chanteperdrix 2010-02-11 10:36 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-11 9:46 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: >>>> Jan Kiszka wrote: >>>>> Gilles Chanteperdrix wrote: >>>>>> Jan Kiszka wrote: >>>>>>> Gilles Chanteperdrix wrote: >>>>>>>> Jan Kiszka wrote: >>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>> Jan Kiszka wrote: >>>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>>> Jan Kiszka wrote: >>>>>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>>>>> GIT version control wrote: >>>>>>>>>>>>>>> Module: xenomai-jki >>>>>>>>>>>>>>> Branch: for-upstream >>>>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>>>>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>>>>>>>>> initialized when using > 1 skin libraries. >>>>>>>>>>>>>> If that would be possible, then it is the conversion variables which >>>>>>>>>>>>>> should made be weak, not the function. >>>>>>>>>>>>>> >>>>>>>>>>>>>> The way I see it, the posix and native skins currently get a different >>>>>>>>>>>>>> set of variables and functions, which works, but with your change, since >>>>>>>>>>>>>> there is only one function, only one set of variable gets initialized by >>>>>>>>>>>>>> the two function calls. And one skin just broke. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>>>>>>>>>> there is non-trivial. But I was able to observe that only one set of >>>>>>>>>>>>> functions is used - which is logical considering the weak marks. And >>>>>>>>>>>>> this breaks due to the static inline initialization. >>>>>>>>>>>>> >>>>>>>>>>>>> However, let's mark both functions and variables weak to fix the issue >>>>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>>>>>>>>>> Ok. I am reverting this patch until you provide me with another >>>>>>>>>>>> solution. It causes latency to segfault purely and simply at startup on >>>>>>>>>>>> my dual PIII. >>>>>>>>>>>> >>>>>>>>>>> Cannot reproduce yet. Do you have a backtrace? >>>>>>>>>> No. But the problem is probably the same as the one signaled by Henri, >>>>>>>>>> a misplaced weak directive ending up in a symbol with no address at all. >>>>>>>>>> Since the current situation works, I am going to wait for the "clean" >>>>>>>>>> fix which puts some code/data in the src/skins/common directory. >>>>>>>>>> >>>>>>>>> Find it in my tree. But it's not yet well tested. >>>>>>>> I do not like it either. Functions which are in src/skins/common should >>>>>>>> still be weak, since this lib is included in all the skins libraries. >>>>>>> Those functions are now in libxeno_common only, so I see no point in >>>>>>> allowing them to be overloaded. >>>>>> Yes, but libxeno_common is included in libpthread_rt.so and >>>>>> libnative.so. So, if you link with both libraries, you get >>>>>> libxeno_common twice. >>>>>> >>>>> Do we link libxeno_common statically? Otherwise, this conflict is not >>>>> logical to me. Also, there are other symbols in bind.c that are non-weak. >>>> libxeno_common is a "convenience library", which means that when >>>> libpthread_rt.so is assembled, the object files from libxeno_common are >>>> included. >>> BTW, what speaks against making it a dynamic library? >> Complications. Currently the way of linking a native application is to do: >> >> `xeno-config --ldflags` -lnative >> >> Since when linking, the order matter, we can not hide this new library >> in xeno-config --ldflags, so we would have to turn this into: >> >> `xeno-config --ldflags` -lnative -lxeno-common >> >> i.e. breaking user way of building applications. I do not think the gain >> is worth the trouble: linking against only one xenomai library looks to >> me like the most common case. > > What would break when swapping -lnative and -xeno-common arbitrarily? If you are linking with static libraries (which is the case that matters), the linker would skip all objects from xeno-common completely since none of their symbols are required, then fail after libnative because of missing symbols (defined in libxeno_common). > > There is a _lot_ of cleanup potential. E.g. all that redundant __real > wrappers are good candidates for libxeno-common. Thanks to the wrap-link.sh script, the __real wrappers are no longer required. Besides, they are only needed for the posix skin, so there is no need to put them in libxeno-common. This is something which needs to be fixed too. > >> >>>> I guess the loader eliminates the duplicates. >>>> >>> It has to. For the same reason, we should be able to clean up >>> skins/common/current.c now. >> I am not that confident about that change. By using the weak directive, >> we make sure that the same __thread variable is used for instance. If we >> do not do that, what prevents the code from using the local symbols in >> each library ? IOW, removing the duplicate could work for libxeno_common >> symbols when invoked externally (which probably almost never happens), >> but not inside the skin libraries. I do not know about that, which is >> why I kept the weak declarations. > > Mixing weak functions with non-weak variables can cause troubles as > well. Sticking our head into the sand is no good approach, understanding > the requirements for weak or not is better - and eliminating weak would > be best. Yes, all non static variables must be made weak. But this should already be the case in libxeno_common. Using the weak attribute is the best approach. Having symbols multiply defined is a bad idea and not "standard compliant". The problem is not to understand what happens with one instance of a toolchain, the problem is to know whether the behaviour is the same on all the toolchains of all the seven architectures supported by Xenomai. The --wrap directive proved that not all versions of all toolchains on all platforms are equal. The only alternative I would accept is a guaranteed portable ld directive that would make all the symbols of the library automagically weak. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 9:46 ` Gilles Chanteperdrix @ 2010-02-11 10:36 ` Jan Kiszka 2010-02-11 11:04 ` Gilles Chanteperdrix 0 siblings, 1 reply; 28+ messages in thread From: Jan Kiszka @ 2010-02-11 10:36 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> Gilles Chanteperdrix wrote: >>>>> Jan Kiszka wrote: >>>>>> Gilles Chanteperdrix wrote: >>>>>>> Jan Kiszka wrote: >>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>> Jan Kiszka wrote: >>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>> Jan Kiszka wrote: >>>>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>>>> Jan Kiszka wrote: >>>>>>>>>>>>>> Gilles Chanteperdrix wrote: >>>>>>>>>>>>>>> GIT version control wrote: >>>>>>>>>>>>>>>> Module: xenomai-jki >>>>>>>>>>>>>>>> Branch: for-upstream >>>>>>>>>>>>>>>> Commit: 6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>>>>> URL: http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=6b40653e9c3c4a2433bb4e91344fc378eb860f75 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid> >>>>>>>>>>>>>>>> Date: Wed Feb 10 13:24:29 2010 +0100 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Make xnarch_init_timeconv an uninlined weak function >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Otherwise the wrong set of time conversion variables might get >>>>>>>>>>>>>>>> initialized when using > 1 skin libraries. >>>>>>>>>>>>>>> If that would be possible, then it is the conversion variables which >>>>>>>>>>>>>>> should made be weak, not the function. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> The way I see it, the posix and native skins currently get a different >>>>>>>>>>>>>>> set of variables and functions, which works, but with your change, since >>>>>>>>>>>>>>> there is only one function, only one set of variable gets initialized by >>>>>>>>>>>>>>> the two function calls. And one skin just broke. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Or am I missing something? Does the patch fix a problem you really had? >>>>>>>>>>>>>> Frankly, I wasn't able to test in the field yet as replacing the libs >>>>>>>>>>>>>> there is non-trivial. But I was able to observe that only one set of >>>>>>>>>>>>>> functions is used - which is logical considering the weak marks. And >>>>>>>>>>>>>> this breaks due to the static inline initialization. >>>>>>>>>>>>>> >>>>>>>>>>>>>> However, let's mark both functions and variables weak to fix the issue >>>>>>>>>>>>>> and avoid leaving unused variables around. Will update my patch in a minute. >>>>>>>>>>>>> Ok. I am reverting this patch until you provide me with another >>>>>>>>>>>>> solution. It causes latency to segfault purely and simply at startup on >>>>>>>>>>>>> my dual PIII. >>>>>>>>>>>>> >>>>>>>>>>>> Cannot reproduce yet. Do you have a backtrace? >>>>>>>>>>> No. But the problem is probably the same as the one signaled by Henri, >>>>>>>>>>> a misplaced weak directive ending up in a symbol with no address at all. >>>>>>>>>>> Since the current situation works, I am going to wait for the "clean" >>>>>>>>>>> fix which puts some code/data in the src/skins/common directory. >>>>>>>>>>> >>>>>>>>>> Find it in my tree. But it's not yet well tested. >>>>>>>>> I do not like it either. Functions which are in src/skins/common should >>>>>>>>> still be weak, since this lib is included in all the skins libraries. >>>>>>>> Those functions are now in libxeno_common only, so I see no point in >>>>>>>> allowing them to be overloaded. >>>>>>> Yes, but libxeno_common is included in libpthread_rt.so and >>>>>>> libnative.so. So, if you link with both libraries, you get >>>>>>> libxeno_common twice. >>>>>>> >>>>>> Do we link libxeno_common statically? Otherwise, this conflict is not >>>>>> logical to me. Also, there are other symbols in bind.c that are non-weak. >>>>> libxeno_common is a "convenience library", which means that when >>>>> libpthread_rt.so is assembled, the object files from libxeno_common are >>>>> included. >>>> BTW, what speaks against making it a dynamic library? >>> Complications. Currently the way of linking a native application is to do: >>> >>> `xeno-config --ldflags` -lnative >>> >>> Since when linking, the order matter, we can not hide this new library >>> in xeno-config --ldflags, so we would have to turn this into: >>> >>> `xeno-config --ldflags` -lnative -lxeno-common >>> >>> i.e. breaking user way of building applications. I do not think the gain >>> is worth the trouble: linking against only one xenomai library looks to >>> me like the most common case. >> What would break when swapping -lnative and -xeno-common arbitrarily? > > If you are linking with static libraries (which is the case that > matters), the linker would skip all objects from xeno-common completely > since none of their symbols are required, then fail after libnative > because of missing symbols (defined in libxeno_common). OK. But I this is a custom scenario and naturally requires careful library ordering by the user anyway. For us the rule is simple: put front-end libs to the front, push the xeno LDFLAGS to the end. BTW, if you link via "`xeno-config --xeno-ldflags` -lnative --static", things break already today (as pthread is processed before native). Adding -lxeno_common to the output of xeno-config would not change the situation. > >> There is a _lot_ of cleanup potential. E.g. all that redundant __real >> wrappers are good candidates for libxeno-common. > > Thanks to the wrap-link.sh script, the __real wrappers are no longer > required. Besides, they are only needed for the posix skin, so there is > no need to put them in libxeno-common. This is something which needs to > be fixed too. wrap-link.sh is a convenience script, I don't think we could make its use mandatory. So every skin that uses wrapped symbols should remain hardened. > >>>>> I guess the loader eliminates the duplicates. >>>>> >>>> It has to. For the same reason, we should be able to clean up >>>> skins/common/current.c now. >>> I am not that confident about that change. By using the weak directive, >>> we make sure that the same __thread variable is used for instance. If we >>> do not do that, what prevents the code from using the local symbols in >>> each library ? IOW, removing the duplicate could work for libxeno_common >>> symbols when invoked externally (which probably almost never happens), >>> but not inside the skin libraries. I do not know about that, which is >>> why I kept the weak declarations. >> Mixing weak functions with non-weak variables can cause troubles as >> well. Sticking our head into the sand is no good approach, understanding >> the requirements for weak or not is better - and eliminating weak would >> be best. > > Yes, all non static variables must be made weak. But this should already > be the case in libxeno_common. Yes, and all static inlines must be made global or the static symbols they access must become weak - that's where the bugs are hidden. > > Using the weak attribute is the best approach. Having symbols multiply > defined is a bad idea and not "standard compliant". The problem is not > to understand what happens with one instance of a toolchain, the problem > is to know whether the behaviour is the same on all the toolchains of > all the seven architectures supported by Xenomai. The --wrap directive > proved that not all versions of all toolchains on all platforms are equal. > > The only alternative I would accept is a guaranteed portable ld > directive that would make all the symbols of the library automagically weak. I'm convinced that weak is the non-standard approach here. If we need to mark commonly used functions weak, it indicates somethings is not yet optimally organized. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 10:36 ` Jan Kiszka @ 2010-02-11 11:04 ` Gilles Chanteperdrix 2010-02-11 14:14 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-11 11:04 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: >>>>> BTW, what speaks against making it a dynamic library? >>>> Complications. Currently the way of linking a native application is to do: >>>> >>>> `xeno-config --ldflags` -lnative >>>> >>>> Since when linking, the order matter, we can not hide this new library >>>> in xeno-config --ldflags, so we would have to turn this into: >>>> >>>> `xeno-config --ldflags` -lnative -lxeno-common >>>> >>>> i.e. breaking user way of building applications. I do not think the gain >>>> is worth the trouble: linking against only one xenomai library looks to >>>> me like the most common case. >>> What would break when swapping -lnative and -xeno-common arbitrarily? >> If you are linking with static libraries (which is the case that >> matters), the linker would skip all objects from xeno-common completely >> since none of their symbols are required, then fail after libnative >> because of missing symbols (defined in libxeno_common). > > OK. But I this is a custom scenario and naturally requires careful > library ordering by the user anyway. For us the rule is simple: put > front-end libs to the front, push the xeno LDFLAGS to the end. It does not fly either, the -L/usr/xenomai/lib is in the LDFLAGS, and must be put before -lnative. It would be nice to to add a --skin option to the xeno-config script, which handles that. But again, we break the user interface. that is: xeno-config --skin=native --ldflags would spit: -L/usr/xenomai/lib -lnative -lxeno-common -lpthread -lrt > > BTW, if you link via "`xeno-config --xeno-ldflags` -lnative --static", > things break already today (as pthread is processed before native). > Adding -lxeno_common to the output of xeno-config would not change the > situation. I guess it works because people usually use some pthread symbols in their program already. > >>> There is a _lot_ of cleanup potential. E.g. all that redundant __real >>> wrappers are good candidates for libxeno-common. >> Thanks to the wrap-link.sh script, the __real wrappers are no longer >> required. Besides, they are only needed for the posix skin, so there is >> no need to put them in libxeno-common. This is something which needs to >> be fixed too. > > wrap-link.sh is a convenience script, I don't think we could make its > use mandatory. So every skin that uses wrapped symbols should remain > hardened. wrap-link.sh is already mandatory if you want to link statically with the posix skin library. Which is also the case why the __real wrappers exist at all. > >>>>>> I guess the loader eliminates the duplicates. >>>>>> >>>>> It has to. For the same reason, we should be able to clean up >>>>> skins/common/current.c now. >>>> I am not that confident about that change. By using the weak directive, >>>> we make sure that the same __thread variable is used for instance. If we >>>> do not do that, what prevents the code from using the local symbols in >>>> each library ? IOW, removing the duplicate could work for libxeno_common >>>> symbols when invoked externally (which probably almost never happens), >>>> but not inside the skin libraries. I do not know about that, which is >>>> why I kept the weak declarations. >>> Mixing weak functions with non-weak variables can cause troubles as >>> well. Sticking our head into the sand is no good approach, understanding >>> the requirements for weak or not is better - and eliminating weak would >>> be best. >> Yes, all non static variables must be made weak. But this should already >> be the case in libxeno_common. > > Yes, and all static inlines must be made global or the static symbols > they access must become weak - that's where the bugs are hidden. Not if the static symbols in all instances are initialized with the same value (which is the case with xnarch_init_timeconv for instance). > >> Using the weak attribute is the best approach. Having symbols multiply >> defined is a bad idea and not "standard compliant". The problem is not >> to understand what happens with one instance of a toolchain, the problem >> is to know whether the behaviour is the same on all the toolchains of >> all the seven architectures supported by Xenomai. The --wrap directive >> proved that not all versions of all toolchains on all platforms are equal. >> >> The only alternative I would accept is a guaranteed portable ld >> directive that would make all the symbols of the library automagically weak. > > I'm convinced that weak is the non-standard approach here. If we need to > mark commonly used functions weak, it indicates somethings is not yet > optimally organized. It is definitely non-standard. But works reliably the same way with gcc on all platforms (well except if you declare some extern symbols weak, but you are looking for trouble when doing that anyway). I think we should not worry too much for these issues, as the current situation works. So, we have plenty of time to decide what to do, and more urgent issues to treat (such as the file descriptors situation, the NTP stuff, or the posix signals to name a few). -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 11:04 ` Gilles Chanteperdrix @ 2010-02-11 14:14 ` Jan Kiszka 2010-02-11 15:59 ` Gilles Chanteperdrix 0 siblings, 1 reply; 28+ messages in thread From: Jan Kiszka @ 2010-02-11 14:14 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >>>>>> BTW, what speaks against making it a dynamic library? >>>>> Complications. Currently the way of linking a native application is to do: >>>>> >>>>> `xeno-config --ldflags` -lnative >>>>> >>>>> Since when linking, the order matter, we can not hide this new library >>>>> in xeno-config --ldflags, so we would have to turn this into: >>>>> >>>>> `xeno-config --ldflags` -lnative -lxeno-common >>>>> >>>>> i.e. breaking user way of building applications. I do not think the gain >>>>> is worth the trouble: linking against only one xenomai library looks to >>>>> me like the most common case. >>>> What would break when swapping -lnative and -xeno-common arbitrarily? >>> If you are linking with static libraries (which is the case that >>> matters), the linker would skip all objects from xeno-common completely >>> since none of their symbols are required, then fail after libnative >>> because of missing symbols (defined in libxeno_common). >> OK. But I this is a custom scenario and naturally requires careful >> library ordering by the user anyway. For us the rule is simple: put >> front-end libs to the front, push the xeno LDFLAGS to the end. > > It does not fly either, the -L/usr/xenomai/lib is in the LDFLAGS, and > must be put before -lnative. But that must be some old toolchain issue. Normally, that doesn't matter at all as the library path are first collected, and then linking starts. > > It would be nice to to add a --skin option to the xeno-config script, > which handles that. But again, we break the user interface. > > that is: > xeno-config --skin=native --ldflags > would spit: > -L/usr/xenomai/lib -lnative -lxeno-common -lpthread -lrt For sure, that would make it most robust against user mistakes. > >> BTW, if you link via "`xeno-config --xeno-ldflags` -lnative --static", >> things break already today (as pthread is processed before native). >> Adding -lxeno_common to the output of xeno-config would not change the >> situation. > > I guess it works because people usually use some pthread symbols in > their program already. > >>>> There is a _lot_ of cleanup potential. E.g. all that redundant __real >>>> wrappers are good candidates for libxeno-common. >>> Thanks to the wrap-link.sh script, the __real wrappers are no longer >>> required. Besides, they are only needed for the posix skin, so there is >>> no need to put them in libxeno-common. This is something which needs to >>> be fixed too. >> wrap-link.sh is a convenience script, I don't think we could make its >> use mandatory. So every skin that uses wrapped symbols should remain >> hardened. > > wrap-link.sh is already mandatory if you want to link statically with > the posix skin library. Which is also the case why the __real wrappers > exist at all. > >>>>>>> I guess the loader eliminates the duplicates. >>>>>>> >>>>>> It has to. For the same reason, we should be able to clean up >>>>>> skins/common/current.c now. >>>>> I am not that confident about that change. By using the weak directive, >>>>> we make sure that the same __thread variable is used for instance. If we >>>>> do not do that, what prevents the code from using the local symbols in >>>>> each library ? IOW, removing the duplicate could work for libxeno_common >>>>> symbols when invoked externally (which probably almost never happens), >>>>> but not inside the skin libraries. I do not know about that, which is >>>>> why I kept the weak declarations. >>>> Mixing weak functions with non-weak variables can cause troubles as >>>> well. Sticking our head into the sand is no good approach, understanding >>>> the requirements for weak or not is better - and eliminating weak would >>>> be best. >>> Yes, all non static variables must be made weak. But this should already >>> be the case in libxeno_common. >> Yes, and all static inlines must be made global or the static symbols >> they access must become weak - that's where the bugs are hidden. > > Not if the static symbols in all instances are initialized with the same > value (which is the case with xnarch_init_timeconv for instance). And instantly - you forget about dlopen scenarios which triggered the bug in timeconv. > >>> Using the weak attribute is the best approach. Having symbols multiply >>> defined is a bad idea and not "standard compliant". The problem is not >>> to understand what happens with one instance of a toolchain, the problem >>> is to know whether the behaviour is the same on all the toolchains of >>> all the seven architectures supported by Xenomai. The --wrap directive >>> proved that not all versions of all toolchains on all platforms are equal. >>> >>> The only alternative I would accept is a guaranteed portable ld >>> directive that would make all the symbols of the library automagically weak. >> I'm convinced that weak is the non-standard approach here. If we need to >> mark commonly used functions weak, it indicates somethings is not yet >> optimally organized. > > It is definitely non-standard. But works reliably the same way with gcc > on all platforms (well except if you declare some extern symbols weak, > but you are looking for trouble when doing that anyway). > > I think we should not worry too much for these issues, as the current > situation works. So, we have plenty of time to decide what to do, and > more urgent issues to treat (such as the file descriptors situation, the > NTP stuff, or the posix signals to name a few). Well, I still have a bug to fix, and doing the requested cleanups just revealed more need for cleanups. So I'm now trying to find a reasonable point to stop the cleanups (for 2.5). Please have a look if the version I just pushed gets closer to what is acceptable. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 14:14 ` Jan Kiszka @ 2010-02-11 15:59 ` Gilles Chanteperdrix 2010-02-11 16:13 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-11 15:59 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > And instantly - you forget about dlopen scenarios which triggered the > bug in timeconv. I do not see why dlopen would trigger a bug. Could you explain it? -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 15:59 ` Gilles Chanteperdrix @ 2010-02-11 16:13 ` Jan Kiszka 2010-02-11 16:16 ` Gilles Chanteperdrix 0 siblings, 1 reply; 28+ messages in thread From: Jan Kiszka @ 2010-02-11 16:13 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> And instantly - you forget about dlopen scenarios which triggered the >> bug in timeconv. > > I do not see why dlopen would trigger a bug. Could you explain it? If you dlopen, say, libnative and call a symbol that uses some timeconv constants, you have to make sure that the init code of libnative initializes those variables that are later used. Currently this is not the case as init_timeconv works on libnative variables while the weak conversion functions may work on a possibly (in our case really) uninitialized set of some other skin. We can work around it, but it remains a bug. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 16:13 ` Jan Kiszka @ 2010-02-11 16:16 ` Gilles Chanteperdrix 2010-02-11 16:42 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-11 16:16 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> And instantly - you forget about dlopen scenarios which triggered the >>> bug in timeconv. >> I do not see why dlopen would trigger a bug. Could you explain it? > > If you dlopen, say, libnative and call a symbol that uses some timeconv > constants, you have to make sure that the init code of libnative > initializes those variables that are later used. That is where I do not follow you. dlopen is called after the startup of the other libs, so either it references its own variables, which are initialized once its constructor has been called. Or it references the variables of the posix lib (only native and posix use timeconv in user-space) which is already initialized. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 16:16 ` Gilles Chanteperdrix @ 2010-02-11 16:42 ` Jan Kiszka 2010-02-11 17:04 ` Gilles Chanteperdrix 0 siblings, 1 reply; 28+ messages in thread From: Jan Kiszka @ 2010-02-11 16:42 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> And instantly - you forget about dlopen scenarios which triggered the >>>> bug in timeconv. >>> I do not see why dlopen would trigger a bug. Could you explain it? >> If you dlopen, say, libnative and call a symbol that uses some timeconv >> constants, you have to make sure that the init code of libnative >> initializes those variables that are later used. > > That is where I do not follow you. dlopen is called after the startup of > the other libs, so either it references its own variables, which are > initialized once its constructor has been called. Or it references the > variables of the posix lib (only native and posix use timeconv in > user-space) which is already initialized. In this case, dlopen was part of some constructor, and the ordering turned out to be "unfortunate". Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 16:42 ` Jan Kiszka @ 2010-02-11 17:04 ` Gilles Chanteperdrix 2010-02-11 17:10 ` Jan Kiszka 0 siblings, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-11 17:04 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: >>>> Jan Kiszka wrote: >>>>> And instantly - you forget about dlopen scenarios which triggered the >>>>> bug in timeconv. >>>> I do not see why dlopen would trigger a bug. Could you explain it? >>> If you dlopen, say, libnative and call a symbol that uses some timeconv >>> constants, you have to make sure that the init code of libnative >>> initializes those variables that are later used. >> That is where I do not follow you. dlopen is called after the startup of >> the other libs, so either it references its own variables, which are >> initialized once its constructor has been called. Or it references the >> variables of the posix lib (only native and posix use timeconv in >> user-space) which is already initialized. > > In this case, dlopen was part of some constructor, and the ordering > turned out to be "unfortunate". Ok. Got it now. Will test your patch, but I will probably leave the weak attribute to the shared functions. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-11 17:04 ` Gilles Chanteperdrix @ 2010-02-11 17:10 ` Jan Kiszka 0 siblings, 0 replies; 28+ messages in thread From: Jan Kiszka @ 2010-02-11 17:10 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>>> Gilles Chanteperdrix wrote: >>>>> Jan Kiszka wrote: >>>>>> And instantly - you forget about dlopen scenarios which triggered the >>>>>> bug in timeconv. >>>>> I do not see why dlopen would trigger a bug. Could you explain it? >>>> If you dlopen, say, libnative and call a symbol that uses some timeconv >>>> constants, you have to make sure that the init code of libnative >>>> initializes those variables that are later used. >>> That is where I do not follow you. dlopen is called after the startup of >>> the other libs, so either it references its own variables, which are >>> initialized once its constructor has been called. Or it references the >>> variables of the posix lib (only native and posix use timeconv in >>> user-space) which is already initialized. >> In this case, dlopen was part of some constructor, and the ordering >> turned out to be "unfortunate". > > Ok. Got it now. Will test your patch, but I will probably leave the weak > attribute to the shared functions. Well, either all or nothing: If you leave it to the runtime services, you also have to tag the init function (that's what my very first tiny patch did). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 21:23 ` Jan Kiszka 2010-02-10 21:26 ` Gilles Chanteperdrix @ 2010-02-10 21:28 ` Gilles Chanteperdrix 2010-02-10 21:35 ` Jan Kiszka 1 sibling, 1 reply; 28+ messages in thread From: Gilles Chanteperdrix @ 2010-02-10 21:28 UTC (permalink / raw) To: Jan Kiszka; +Cc: Xenomai core Jan Kiszka wrote: > If xnarch_init_timeconv shall be executed unconditionally, I will > happily make it static inline again and call it from xeno_bind_skin_opt > (which would imply retrieving sysinfo as well). It is executed unconditionally, but needs to be available to kernel space as well. -- Gilles. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function 2010-02-10 21:28 ` Gilles Chanteperdrix @ 2010-02-10 21:35 ` Jan Kiszka 0 siblings, 0 replies; 28+ messages in thread From: Jan Kiszka @ 2010-02-10 21:35 UTC (permalink / raw) To: Gilles Chanteperdrix; +Cc: Xenomai core [-- Attachment #1: Type: text/plain, Size: 418 bytes --] Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> If xnarch_init_timeconv shall be executed unconditionally, I will >> happily make it static inline again and call it from xeno_bind_skin_opt >> (which would imply retrieving sysinfo as well). > > It is executed unconditionally, but needs to be available to kernel > space as well. Not yet (it was POSIX- and Native-only), but we can change this. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2010-02-11 17:10 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <E1NfBdX-0001ga-4b@domain.hid> 2010-02-10 12:34 ` [Xenomai-core] [Xenomai-git] Jan Kiszka : Make xnarch_init_timeconv an uninlined weak function Gilles Chanteperdrix 2010-02-10 12:43 ` Jan Kiszka 2010-02-10 14:01 ` Gilles Chanteperdrix 2010-02-10 14:48 ` Philippe Gerum 2010-02-10 19:33 ` Gilles Chanteperdrix 2010-02-10 20:24 ` Jan Kiszka 2010-02-10 20:33 ` Gilles Chanteperdrix 2010-02-10 21:09 ` Jan Kiszka 2010-02-10 21:13 ` Gilles Chanteperdrix 2010-02-10 21:23 ` Jan Kiszka 2010-02-10 21:26 ` Gilles Chanteperdrix 2010-02-10 21:34 ` Jan Kiszka 2010-02-10 21:39 ` Gilles Chanteperdrix 2010-02-11 8:38 ` Jan Kiszka 2010-02-11 8:47 ` Gilles Chanteperdrix 2010-02-11 9:37 ` Jan Kiszka 2010-02-11 9:46 ` Gilles Chanteperdrix 2010-02-11 10:36 ` Jan Kiszka 2010-02-11 11:04 ` Gilles Chanteperdrix 2010-02-11 14:14 ` Jan Kiszka 2010-02-11 15:59 ` Gilles Chanteperdrix 2010-02-11 16:13 ` Jan Kiszka 2010-02-11 16:16 ` Gilles Chanteperdrix 2010-02-11 16:42 ` Jan Kiszka 2010-02-11 17:04 ` Gilles Chanteperdrix 2010-02-11 17:10 ` Jan Kiszka 2010-02-10 21:28 ` Gilles Chanteperdrix 2010-02-10 21:35 ` 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.