lkml.org 
[lkml]   [2008]   [Jun]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] add a printk_init variant storing format strings in __initdata
    It is important about embedded system, too.
    So I add CC, linux-embedded.

    On Wed, Jun 4, 2008 at 5:16 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
    > On Tue, 3 Jun 2008 10:27:32 +0100 Andy Whitcroft <apw@shadowen.org> wrote:
    >
    >>
    >> [As gcc seems unable to help us out selecting the appropriate data segment
    >> for the code, how about we did something like this?]
    >>
    >> When using printk from __init functions it would be desirable to place
    >> the printk format strings in __initdata. Add a printk_init() variant
    >> which does this.
    >>
    >> This printk_init() is necessarily a #define so that we can declare the
    >> format string in static scope and mark it __initdata. We then call a
    >> newly introduced __printk_init() variant which is identicle to printk() but
    >> marked __init itself. By ensuring that an __init variant of printk is used
    >> we get proper section violation warnings when this is used incorrectly:
    >>
    >> WARNING: vmlinux.o(.text+0x3): Section mismatch in reference from the
    >> function something() to the variable .init.data:__printk_init_fmt.31426
    >> The function something() references
    >> the variable __initdata __printk_init_fmt.31426.
    >> This is often because something lacks a __initdata
    >> annotation or the annotation of __printk_init_fmt.31426 is wrong.
    >>
    >> Note I have followed printk's pattern for __cold annotations.
    >>
    >
    > Ho hum. This give everyone another way in which to bury everyone else
    > with patches.
    >
    > Wouldn't it be great if checkpatch were to detect
    > fail-to-use-printk_init() in an __init function?
    >
    > oh, speaking of checkpatch: please use it :)
    >
    >> ---
    >> include/linux/kernel.h | 10 ++++++++++
    >> kernel/printk.c | 12 ++++++++++++
    >> 2 files changed, 22 insertions(+), 0 deletions(-)
    >> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    >> index 792bf0a..7754196 100644
    >> --- a/include/linux/kernel.h
    >> +++ b/include/linux/kernel.h
    >> @@ -180,6 +180,13 @@ struct pid;
    >> extern struct pid *session_of_pgrp(struct pid *pgrp);
    >>
    >> #ifdef CONFIG_PRINTK
    >> +#define printk_init(fmt, args...) \
    >> +do { \
    >> + static char __printk_init_fmt[] __initdata = fmt; \
    >> + __printk_init(__printk_init_fmt, ##args); \
    >> +} while (0)
    >> +asmlinkage int __printk_init(const char * fmt, ...)
    >> + __attribute__ ((format (printf, 1, 2))) __cold;
    >> asmlinkage int vprintk(const char *fmt, va_list args)
    >> __attribute__ ((format (printf, 1, 0)));
    >> asmlinkage int printk(const char * fmt, ...)
    >> @@ -196,6 +203,9 @@ extern int __printk_ratelimit(int ratelimit_jiffies, int ratelimit_burst);
    >> extern bool printk_timed_ratelimit(unsigned long *caller_jiffies,
    >> unsigned int interval_msec);
    >> #else
    >> +asmlinkage int printk_init(const char * fmt, ...)
    >> + __attribute__ ((format (printf, 1, 2))) __cold;
    >> +static inline int __cold printk_init(const char *s, ...) { return 0; }
    >> static inline int vprintk(const char *s, va_list args)
    >> __attribute__ ((format (printf, 1, 0)));
    >> static inline int vprintk(const char *s, va_list args) { return 0; }
    >> diff --git a/kernel/printk.c b/kernel/printk.c
    >> index 8fb01c3..992a5c0 100644
    >> --- a/kernel/printk.c
    >> +++ b/kernel/printk.c
    >> @@ -616,6 +616,18 @@ asmlinkage int printk(const char *fmt, ...)
    >> return r;
    >> }
    >>
    >> +asmlinkage __init int __printk_init(const char *fmt, ...)
    >> +{
    >> + va_list args;
    >> + int r;
    >> +
    >> + va_start(args, fmt);
    >> + r = vprintk(fmt, args);
    >> + va_end(args);
    >> +
    >> + return r;
    >> +}
    >
    > We're going to want to be able to call printk_init() from modules.
    > Please fix and test that, if we decide to proceed.
    >
    > Oh, and we're going to need printk_meminit() and printk_cpuinit() and
    > whatever.
    >
    > Which probably means that __printk_init() can't be __init, unless all
    > the CONFIG_ settings which control __cpuinit, __meminit etc are blowing
    > in the right direction.
    >
    > It would be good if we could get some idea of the savings here, because
    > boy this is going to be a pain.
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at http://www.tux.org/lkml/
    >



    --
    Kinds regards,
    MinChan Kim


    \
     
     \ /
      Last update: 2008-06-04 10:35    [W:0.029 / U:180.040 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site