| Message ID | 20200531210059.647066-1-christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | kernel: power: swap: mark a function as __init to save some memory | expand |
(adding Dan Carpenter) On Sun, 2020-05-31 at 23:00 +0200, Christophe JAILLET wrote: > 'swsusp_header_init()' is only called via 'core_initcall'. > It can be marked as __init to save a few bytes of memory. Hey Dan smatch has a full function calling tree right? Can smatch find unmarked functions called only by __init functions so those unmarked functions can be appropriately marked with __init like the below? > --- > kernel/power/swap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/power/swap.c b/kernel/power/swap.c > index ca0fcb5ced71..01e2858b5fe3 100644 > --- a/kernel/power/swap.c > +++ b/kernel/power/swap.c > @@ -1590,7 +1590,7 @@ int swsusp_unmark(void) > } > #endif > > -static int swsusp_header_init(void) > +static int __init swsusp_header_init(void) > { > swsusp_header = (struct swsusp_header*) __get_free_page(GFP_KERNEL); > if (!swsusp_header)
Le 01/06/2020 à 00:11, Joe Perches a écrit : > (adding Dan Carpenter) > > On Sun, 2020-05-31 at 23:00 +0200, Christophe JAILLET wrote: >> 'swsusp_header_init()' is only called via 'core_initcall'. >> It can be marked as __init to save a few bytes of memory. > > Hey Dan > > smatch has a full function calling tree right? > > Can smatch find unmarked functions called only by __init > functions so those unmarked functions can be appropriately > marked with __init like the below? > Hi, in case of interest for anyone, I actually find such things as follow: - grep to spot xxx_initcall macro (see comments in the perl script below) - a perl script which tries to spot missing __init The false positive rate is low. Feel free to use and propose patches based on it. CJ
On Sun, May 31, 2020 at 11:01 PM Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > 'swsusp_header_init()' is only called via 'core_initcall'. > It can be marked as __init to save a few bytes of memory. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > kernel/power/swap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/power/swap.c b/kernel/power/swap.c > index ca0fcb5ced71..01e2858b5fe3 100644 > --- a/kernel/power/swap.c > +++ b/kernel/power/swap.c > @@ -1590,7 +1590,7 @@ int swsusp_unmark(void) > } > #endif > > -static int swsusp_header_init(void) > +static int __init swsusp_header_init(void) > { > swsusp_header = (struct swsusp_header*) __get_free_page(GFP_KERNEL); > if (!swsusp_header) > -- Applied as 5.8-rc material under the "PM: hibernate: Add __init annotation to swsusp_header_init()" subject, thanks!
On Sun, May 31, 2020 at 03:11:27PM -0700, Joe Perches wrote: > (adding Dan Carpenter) > > On Sun, 2020-05-31 at 23:00 +0200, Christophe JAILLET wrote: > > 'swsusp_header_init()' is only called via 'core_initcall'. > > It can be marked as __init to save a few bytes of memory. > > Hey Dan > > smatch has a full function calling tree right? > > Can smatch find unmarked functions called only by __init > functions so those unmarked functions can be appropriately > marked with __init like the below? > It turns out it's complicated to do this in Smatch because Sparse ignores the section attribute. :/ regards, dan carpenter
On Mon, 8 Jun 2020, Dan Carpenter wrote: > On Sun, May 31, 2020 at 03:11:27PM -0700, Joe Perches wrote: > > (adding Dan Carpenter) > > > > On Sun, 2020-05-31 at 23:00 +0200, Christophe JAILLET wrote: > > > 'swsusp_header_init()' is only called via 'core_initcall'. > > > It can be marked as __init to save a few bytes of memory. > > > > Hey Dan > > > > smatch has a full function calling tree right? > > > > Can smatch find unmarked functions called only by __init > > functions so those unmarked functions can be appropriately > > marked with __init like the below? > > > > It turns out it's complicated to do this in Smatch because Sparse > ignores the section attribute. :/ I wrote a script at one point for this for Coccinelle, and sent some patches. It requires some effort, because you want to run it over and over - once function Y becomes init, some other functions might become init as well. The iteration could be done automatically with Coccinelle, but I didn't take that option, because it semed safer to check the results along the way. A version of the script is attached. julia // No iteration. Do it by hand. @initialize:ocaml@ @@ let itbl = Hashtbl.create 101 let ltbl = Hashtbl.create 101 let thefile = ref "" let hashadd t k = let cell = try Hashtbl.find t k with Not_found -> let cell = ref 0 in Hashtbl.add t k cell; cell in cell := !cell + 1 let hashget t k = try !(Hashtbl.find t k) with Not_found -> 0 let seen = ref [] @script:ocaml@ @@ (let file = List.hd (Coccilib.files()) in thefile := file; let file = try List.hd(List.tl (Str.split (Str.regexp "/linux-next/") file)) with _ -> file in let ofile = "/var/julia/linux-next/" ^ (Filename.chop_extension file) ^ ".o" in if not(Sys.file_exists ofile) then Coccilib.exit()); Hashtbl.clear itbl; Hashtbl.clear ltbl; seen := [] @r@ identifier f; @@ __init f(...) { ... } @script:ocaml@ f << r.f; @@ Hashtbl.add itbl f () @s disable optional_attributes@ identifier f; @@ static f(...) { ... } @script:ocaml@ f << s.f; @@ Hashtbl.add ltbl f () @t exists@ identifier f,g; position p; @@ __init f(...) { ... when any g@p(...) ... when any } @script:ocaml@ g << t.g; _p << t.p; @@ if not (Hashtbl.mem ltbl g) || Hashtbl.mem itbl g then Coccilib.include_match false @ok1 disable optional_attributes exists@ identifier f,t.g; @@ f(...) { ... when any g ... when any } @ok2 disable optional_attributes exists@ identifier i,j,fld,t.g; @@ struct i j = { .fld = g, }; @ok3 disable optional_attributes exists@ identifier t.g; declarer d; @@ d(...,g,...); @ok4 disable optional_attributes exists@ identifier t.g; expression e; @@ ( e(...,g,...) | e(...,&g,...) | e = &g | e = g ) @script:ocaml depends on !ok1 && !ok2 && !ok3 && !ok4@ g << t.g; @@ let file = !thefile in let file = try List.hd(List.tl (Str.split (Str.regexp "/linux-next/") file)) with _ -> file in if not(List.mem (g,file) !seen) then begin seen := (g,file) :: !seen; let ofile = "/var/julia/linux-next/" ^ (Filename.chop_extension file) ^ ".o" in if Sys.file_exists ofile then let l = Common.cmd_to_list (Printf.sprintf "objdump -x %s | grep -w %s | grep -w F | grep .text.unlikely" ofile g) in match l with [] -> Coccilib.include_match false | _ -> Printf.printf "Info for %s %s\n" file g; List.iter (function l -> Printf.printf "%s\n" l) l; Printf.printf "\n"; flush stdout else Coccilib.include_match false end else Coccilib.include_match false @depends on !ok1 && !ok2 && !ok3 && !ok4@ identifier t.g; @@ - g +__init g (...) { ... }
diff --git a/kernel/power/swap.c b/kernel/power/swap.c index ca0fcb5ced71..01e2858b5fe3 100644 --- a/kernel/power/swap.c +++ b/kernel/power/swap.c @@ -1590,7 +1590,7 @@ int swsusp_unmark(void) } #endif -static int swsusp_header_init(void) +static int __init swsusp_header_init(void) { swsusp_header = (struct swsusp_header*) __get_free_page(GFP_KERNEL); if (!swsusp_header)
'swsusp_header_init()' is only called via 'core_initcall'. It can be marked as __init to save a few bytes of memory. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- kernel/power/swap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)