Re: [PATCH] introduce __init_exit function annotation
On 17/07/07 19:02 +0200, Takashi Iwai wrote:
> At Tue, 17 Jul 2007 18:48:46 +0200,
> Sam Ravnborg wrote:
> >
> > On Tue, Jul 17, 2007 at 05:40:15PM +0200, Takashi Iwai wrote:
> > > At Tue, 17 Jul 2007 17:32:36 +0200,
> > > Sam Ravnborg wrote:
> > > >
> > > > On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
> > > > > At Tue, 17 Jul 2007 17:14:32 +0200,
> > > > > Sam Ravnborg wrote:
> > > > > >
> > > > > > On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
> > > > > > > At Tue, 17 Jul 2007 15:02:30 +0200,
> > > > > > > Sam Ravnborg wrote:
> > > > > > > >
> > > > > > > > On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
> > > > > > > > > Introduce __init_exit, which is useful ie. for drivers that
> > > > > > > > > call
> > > > > > > > > cleanup functions when they fail in __init functions.
> > > > > > > >
> > > > > > > > This is wrong.
> > > > > > > > On arm (just one example of several) the __exit section are
> > > > > > > > discarded
> > > > > > > > at buildtime so any reference from __init to __exit will cause
> > > > > > > > the
> > > > > > > > linker to error out.
> > > > > > >
> > > > > > > Hmm, from what I see, it adds __init to the function. There is no
> > > > > > > reference to __exit.
> > > > > >
> > > > > > The cleanup functions are marked __exit in the referenced case.
> > > > >
> > > > > My understanding is that it's the very purpose of this patch --
> > > > > change the mark from __exit to __init_exit for such clean-up
> > > > > functions.
> > > >
> > > > And that is wrong.
> > >
> > > You misunderstood. What I meant is the case like this:
> > >
> > > static void __init_exit cleanup()
> > > {
> > > ...
> > > }
> > >
> > > static void __init foo_init()
> > > {
> > > if (error)
> > > cleanup();
> > > }
> > >
> > > static void __exit foo_exit()
> > > {
> > > cleanup();
> > > }
> > >
> > > Currently, there is no proper way to mark cleanup(). Neither __init,
> > > __exit, __devinit nor __devexit can be used there.
> >
> > Then you get the annotation sorted out so cleanup() get discarded in the
> > built-in case. But you leave no room for automated tools to detect this.
> >
> > If this is really necessary (and I daught) then a specific section should be
> > dedicated for this usage.
> >
> > We have lot of issues with current __init/__exit, __devinit/__devexit,
> > __cpuint/__cpuexit
> > and introducing more of the kind does not help it.
> > So even if it saves a few bytes in some odd cases the added complaxity is
> > IMHO not worth it.
>
> Well, I don't think it's a few bytes and not so odd, but I agree that
> this solution isn't the best way. And, I now remember that this won't
> work anyway, too. Calling __init from __exit also causes error...
I made this patch because I saw __init calling __exit in yet another
driver (gianfar). Guess I'll just send the old way fix, and remove __exit.
As for calling __init_exit from __exit:
1 - in kernel, there's no __exit => no problem
2 - module, __init_exit is a no-op => no problem
the code in question again:
> #ifdef MODULE
> #define __exit __attribute__ ((__section__(".exit.text")))
> +#define __init_exit
> #else
> #define __exit __attribute_used__ __attribute__
> ((__section__(".exit.text")))
> +#define __init_exit __init
> #endif
Or maybe it's the name that is confuzing, but it makes sense to me:
__init_exit - you can call it from __init or __exit.
__init_or_exit?
Domen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On 17/07/07 17:40 +0200, Takashi Iwai wrote:
> At Tue, 17 Jul 2007 17:32:36 +0200,
> Sam Ravnborg wrote:
> >
> > On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
> > > At Tue, 17 Jul 2007 17:14:32 +0200,
> > > Sam Ravnborg wrote:
> > > >
> > > > On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
> > > > > At Tue, 17 Jul 2007 15:02:30 +0200,
> > > > > Sam Ravnborg wrote:
> > > > > >
> > > > > > On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
> > > > > > > Introduce __init_exit, which is useful ie. for drivers that call
> > > > > > > cleanup functions when they fail in __init functions.
> > > > > >
> > > > > > This is wrong.
> > > > > > On arm (just one example of several) the __exit section are
> > > > > > discarded
> > > > > > at buildtime so any reference from __init to __exit will cause the
> > > > > > linker to error out.
> > > > >
> > > > > Hmm, from what I see, it adds __init to the function. There is no
> > > > > reference to __exit.
> > > >
> > > > The cleanup functions are marked __exit in the referenced case.
> > >
> > > My understanding is that it's the very purpose of this patch --
> > > change the mark from __exit to __init_exit for such clean-up
> > > functions.
> >
> > And that is wrong.
>
> You misunderstood. What I meant is the case like this:
>
> static void __init_exit cleanup()
> {
> ...
> }
>
> static void __init foo_init()
> {
> if (error)
> cleanup();
> }
>
> static void __exit foo_exit()
> {
> cleanup();
> }
Uh, yes, this, or just __init_exit foo_exit() as in Sam's example.
It seemed obvious to me, sorry.
>
> Currently, there is no proper way to mark cleanup(). Neither __init,
> __exit, __devinit nor __devexit can be used there.
>
>
> Takashi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
At Tue, 17 Jul 2007 18:48:46 +0200,
Sam Ravnborg wrote:
>
> On Tue, Jul 17, 2007 at 05:40:15PM +0200, Takashi Iwai wrote:
> > At Tue, 17 Jul 2007 17:32:36 +0200,
> > Sam Ravnborg wrote:
> > >
> > > On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
> > > > At Tue, 17 Jul 2007 17:14:32 +0200,
> > > > Sam Ravnborg wrote:
> > > > >
> > > > > On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
> > > > > > At Tue, 17 Jul 2007 15:02:30 +0200,
> > > > > > Sam Ravnborg wrote:
> > > > > > >
> > > > > > > On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
> > > > > > > > Introduce __init_exit, which is useful ie. for drivers that call
> > > > > > > > cleanup functions when they fail in __init functions.
> > > > > > >
> > > > > > > This is wrong.
> > > > > > > On arm (just one example of several) the __exit section are
> > > > > > > discarded
> > > > > > > at buildtime so any reference from __init to __exit will cause the
> > > > > > > linker to error out.
> > > > > >
> > > > > > Hmm, from what I see, it adds __init to the function. There is no
> > > > > > reference to __exit.
> > > > >
> > > > > The cleanup functions are marked __exit in the referenced case.
> > > >
> > > > My understanding is that it's the very purpose of this patch --
> > > > change the mark from __exit to __init_exit for such clean-up
> > > > functions.
> > >
> > > And that is wrong.
> >
> > You misunderstood. What I meant is the case like this:
> >
> > static void __init_exit cleanup()
> > {
> > ...
> > }
> >
> > static void __init foo_init()
> > {
> > if (error)
> > cleanup();
> > }
> >
> > static void __exit foo_exit()
> > {
> > cleanup();
> > }
> >
> > Currently, there is no proper way to mark cleanup(). Neither __init,
> > __exit, __devinit nor __devexit can be used there.
>
> Then you get the annotation sorted out so cleanup() get discarded in the
> built-in case. But you leave no room for automated tools to detect this.
>
> If this is really necessary (and I daught) then a specific section should be
> dedicated for this usage.
>
> We have lot of issues with current __init/__exit, __devinit/__devexit,
> __cpuint/__cpuexit
> and introducing more of the kind does not help it.
> So even if it saves a few bytes in some odd cases the added complaxity is
> IMHO not worth it.
Well, I don't think it's a few bytes and not so odd, but I agree that
this solution isn't the best way. And, I now remember that this won't
work anyway, too. Calling __init from __exit also causes error...
BTW, this reminds me why we have to add annotations for each
subisdiary function manually. A tool to parse the code statically and
give the proper annotations/hints would be really nice.
Takashi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 05:40:15PM +0200, Takashi Iwai wrote:
> At Tue, 17 Jul 2007 17:32:36 +0200,
> Sam Ravnborg wrote:
> >
> > On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
> > > At Tue, 17 Jul 2007 17:14:32 +0200,
> > > Sam Ravnborg wrote:
> > > >
> > > > On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
> > > > > At Tue, 17 Jul 2007 15:02:30 +0200,
> > > > > Sam Ravnborg wrote:
> > > > > >
> > > > > > On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
> > > > > > > Introduce __init_exit, which is useful ie. for drivers that call
> > > > > > > cleanup functions when they fail in __init functions.
> > > > > >
> > > > > > This is wrong.
> > > > > > On arm (just one example of several) the __exit section are
> > > > > > discarded
> > > > > > at buildtime so any reference from __init to __exit will cause the
> > > > > > linker to error out.
> > > > >
> > > > > Hmm, from what I see, it adds __init to the function. There is no
> > > > > reference to __exit.
> > > >
> > > > The cleanup functions are marked __exit in the referenced case.
> > >
> > > My understanding is that it's the very purpose of this patch --
> > > change the mark from __exit to __init_exit for such clean-up
> > > functions.
> >
> > And that is wrong.
>
> You misunderstood. What I meant is the case like this:
>
> static void __init_exit cleanup()
> {
> ...
> }
>
> static void __init foo_init()
> {
> if (error)
> cleanup();
> }
>
> static void __exit foo_exit()
> {
> cleanup();
> }
>
> Currently, there is no proper way to mark cleanup(). Neither __init,
> __exit, __devinit nor __devexit can be used there.
Then you get the annotation sorted out so cleanup() get discarded in the
built-in case. But you leave no room for automated tools to detect this.
If this is really necessary (and I daught) then a specific section should be
dedicated for this usage.
We have lot of issues with current __init/__exit, __devinit/__devexit,
__cpuint/__cpuexit
and introducing more of the kind does not help it.
So even if it saves a few bytes in some odd cases the added complaxity is IMHO
not worth it.
Sam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
At Tue, 17 Jul 2007 17:32:36 +0200,
Sam Ravnborg wrote:
>
> On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
> > At Tue, 17 Jul 2007 17:14:32 +0200,
> > Sam Ravnborg wrote:
> > >
> > > On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
> > > > At Tue, 17 Jul 2007 15:02:30 +0200,
> > > > Sam Ravnborg wrote:
> > > > >
> > > > > On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
> > > > > > Introduce __init_exit, which is useful ie. for drivers that call
> > > > > > cleanup functions when they fail in __init functions.
> > > > >
> > > > > This is wrong.
> > > > > On arm (just one example of several) the __exit section are discarded
> > > > > at buildtime so any reference from __init to __exit will cause the
> > > > > linker to error out.
> > > >
> > > > Hmm, from what I see, it adds __init to the function. There is no
> > > > reference to __exit.
> > >
> > > The cleanup functions are marked __exit in the referenced case.
> >
> > My understanding is that it's the very purpose of this patch --
> > change the mark from __exit to __init_exit for such clean-up
> > functions.
>
> And that is wrong.
You misunderstood. What I meant is the case like this:
static void __init_exit cleanup()
{
...
}
static void __init foo_init()
{
if (error)
cleanup();
}
static void __exit foo_exit()
{
cleanup();
}
Currently, there is no proper way to mark cleanup(). Neither __init,
__exit, __devinit nor __devexit can be used there.
Takashi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
> At Tue, 17 Jul 2007 17:14:32 +0200,
> Sam Ravnborg wrote:
> >
> > On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
> > > At Tue, 17 Jul 2007 15:02:30 +0200,
> > > Sam Ravnborg wrote:
> > > >
> > > > On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
> > > > > Introduce __init_exit, which is useful ie. for drivers that call
> > > > > cleanup functions when they fail in __init functions.
> > > >
> > > > This is wrong.
> > > > On arm (just one example of several) the __exit section are discarded
> > > > at buildtime so any reference from __init to __exit will cause the
> > > > linker to error out.
> > >
> > > Hmm, from what I see, it adds __init to the function. There is no
> > > reference to __exit.
> >
> > The cleanup functions are marked __exit in the referenced case.
>
> My understanding is that it's the very purpose of this patch --
> change the mark from __exit to __init_exit for such clean-up
> functions.
And that is wrong.
See following example:
static void __init foo_init()
{
if (error)
foo_exit();
}
static void __exit foo_exit()
{
}
If foo_init is annotated with __init_exit then in the build-in case it
become __init and there is a reference to a non existing function because
functions marked __exit are discarded during link or run-time (depending on
arch).
If foo_exit() are marked __init_exit then it becomes __init in the non-module
case
which seems coorrect. If this is the intention of the patch then it should
be OK but then this intention should be spelled out.
Sam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
At Tue, 17 Jul 2007 17:14:32 +0200, Sam Ravnborg wrote: > > On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote: > > At Tue, 17 Jul 2007 15:02:30 +0200, > > Sam Ravnborg wrote: > > > > > > On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: > > > > Introduce __init_exit, which is useful ie. for drivers that call > > > > cleanup functions when they fail in __init functions. > > > > > > This is wrong. > > > On arm (just one example of several) the __exit section are discarded > > > at buildtime so any reference from __init to __exit will cause the > > > linker to error out. > > > > Hmm, from what I see, it adds __init to the function. There is no > > reference to __exit. > > The cleanup functions are marked __exit in the referenced case. My understanding is that it's the very purpose of this patch -- change the mark from __exit to __init_exit for such clean-up functions. Takashi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote: > At Tue, 17 Jul 2007 15:02:30 +0200, > Sam Ravnborg wrote: > > > > On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: > > > Introduce __init_exit, which is useful ie. for drivers that call > > > cleanup functions when they fail in __init functions. > > > > This is wrong. > > On arm (just one example of several) the __exit section are discarded > > at buildtime so any reference from __init to __exit will cause the > > linker to error out. > > Hmm, from what I see, it adds __init to the function. There is no > reference to __exit. The cleanup functions are marked __exit in the referenced case. Sam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
At Tue, 17 Jul 2007 15:02:30 +0200, Sam Ravnborg wrote: > > On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: > > Introduce __init_exit, which is useful ie. for drivers that call > > cleanup functions when they fail in __init functions. > > This is wrong. > On arm (just one example of several) the __exit section are discarded > at buildtime so any reference from __init to __exit will cause the > linker to error out. Hmm, from what I see, it adds __init to the function. There is no reference to __exit. Takashi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: > Introduce __init_exit, which is useful ie. for drivers that call > cleanup functions when they fail in __init functions. This is wrong. On arm (just one example of several) the __exit section are discarded at buildtime so any reference from __init to __exit will cause the linker to error out. The real solution is only to declare functiones used solely during exit as __exit. The whole point of using __exit is to tell that this can be safely dropped when built-in because it is not used then. Sam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On 17/07/07 10:31 +0200, Adrian Bunk wrote:
> On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
> > Introduce __init_exit, which is useful ie. for drivers that call
> > cleanup functions when they fail in __init functions.
> >
> >
> > Signed-off-by: Domen Puncer <[EMAIL PROTECTED]>
> > ---
> > include/linux/init.h |2 ++
> > 1 file changed, 2 insertions(+)
> >
> > Index: work-powerpc.git/include/linux/init.h
> > ===
> > --- work-powerpc.git.orig/include/linux/init.h
> > +++ work-powerpc.git/include/linux/init.h
> > @@ -60,8 +60,10 @@
> >
> > #ifdef MODULE
> > #define __exit __attribute__ ((__section__(".exit.text")))
> > +#define __init_exit
> > #else
> > #define __exit __attribute_used__ __attribute__
> > ((__section__(".exit.text")))
> > +#define __init_exit__init
> > #endif
> >
> > /* For assembly routines */
>
> This doesn't work on architectures like i386 where __exit code is
> discarded at runtime.
If it's a module, then it shouldn't be discarded until unload anyway.
If it's in-kernel, then it'll be discarded as __init stuff.
I don't see a problem?
BTW. can you point me to reasoning for discarding __exit at runtime?
Domen
>
> cu
> Adrian
>
> --
>
>"Is there not promise of rain?" Ling Tan asked suddenly out
> of the darkness. There had been need of rain for many days.
>"Only a promise," Lao Er said.
>Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
> Introduce __init_exit, which is useful ie. for drivers that call
> cleanup functions when they fail in __init functions.
>
>
> Signed-off-by: Domen Puncer <[EMAIL PROTECTED]>
> ---
> include/linux/init.h |2 ++
> 1 file changed, 2 insertions(+)
>
> Index: work-powerpc.git/include/linux/init.h
> ===
> --- work-powerpc.git.orig/include/linux/init.h
> +++ work-powerpc.git/include/linux/init.h
> @@ -60,8 +60,10 @@
>
> #ifdef MODULE
> #define __exit __attribute__ ((__section__(".exit.text")))
> +#define __init_exit
> #else
> #define __exit __attribute_used__ __attribute__
> ((__section__(".exit.text")))
> +#define __init_exit __init
> #endif
>
> /* For assembly routines */
This doesn't work on architectures like i386 where __exit code is
discarded at runtime.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] introduce __init_exit function annotation
Introduce __init_exit, which is useful ie. for drivers that call
cleanup functions when they fail in __init functions.
Signed-off-by: Domen Puncer <[EMAIL PROTECTED]>
---
include/linux/init.h |2 ++
1 file changed, 2 insertions(+)
Index: work-powerpc.git/include/linux/init.h
===
--- work-powerpc.git.orig/include/linux/init.h
+++ work-powerpc.git/include/linux/init.h
@@ -60,8 +60,10 @@
#ifdef MODULE
#define __exit __attribute__ ((__section__(".exit.text")))
+#define __init_exit
#else
#define __exit __attribute_used__ __attribute__
((__section__(".exit.text")))
+#define __init_exit__init
#endif
/* For assembly routines */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] introduce __init_exit function annotation
Introduce __init_exit, which is useful ie. for drivers that call cleanup functions when they fail in __init functions. Signed-off-by: Domen Puncer [EMAIL PROTECTED] --- include/linux/init.h |2 ++ 1 file changed, 2 insertions(+) Index: work-powerpc.git/include/linux/init.h === --- work-powerpc.git.orig/include/linux/init.h +++ work-powerpc.git/include/linux/init.h @@ -60,8 +60,10 @@ #ifdef MODULE #define __exit __attribute__ ((__section__(.exit.text))) +#define __init_exit #else #define __exit __attribute_used__ __attribute__ ((__section__(.exit.text))) +#define __init_exit__init #endif /* For assembly routines */ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: Introduce __init_exit, which is useful ie. for drivers that call cleanup functions when they fail in __init functions. Signed-off-by: Domen Puncer [EMAIL PROTECTED] --- include/linux/init.h |2 ++ 1 file changed, 2 insertions(+) Index: work-powerpc.git/include/linux/init.h === --- work-powerpc.git.orig/include/linux/init.h +++ work-powerpc.git/include/linux/init.h @@ -60,8 +60,10 @@ #ifdef MODULE #define __exit __attribute__ ((__section__(.exit.text))) +#define __init_exit #else #define __exit __attribute_used__ __attribute__ ((__section__(.exit.text))) +#define __init_exit __init #endif /* For assembly routines */ This doesn't work on architectures like i386 where __exit code is discarded at runtime. cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On 17/07/07 10:31 +0200, Adrian Bunk wrote: On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: Introduce __init_exit, which is useful ie. for drivers that call cleanup functions when they fail in __init functions. Signed-off-by: Domen Puncer [EMAIL PROTECTED] --- include/linux/init.h |2 ++ 1 file changed, 2 insertions(+) Index: work-powerpc.git/include/linux/init.h === --- work-powerpc.git.orig/include/linux/init.h +++ work-powerpc.git/include/linux/init.h @@ -60,8 +60,10 @@ #ifdef MODULE #define __exit __attribute__ ((__section__(.exit.text))) +#define __init_exit #else #define __exit __attribute_used__ __attribute__ ((__section__(.exit.text))) +#define __init_exit__init #endif /* For assembly routines */ This doesn't work on architectures like i386 where __exit code is discarded at runtime. If it's a module, then it shouldn't be discarded until unload anyway. If it's in-kernel, then it'll be discarded as __init stuff. I don't see a problem? BTW. can you point me to reasoning for discarding __exit at runtime? Domen cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: Introduce __init_exit, which is useful ie. for drivers that call cleanup functions when they fail in __init functions. This is wrong. On arm (just one example of several) the __exit section are discarded at buildtime so any reference from __init to __exit will cause the linker to error out. The real solution is only to declare functiones used solely during exit as __exit. The whole point of using __exit is to tell that this can be safely dropped when built-in because it is not used then. Sam - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
At Tue, 17 Jul 2007 15:02:30 +0200, Sam Ravnborg wrote: On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: Introduce __init_exit, which is useful ie. for drivers that call cleanup functions when they fail in __init functions. This is wrong. On arm (just one example of several) the __exit section are discarded at buildtime so any reference from __init to __exit will cause the linker to error out. Hmm, from what I see, it adds __init to the function. There is no reference to __exit. Takashi - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote: At Tue, 17 Jul 2007 15:02:30 +0200, Sam Ravnborg wrote: On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: Introduce __init_exit, which is useful ie. for drivers that call cleanup functions when they fail in __init functions. This is wrong. On arm (just one example of several) the __exit section are discarded at buildtime so any reference from __init to __exit will cause the linker to error out. Hmm, from what I see, it adds __init to the function. There is no reference to __exit. The cleanup functions are marked __exit in the referenced case. Sam - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
At Tue, 17 Jul 2007 17:14:32 +0200, Sam Ravnborg wrote: On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote: At Tue, 17 Jul 2007 15:02:30 +0200, Sam Ravnborg wrote: On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote: Introduce __init_exit, which is useful ie. for drivers that call cleanup functions when they fail in __init functions. This is wrong. On arm (just one example of several) the __exit section are discarded at buildtime so any reference from __init to __exit will cause the linker to error out. Hmm, from what I see, it adds __init to the function. There is no reference to __exit. The cleanup functions are marked __exit in the referenced case. My understanding is that it's the very purpose of this patch -- change the mark from __exit to __init_exit for such clean-up functions. Takashi - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:14:32 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 15:02:30 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
Introduce __init_exit, which is useful ie. for drivers that call
cleanup functions when they fail in __init functions.
This is wrong.
On arm (just one example of several) the __exit section are discarded
at buildtime so any reference from __init to __exit will cause the
linker to error out.
Hmm, from what I see, it adds __init to the function. There is no
reference to __exit.
The cleanup functions are marked __exit in the referenced case.
My understanding is that it's the very purpose of this patch --
change the mark from __exit to __init_exit for such clean-up
functions.
And that is wrong.
See following example:
static void __init foo_init()
{
if (error)
foo_exit();
}
static void __exit foo_exit()
{
}
If foo_init is annotated with __init_exit then in the build-in case it
become __init and there is a reference to a non existing function because
functions marked __exit are discarded during link or run-time (depending on
arch).
If foo_exit() are marked __init_exit then it becomes __init in the non-module
case
which seems coorrect. If this is the intention of the patch then it should
be OK but then this intention should be spelled out.
Sam
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
At Tue, 17 Jul 2007 17:32:36 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:14:32 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 15:02:30 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
Introduce __init_exit, which is useful ie. for drivers that call
cleanup functions when they fail in __init functions.
This is wrong.
On arm (just one example of several) the __exit section are discarded
at buildtime so any reference from __init to __exit will cause the
linker to error out.
Hmm, from what I see, it adds __init to the function. There is no
reference to __exit.
The cleanup functions are marked __exit in the referenced case.
My understanding is that it's the very purpose of this patch --
change the mark from __exit to __init_exit for such clean-up
functions.
And that is wrong.
You misunderstood. What I meant is the case like this:
static void __init_exit cleanup()
{
...
}
static void __init foo_init()
{
if (error)
cleanup();
}
static void __exit foo_exit()
{
cleanup();
}
Currently, there is no proper way to mark cleanup(). Neither __init,
__exit, __devinit nor __devexit can be used there.
Takashi
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On Tue, Jul 17, 2007 at 05:40:15PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:32:36 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:14:32 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 15:02:30 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
Introduce __init_exit, which is useful ie. for drivers that call
cleanup functions when they fail in __init functions.
This is wrong.
On arm (just one example of several) the __exit section are
discarded
at buildtime so any reference from __init to __exit will cause the
linker to error out.
Hmm, from what I see, it adds __init to the function. There is no
reference to __exit.
The cleanup functions are marked __exit in the referenced case.
My understanding is that it's the very purpose of this patch --
change the mark from __exit to __init_exit for such clean-up
functions.
And that is wrong.
You misunderstood. What I meant is the case like this:
static void __init_exit cleanup()
{
...
}
static void __init foo_init()
{
if (error)
cleanup();
}
static void __exit foo_exit()
{
cleanup();
}
Currently, there is no proper way to mark cleanup(). Neither __init,
__exit, __devinit nor __devexit can be used there.
Then you get the annotation sorted out so cleanup() get discarded in the
built-in case. But you leave no room for automated tools to detect this.
If this is really necessary (and I daught) then a specific section should be
dedicated for this usage.
We have lot of issues with current __init/__exit, __devinit/__devexit,
__cpuint/__cpuexit
and introducing more of the kind does not help it.
So even if it saves a few bytes in some odd cases the added complaxity is IMHO
not worth it.
Sam
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
At Tue, 17 Jul 2007 18:48:46 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 05:40:15PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:32:36 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:14:32 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 15:02:30 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
Introduce __init_exit, which is useful ie. for drivers that call
cleanup functions when they fail in __init functions.
This is wrong.
On arm (just one example of several) the __exit section are
discarded
at buildtime so any reference from __init to __exit will cause the
linker to error out.
Hmm, from what I see, it adds __init to the function. There is no
reference to __exit.
The cleanup functions are marked __exit in the referenced case.
My understanding is that it's the very purpose of this patch --
change the mark from __exit to __init_exit for such clean-up
functions.
And that is wrong.
You misunderstood. What I meant is the case like this:
static void __init_exit cleanup()
{
...
}
static void __init foo_init()
{
if (error)
cleanup();
}
static void __exit foo_exit()
{
cleanup();
}
Currently, there is no proper way to mark cleanup(). Neither __init,
__exit, __devinit nor __devexit can be used there.
Then you get the annotation sorted out so cleanup() get discarded in the
built-in case. But you leave no room for automated tools to detect this.
If this is really necessary (and I daught) then a specific section should be
dedicated for this usage.
We have lot of issues with current __init/__exit, __devinit/__devexit,
__cpuint/__cpuexit
and introducing more of the kind does not help it.
So even if it saves a few bytes in some odd cases the added complaxity is
IMHO not worth it.
Well, I don't think it's a few bytes and not so odd, but I agree that
this solution isn't the best way. And, I now remember that this won't
work anyway, too. Calling __init from __exit also causes error...
BTW, this reminds me why we have to add annotations for each
subisdiary function manually. A tool to parse the code statically and
give the proper annotations/hints would be really nice.
Takashi
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On 17/07/07 17:40 +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:32:36 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:14:32 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 15:02:30 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
Introduce __init_exit, which is useful ie. for drivers that call
cleanup functions when they fail in __init functions.
This is wrong.
On arm (just one example of several) the __exit section are
discarded
at buildtime so any reference from __init to __exit will cause the
linker to error out.
Hmm, from what I see, it adds __init to the function. There is no
reference to __exit.
The cleanup functions are marked __exit in the referenced case.
My understanding is that it's the very purpose of this patch --
change the mark from __exit to __init_exit for such clean-up
functions.
And that is wrong.
You misunderstood. What I meant is the case like this:
static void __init_exit cleanup()
{
...
}
static void __init foo_init()
{
if (error)
cleanup();
}
static void __exit foo_exit()
{
cleanup();
}
Uh, yes, this, or just __init_exit foo_exit() as in Sam's example.
It seemed obvious to me, sorry.
Currently, there is no proper way to mark cleanup(). Neither __init,
__exit, __devinit nor __devexit can be used there.
Takashi
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] introduce __init_exit function annotation
On 17/07/07 19:02 +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 18:48:46 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 05:40:15PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:32:36 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 05:16:13PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 17:14:32 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 04:52:12PM +0200, Takashi Iwai wrote:
At Tue, 17 Jul 2007 15:02:30 +0200,
Sam Ravnborg wrote:
On Tue, Jul 17, 2007 at 10:02:48AM +0200, Domen Puncer wrote:
Introduce __init_exit, which is useful ie. for drivers that
call
cleanup functions when they fail in __init functions.
This is wrong.
On arm (just one example of several) the __exit section are
discarded
at buildtime so any reference from __init to __exit will cause
the
linker to error out.
Hmm, from what I see, it adds __init to the function. There is no
reference to __exit.
The cleanup functions are marked __exit in the referenced case.
My understanding is that it's the very purpose of this patch --
change the mark from __exit to __init_exit for such clean-up
functions.
And that is wrong.
You misunderstood. What I meant is the case like this:
static void __init_exit cleanup()
{
...
}
static void __init foo_init()
{
if (error)
cleanup();
}
static void __exit foo_exit()
{
cleanup();
}
Currently, there is no proper way to mark cleanup(). Neither __init,
__exit, __devinit nor __devexit can be used there.
Then you get the annotation sorted out so cleanup() get discarded in the
built-in case. But you leave no room for automated tools to detect this.
If this is really necessary (and I daught) then a specific section should be
dedicated for this usage.
We have lot of issues with current __init/__exit, __devinit/__devexit,
__cpuint/__cpuexit
and introducing more of the kind does not help it.
So even if it saves a few bytes in some odd cases the added complaxity is
IMHO not worth it.
Well, I don't think it's a few bytes and not so odd, but I agree that
this solution isn't the best way. And, I now remember that this won't
work anyway, too. Calling __init from __exit also causes error...
I made this patch because I saw __init calling __exit in yet another
driver (gianfar). Guess I'll just send the old way fix, and remove __exit.
As for calling __init_exit from __exit:
1 - in kernel, there's no __exit = no problem
2 - module, __init_exit is a no-op = no problem
the code in question again:
#ifdef MODULE
#define __exit __attribute__ ((__section__(.exit.text)))
+#define __init_exit
#else
#define __exit __attribute_used__ __attribute__
((__section__(.exit.text)))
+#define __init_exit __init
#endif
Or maybe it's the name that is confuzing, but it makes sense to me:
__init_exit - you can call it from __init or __exit.
__init_or_exit?
Domen
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
