* [PATCH] kernel: power: swap: mark a function as __init to save some memory @ 2020-05-31 21:00 Christophe JAILLET 2020-05-31 22:11 ` Joe Perches 2020-06-05 11:56 ` Rafael J. Wysocki 0 siblings, 2 replies; 6+ messages in thread From: Christophe JAILLET @ 2020-05-31 21:00 UTC (permalink / raw) To: rjw, pavel, len.brown Cc: linux-pm, linux-kernel, kernel-janitors, Christophe JAILLET '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) -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel: power: swap: mark a function as __init to save some memory 2020-05-31 21:00 [PATCH] kernel: power: swap: mark a function as __init to save some memory Christophe JAILLET @ 2020-05-31 22:11 ` Joe Perches 2020-06-01 17:17 ` Christophe JAILLET 2020-06-08 11:22 ` Dan Carpenter 2020-06-05 11:56 ` Rafael J. Wysocki 1 sibling, 2 replies; 6+ messages in thread From: Joe Perches @ 2020-05-31 22:11 UTC (permalink / raw) To: Christophe JAILLET, rjw, pavel, len.brown, Dan Carpenter Cc: linux-pm, linux-kernel, kernel-janitors (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) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel: power: swap: mark a function as __init to save some memory 2020-05-31 22:11 ` Joe Perches @ 2020-06-01 17:17 ` Christophe JAILLET 2020-06-08 11:22 ` Dan Carpenter 1 sibling, 0 replies; 6+ messages in thread From: Christophe JAILLET @ 2020-06-01 17:17 UTC (permalink / raw) To: Joe Perches, rjw, pavel, len.brown, Dan Carpenter Cc: linux-pm, linux-kernel, kernel-janitors 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 ________________________________________________ #!/usr/bin/perl use warnings; use strict; # grep -r --include=*.c -E '^[[:space:]]*(early|core|postcore|arch|subsys|fs|device|late)_initcall\(.*\)' * > tmp.txt my $tmp="tmp.txt"; open(my $fh, "<", $tmp); while (my $line = <$fh>) { # Each line looks like: # net/mac80211/main.c:subsys_initcall(ieee80211_init); if ($line =~ /^(.*):.*\((.*)\)/) { system("grep -E '$2\\(void' $1 | grep -v -E '__.*init'"); } } close($fh) >> --- >> 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) > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel: power: swap: mark a function as __init to save some memory 2020-05-31 22:11 ` Joe Perches 2020-06-01 17:17 ` Christophe JAILLET @ 2020-06-08 11:22 ` Dan Carpenter 2020-06-08 11:34 ` Julia Lawall 1 sibling, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2020-06-08 11:22 UTC (permalink / raw) To: Joe Perches Cc: Christophe JAILLET, rjw, pavel, len.brown, Dan Carpenter, linux-pm, linux-kernel, kernel-janitors 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel: power: swap: mark a function as __init to save some memory 2020-06-08 11:22 ` Dan Carpenter @ 2020-06-08 11:34 ` Julia Lawall 0 siblings, 0 replies; 6+ messages in thread From: Julia Lawall @ 2020-06-08 11:34 UTC (permalink / raw) To: Dan Carpenter Cc: Joe Perches, Christophe JAILLET, rjw, pavel, len.brown, Dan Carpenter, linux-pm, linux-kernel, kernel-janitors [-- Attachment #1: Type: text/plain, Size: 1101 bytes --] 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 [-- Attachment #2: Type: text/plain, Size: 2715 bytes --] // 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 (...) { ... } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel: power: swap: mark a function as __init to save some memory 2020-05-31 21:00 [PATCH] kernel: power: swap: mark a function as __init to save some memory Christophe JAILLET 2020-05-31 22:11 ` Joe Perches @ 2020-06-05 11:56 ` Rafael J. Wysocki 1 sibling, 0 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2020-06-05 11:56 UTC (permalink / raw) To: Christophe JAILLET Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Linux PM, Linux Kernel Mailing List, kernel-janitors 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! ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-08 11:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-31 21:00 [PATCH] kernel: power: swap: mark a function as __init to save some memory Christophe JAILLET 2020-05-31 22:11 ` Joe Perches 2020-06-01 17:17 ` Christophe JAILLET 2020-06-08 11:22 ` Dan Carpenter 2020-06-08 11:34 ` Julia Lawall 2020-06-05 11:56 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).