* [patch]fastboot: remove duplicate unpack_to_rootfs() @ 2008-08-13 6:07 Shaohua Li 2008-08-13 7:45 ` Ingo Molnar 0 siblings, 1 reply; 10+ messages in thread From: Shaohua Li @ 2008-08-13 6:07 UTC (permalink / raw) To: lkml; +Cc: Andrew Morton, Ingo Molnar, Arjan van de Ven we check if initrd is initramfs first and then do real unpack. The check isn't required, we can directly do unpack. If initrd isn't initramfs, we can remove garbage. In my laptop, this saves 0.1s boot time. This penalizes non-initramfs case, but now initramfs is mostly widely used. Signed-off-by: Shaohua Li <shaohua.li@intel.com> diff --git a/init/initramfs.c b/init/initramfs.c index 644fc01..e51c92b 100644 --- a/init/initramfs.c +++ b/init/initramfs.c @@ -5,6 +5,7 @@ #include <linux/fcntl.h> #include <linux/delay.h> #include <linux/string.h> +#include <linux/dirent.h> #include <linux/syscalls.h> static __initdata char *message; @@ -520,6 +521,37 @@ skip: initrd_end = 0; } +static void __init clean_rootfs(void) +{ + int fd = sys_open("/", O_RDONLY, 0); + void *buf = malloc(1024); + struct linux_dirent64 *dirp = buf; + int count; + + memset(buf, 0, PAGE_SIZE); + count = sys_getdents64(fd, dirp, PAGE_SIZE); + while (count > 0) { + while (count > 0) { + struct stat st; + + sys_newlstat(dirp->d_name, &st); + if (S_ISDIR(st.st_mode)) + sys_rmdir(dirp->d_name); + else + sys_unlink(dirp->d_name); + + count -= dirp->d_reclen; + dirp = (void *)dirp + dirp->d_reclen; + } + dirp = buf; + memset(buf, 0, 1024); + count = sys_getdents64(fd, dirp, PAGE_SIZE); + } + + sys_close(fd); + free(buf); +} + static int __init populate_rootfs(void) { char *err = unpack_to_rootfs(__initramfs_start, @@ -531,13 +563,15 @@ static int __init populate_rootfs(void) int fd; printk(KERN_INFO "checking if image is initramfs..."); err = unpack_to_rootfs((char *)initrd_start, - initrd_end - initrd_start, 1); + initrd_end - initrd_start, 0); if (!err) { printk(" it is\n"); - unpack_to_rootfs((char *)initrd_start, - initrd_end - initrd_start, 0); free_initrd(); return 0; + } else { + clean_rootfs(); + unpack_to_rootfs(__initramfs_start, + __initramfs_end - __initramfs_start, 0); } printk("it isn't (%s); looks like an initrd\n", err); fd = sys_open("/initrd.image", O_WRONLY|O_CREAT, 0700); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]fastboot: remove duplicate unpack_to_rootfs() 2008-08-13 6:07 [patch]fastboot: remove duplicate unpack_to_rootfs() Shaohua Li @ 2008-08-13 7:45 ` Ingo Molnar 2008-08-13 7:52 ` Ingo Molnar ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Ingo Molnar @ 2008-08-13 7:45 UTC (permalink / raw) To: Shaohua Li; +Cc: lkml, Andrew Morton, Arjan van de Ven * Shaohua Li <shaohua.li@intel.com> wrote: > we check if initrd is initramfs first and then do real unpack. The > check isn't required, we can directly do unpack. If initrd isn't > initramfs, we can remove garbage. In my laptop, this saves 0.1s boot > time. This penalizes non-initramfs case, but now initramfs is mostly > widely used. clever concept! a few observations about the cleanup function: > +static void __init clean_rootfs(void) > +{ > + int fd = sys_open("/", O_RDONLY, 0); can this ever fail? > + void *buf = malloc(1024); no error checking for buf==NULL. > + struct linux_dirent64 *dirp = buf; > + int count; > + > + memset(buf, 0, PAGE_SIZE); overflow: clearly allocating a 1024 bytes buffer and then clearing 4096 bytes isnt that good? you could introduce a default-off CONFIG_DEBUG_ROOTFS_CLEANUP option that does two runs of unpack_to_rootfs() and inserts an artificial clean_rootfs() inbetween? Even if that debug patch doesnt get integrated its a good test for the cleanup function. > + count = sys_getdents64(fd, dirp, PAGE_SIZE); ... and then doing an up to 4096 bytes getdents into the buffer. A large enough initramfs will overflow this. > + while (count > 0) { > + while (count > 0) { > + struct stat st; > + > + sys_newlstat(dirp->d_name, &st); can this ever fail? If yes we should at least WARN_ON_ONCE(). > + if (S_ISDIR(st.st_mode)) > + sys_rmdir(dirp->d_name); > + else > + sys_unlink(dirp->d_name); > + > + count -= dirp->d_reclen; can this ever zero-underflow, with a sufficiently corrupted initramfs? We should check for 0 underflow to be sure. > + dirp = (void *)dirp + dirp->d_reclen; likewise, we should size-overflow check this pointer. Failure modes of overrunning the buffer are subtle and hard to notice/track down. > + } > + dirp = buf; > + memset(buf, 0, 1024); > + count = sys_getdents64(fd, dirp, PAGE_SIZE); overflow: we do a 4096 bytes getdents into a 1K buffer. > + } > + > + sys_close(fd); > + free(buf); > +} > + > static int __init populate_rootfs(void) > { > char *err = unpack_to_rootfs(__initramfs_start, > @@ -531,13 +563,15 @@ static int __init populate_rootfs(void) > int fd; > printk(KERN_INFO "checking if image is initramfs..."); > err = unpack_to_rootfs((char *)initrd_start, > - initrd_end - initrd_start, 1); > + initrd_end - initrd_start, 0); > if (!err) { > printk(" it is\n"); > - unpack_to_rootfs((char *)initrd_start, > - initrd_end - initrd_start, 0); > free_initrd(); > return 0; > + } else { > + clean_rootfs(); > + unpack_to_rootfs(__initramfs_start, > + __initramfs_end - __initramfs_start, 0); > } the dry_run variable is now unused in unpack_to_rootfs() and could be eliminated. > printk("it isn't (%s); looks like an initrd\n", err); > fd = sys_open("/initrd.image", O_WRONLY|O_CREAT, 0700); Ingo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]fastboot: remove duplicate unpack_to_rootfs() 2008-08-13 7:45 ` Ingo Molnar @ 2008-08-13 7:52 ` Ingo Molnar 2008-08-13 8:06 ` Andrew Morton 2008-08-13 8:11 ` Frans Meulenbroeks 2008-08-13 8:00 ` Li, Shaohua 2008-08-13 9:26 ` Li, Shaohua 2 siblings, 2 replies; 10+ messages in thread From: Ingo Molnar @ 2008-08-13 7:52 UTC (permalink / raw) To: Shaohua Li; +Cc: lkml, Andrew Morton, Arjan van de Ven * Ingo Molnar <mingo@elte.hu> wrote: > the dry_run variable is now unused in unpack_to_rootfs() and could be > eliminated. also, while we are materially touching init/initramfs.c, that file has collected a few uglies in the past few years, checkpatch --file says: total: 7 errors, 7 warnings, 3 checks, 562 lines checked it has a few other problems as well that can be seen if you look at the file. Unused macros: /* Diagnostic functions (stubbed out) */ #define Assert(cond,msg) #define Trace(x) #define Tracev(x) #define Tracevv(x) #define Tracec(c,x) #define Tracecv(c,x) #define STATIC static #define INIT __init lots of no-newline-after-variable-definitions instances: { int written; dry_run = check_only; no-newline-before-return: kfree(header_buf); return message; } so it would be nice to start off with a cleanup [strictly no code changed] patch. then it also has an absolutely crazy turn-error-printouts-off hack: static __initdata char *message; static void __init error(char *x) { if (!message) message = x; } which is unobvious, 100% unused and should be removed. those error()s should be pr_debug() perhaps. [this should be a second cleanup patch as it changes the code] Hm? Ingo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]fastboot: remove duplicate unpack_to_rootfs() 2008-08-13 7:52 ` Ingo Molnar @ 2008-08-13 8:06 ` Andrew Morton 2008-08-13 9:14 ` Ingo Molnar 2008-08-13 8:11 ` Frans Meulenbroeks 1 sibling, 1 reply; 10+ messages in thread From: Andrew Morton @ 2008-08-13 8:06 UTC (permalink / raw) To: Ingo Molnar; +Cc: Shaohua Li, lkml, Arjan van de Ven On Wed, 13 Aug 2008 09:52:36 +0200 Ingo Molnar <mingo@elte.hu> wrote: > no-newline-before-return: > > kfree(header_buf); > return message; > } I accidentally delete those newlines when nobody is looking. They don't seem worth the space they consume. (what do we do with a function which has multiple `return's?) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]fastboot: remove duplicate unpack_to_rootfs() 2008-08-13 8:06 ` Andrew Morton @ 2008-08-13 9:14 ` Ingo Molnar 0 siblings, 0 replies; 10+ messages in thread From: Ingo Molnar @ 2008-08-13 9:14 UTC (permalink / raw) To: Andrew Morton; +Cc: Shaohua Li, lkml, Arjan van de Ven * Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 13 Aug 2008 09:52:36 +0200 Ingo Molnar <mingo@elte.hu> wrote: > > > no-newline-before-return: > > > > kfree(header_buf); > > return message; > > } > > I accidentally delete those newlines when nobody is looking. They > don't seem worth the space they consume. yeah - for me it's case-dependent. My benchmark for it is absolutely objective and easy to describe: i add a newline when it looks nicer and more maintainable that way ;-) > (what do we do with a function which has multiple `return's?) i really didnt want to make a full scale style discussion out of this. Lets ignore my suggestion. The valid case when i use a newline is for example when the return obscures what happens: if (something) { do_one(); repeat_this(); return; } as visually it's easy to miss the return - especially if the lines above it look similar. So i use: if (something) { do_one(); repeat_this(); return; } because way too often do i miss a stray return somewhere and misunderstand the code flow of a function if it does not stand out, even with syntax highlighting. Another case is when there's a long linear block of cleanup statements followed by a return: q->mode = mode; strcpy(q->name, name); q->next = NULL; *p = q; return NULL; } i usually add a newline: q->mode = mode; strcpy(q->name, name); q->next = NULL; *p = q; return NULL; } as the 'return NULL' is a separate concept from the preceding activities. So in this case it is not really because the return is specialy, this is because i like to separate groups of statements per type of activity. So i'd do the same if there were two groups of statements, i'd turn this: q->mode = mode; strcpy(q->name, name); q->next = NULL; *p = q; other_stuff = 2; some_other_stuff(other_stuff) into this: q->mode = mode; strcpy(q->name, name); q->next = NULL; *p = q; other_stuff = 2; some_other_stuff(other_stuff) to make sure the two groups of statements stand out. (Sometimes a pure newline does a better job at inserting the right kind of visual structure than a comment line.) but again ... these are nuances where reasonable people might disagree, and i only made them because this topic lives, is developed and tested in tip/fastboot at the moment. Ingo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]fastboot: remove duplicate unpack_to_rootfs() 2008-08-13 7:52 ` Ingo Molnar 2008-08-13 8:06 ` Andrew Morton @ 2008-08-13 8:11 ` Frans Meulenbroeks 2008-08-13 9:15 ` Ingo Molnar 1 sibling, 1 reply; 10+ messages in thread From: Frans Meulenbroeks @ 2008-08-13 8:11 UTC (permalink / raw) To: Ingo Molnar; +Cc: Shaohua Li, lkml, Andrew Morton, Arjan van de Ven 2008/8/13, Ingo Molnar <mingo@elte.hu>: > > * Ingo Molnar <mingo@elte.hu> wrote: > > > the dry_run variable is now unused in unpack_to_rootfs() and could be > > eliminated. > > also, while we are materially touching init/initramfs.c, that file has > collected a few uglies in the past few years, checkpatch --file says: > > total: 7 errors, 7 warnings, 3 checks, 562 lines checked > > it has a few other problems as well that can be seen if you look at the > file. Unused macros: > > /* Diagnostic functions (stubbed out) */ > #define Assert(cond,msg) > #define Trace(x) > #define Tracev(x) > #define Tracevv(x) > #define Tracec(c,x) > #define Tracecv(c,x) > > #define STATIC static > #define INIT __init > These are not really unused. A few lines later it reads: #include "../src/inflate.c" These macros are used within inflate.c (and perhaps the inclusion of inflate.c is not a good idea, maybe this should be in lib.a note that inflate.c is also included in init/do_mounts_rd.c; fortunately this is all init code (which is probably why include was used in the first place)) Frans. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch]fastboot: remove duplicate unpack_to_rootfs() 2008-08-13 8:11 ` Frans Meulenbroeks @ 2008-08-13 9:15 ` Ingo Molnar 0 siblings, 0 replies; 10+ messages in thread From: Ingo Molnar @ 2008-08-13 9:15 UTC (permalink / raw) To: Frans Meulenbroeks; +Cc: Shaohua Li, lkml, Andrew Morton, Arjan van de Ven * Frans Meulenbroeks <fransmeulenbroeks@gmail.com> wrote: > 2008/8/13, Ingo Molnar <mingo@elte.hu>: > > > > * Ingo Molnar <mingo@elte.hu> wrote: > > > > > the dry_run variable is now unused in unpack_to_rootfs() and could be > > > eliminated. > > > > also, while we are materially touching init/initramfs.c, that file has > > collected a few uglies in the past few years, checkpatch --file says: > > > > total: 7 errors, 7 warnings, 3 checks, 562 lines checked > > > > it has a few other problems as well that can be seen if you look at the > > file. Unused macros: > > > > /* Diagnostic functions (stubbed out) */ > > #define Assert(cond,msg) > > #define Trace(x) > > #define Tracev(x) > > #define Tracevv(x) > > #define Tracec(c,x) > > #define Tracecv(c,x) > > > > #define STATIC static > > #define INIT __init > > > These are not really unused. A few lines later it reads: > > #include "../src/inflate.c" > > These macros are used within inflate.c oh, i _knew_ i saw this zlib ugliness sometime in the past. > (and perhaps the inclusion of inflate.c is not a good idea, maybe this > should be in lib.a note that inflate.c is also included in > init/do_mounts_rd.c; fortunately this is all init code (which is > probably why include was used in the first place)) definitely. It's a different patch though. Ingo ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [patch]fastboot: remove duplicate unpack_to_rootfs() 2008-08-13 7:45 ` Ingo Molnar 2008-08-13 7:52 ` Ingo Molnar @ 2008-08-13 8:00 ` Li, Shaohua 2008-08-13 9:26 ` Li, Shaohua 2 siblings, 0 replies; 10+ messages in thread From: Li, Shaohua @ 2008-08-13 8:00 UTC (permalink / raw) To: Ingo Molnar; +Cc: lkml, Andrew Morton, Arjan van de Ven >-----Original Message----- >From: Ingo Molnar [mailto:mingo@elte.hu] >Sent: Wednesday, August 13, 2008 3:45 PM >To: Li, Shaohua >Cc: lkml; Andrew Morton; Arjan van de Ven >Subject: Re: [patch]fastboot: remove duplicate unpack_to_rootfs() > > >* Shaohua Li <shaohua.li@intel.com> wrote: > >> we check if initrd is initramfs first and then do real unpack. The >> check isn't required, we can directly do unpack. If initrd isn't >> initramfs, we can remove garbage. In my laptop, this saves 0.1s boot >> time. This penalizes non-initramfs case, but now initramfs is mostly >> widely used. > >clever concept! > >a few observations about the cleanup function: > >> +static void __init clean_rootfs(void) >> +{ >> + int fd = sys_open("/", O_RDONLY, 0); > >can this ever fail? I thought there will be a panic if it fails, so I didn't explicitly add a check here. I can add one. >> + struct linux_dirent64 *dirp = buf; >> + int count; >> + >> + memset(buf, 0, PAGE_SIZE); > >overflow: clearly allocating a 1024 bytes buffer and then clearing 4096 >bytes isnt that good? Oops, I use 4096 first and found it's too big, so changed to 1024, but forgot change all. My bad. >you could introduce a default-off CONFIG_DEBUG_ROOTFS_CLEANUP option >that does two runs of unpack_to_rootfs() and inserts an artificial >clean_rootfs() inbetween? Even if that debug patch doesnt get integrated >its a good test for the cleanup function. Actually I did the test already, just forgot change all size. >> + while (count > 0) { >> + while (count > 0) { >> + struct stat st; >> + >> + sys_newlstat(dirp->d_name, &st); > >can this ever fail? If yes we should at least WARN_ON_ONCE(). I'll add check. >> + if (S_ISDIR(st.st_mode)) >> + sys_rmdir(dirp->d_name); >> + else >> + sys_unlink(dirp->d_name); >> + >> + count -= dirp->d_reclen; > >can this ever zero-underflow, with a sufficiently corrupted initramfs? >We should check for 0 underflow to be sure. >> + dirp = (void *)dirp + dirp->d_reclen; > >likewise, we should size-overflow check this pointer. Failure modes of >overrunning the buffer are subtle and hard to notice/track down. I'm not quite sure here. Do you think the .d_reclen can be a incorrect value? >> static int __init populate_rootfs(void) >> { >> char *err = unpack_to_rootfs(__initramfs_start, >> @@ -531,13 +563,15 @@ static int __init populate_rootfs(void) >> int fd; >> printk(KERN_INFO "checking if image is initramfs..."); >> err = unpack_to_rootfs((char *)initrd_start, >> - initrd_end - initrd_start, 1); >> + initrd_end - initrd_start, 0); >> if (!err) { >> printk(" it is\n"); >> - unpack_to_rootfs((char *)initrd_start, >> - initrd_end - initrd_start, 0); >> free_initrd(); >> return 0; >> + } else { >> + clean_rootfs(); >> + unpack_to_rootfs(__initramfs_start, >> + __initramfs_end - __initramfs_start, 0); >> } > >the dry_run variable is now unused in unpack_to_rootfs() and could be >eliminated. Ok, I can cleanup this. Thanks, Shaohua ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [patch]fastboot: remove duplicate unpack_to_rootfs() 2008-08-13 7:45 ` Ingo Molnar 2008-08-13 7:52 ` Ingo Molnar 2008-08-13 8:00 ` Li, Shaohua @ 2008-08-13 9:26 ` Li, Shaohua 2 siblings, 0 replies; 10+ messages in thread From: Li, Shaohua @ 2008-08-13 9:26 UTC (permalink / raw) To: Ingo Molnar; +Cc: lkml, Andrew Morton, Arjan van de Ven [-- Attachment #1: Type: text/plain, Size: 695 bytes --] >-----Original Message----- >From: Ingo Molnar [mailto:mingo@elte.hu] >Sent: Wednesday, August 13, 2008 3:45 PM >To: Li, Shaohua >Cc: lkml; Andrew Morton; Arjan van de Ven >Subject: Re: [patch]fastboot: remove duplicate unpack_to_rootfs() > > >* Shaohua Li <shaohua.li@intel.com> wrote: > >> we check if initrd is initramfs first and then do real unpack. The >> check isn't required, we can directly do unpack. If initrd isn't >> initramfs, we can remove garbage. In my laptop, this saves 0.1s boot >> time. This penalizes non-initramfs case, but now initramfs is mostly >> widely used. Updated patch. Sorry for the attachment, my email client is broken. Thanks, Shaohua [-- Attachment #2: initramfs.patch --] [-- Type: application/octet-stream, Size: 3902 bytes --] we check if initrd is initramfs first and then do real unpack. The check isn't required, we can directly do unpack. If initrd isn't initramfs, we can remove garbage. In my laptop, this saves 0.1s boot time. This penalizes non-initramfs case, but now initramfs is mostly widely used. Signed-off-by: Shaohua Li <shaohua.li@intel.com> diff --git a/init/initramfs.c b/init/initramfs.c index 644fc01..da8d030 100644 --- a/init/initramfs.c +++ b/init/initramfs.c @@ -5,6 +5,7 @@ #include <linux/fcntl.h> #include <linux/delay.h> #include <linux/string.h> +#include <linux/dirent.h> #include <linux/syscalls.h> static __initdata char *message; @@ -121,8 +122,6 @@ static __initdata char *victim; static __initdata unsigned count; static __initdata loff_t this_header, next_header; -static __initdata int dry_run; - static inline void __init eat(unsigned n) { victim += n; @@ -183,10 +182,6 @@ static int __init do_header(void) parse_header(collected); next_header = this_header + N_ALIGN(name_len) + body_len; next_header = (next_header + 3) & ~3; - if (dry_run) { - read_into(name_buf, N_ALIGN(name_len), GotName); - return 0; - } state = SkipIt; if (name_len <= 0 || name_len > PATH_MAX) return 0; @@ -257,8 +252,6 @@ static int __init do_name(void) free_hash(); return 0; } - if (dry_run) - return 0; clean_path(collected, mode); if (S_ISREG(mode)) { int ml = maybe_link(); @@ -423,10 +416,9 @@ static void __init flush_window(void) outcnt = 0; } -static char * __init unpack_to_rootfs(char *buf, unsigned len, int check_only) +static char * __init unpack_to_rootfs(char *buf, unsigned len) { int written; - dry_run = check_only; header_buf = kmalloc(110, GFP_KERNEL); symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL); name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL); @@ -520,10 +512,57 @@ skip: initrd_end = 0; } +#define BUF_SIZE 1024 +static void __init clean_rootfs(void) +{ + int fd; + void *buf; + struct linux_dirent64 *dirp; + int count; + + fd = sys_open("/", O_RDONLY, 0); + WARN_ON(fd < 0); + if (fd < 0) + return; + buf = kzalloc(BUF_SIZE, GFP_KERNEL); + WARN_ON(!buf); + if (!buf) { + sys_close(fd); + return; + } + + dirp = buf; + count = sys_getdents64(fd, dirp, BUF_SIZE); + while (count > 0) { + while (count > 0) { + struct stat st; + int ret; + + ret = sys_newlstat(dirp->d_name, &st); + WARN_ON_ONCE(ret); + if (!ret) { + if (S_ISDIR(st.st_mode)) + sys_rmdir(dirp->d_name); + else + sys_unlink(dirp->d_name); + } + + count -= dirp->d_reclen; + dirp = (void *)dirp + dirp->d_reclen; + } + dirp = buf; + memset(buf, 0, BUF_SIZE); + count = sys_getdents64(fd, dirp, BUF_SIZE); + } + + sys_close(fd); + kfree(buf); +} + static int __init populate_rootfs(void) { char *err = unpack_to_rootfs(__initramfs_start, - __initramfs_end - __initramfs_start, 0); + __initramfs_end - __initramfs_start); if (err) panic(err); if (initrd_start) { @@ -531,13 +570,15 @@ static int __init populate_rootfs(void) int fd; printk(KERN_INFO "checking if image is initramfs..."); err = unpack_to_rootfs((char *)initrd_start, - initrd_end - initrd_start, 1); + initrd_end - initrd_start); if (!err) { printk(" it is\n"); - unpack_to_rootfs((char *)initrd_start, - initrd_end - initrd_start, 0); free_initrd(); return 0; + } else { + clean_rootfs(); + unpack_to_rootfs(__initramfs_start, + __initramfs_end - __initramfs_start); } printk("it isn't (%s); looks like an initrd\n", err); fd = sys_open("/initrd.image", O_WRONLY|O_CREAT, 0700); @@ -550,7 +591,7 @@ static int __init populate_rootfs(void) #else printk(KERN_INFO "Unpacking initramfs..."); err = unpack_to_rootfs((char *)initrd_start, - initrd_end - initrd_start, 0); + initrd_end - initrd_start); if (err) panic(err); printk(" done\n"); ^ permalink raw reply [flat|nested] 10+ messages in thread
* [2.6.30] What's in the async boot tree
@ 2009-03-15 18:23 Arjan van de Ven
2009-03-15 18:25 ` [PATCH] fastboot: remove duplicate unpack_to_rootfs() Arjan van de Ven
0 siblings, 1 reply; 10+ messages in thread
From: Arjan van de Ven @ 2009-03-15 18:23 UTC (permalink / raw)
To: linux-kernel
The following changes since commit 326d8519fc97be186c55ac605c3a7c957c758ae1:
Linus Torvalds (1):
Merge branch 'upstream' of git://ftp.linux-mips.org/pub/scm/upstream-linus
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arjan/linux-2.6-async-for-30.git master
Arjan van de Ven (2):
async: remove the temporary (2.6.29) "async is off by default" code
ide/net: flip the order of SATA and network init
Li, Shaohua (1):
fastboot: remove duplicate unpack_to_rootfs()
drivers/Makefile | 5 ++-
init/initramfs.c | 71 ++++++++++++++++++++++++++++++++++++++++++-----------
kernel/async.c | 10 +-------
3 files changed, 60 insertions(+), 26 deletions(-)
I will reply to this email with the three patches
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] fastboot: remove duplicate unpack_to_rootfs() 2009-03-15 18:23 [2.6.30] What's in the async boot tree Arjan van de Ven @ 2009-03-15 18:25 ` Arjan van de Ven 0 siblings, 0 replies; 10+ messages in thread From: Arjan van de Ven @ 2009-03-15 18:25 UTC (permalink / raw) To: linux-kernel; +Cc: Arjan van de Ven >From 4ea6c5331652555dcc96b86e078b7dceaa38005b Mon Sep 17 00:00:00 2001 From: Li, Shaohua <shaohua.li@intel.com> Date: Wed, 13 Aug 2008 17:26:01 +0800 Subject: [PATCH] fastboot: remove duplicate unpack_to_rootfs() we check if initrd is initramfs first and then do the real unpack. The check isn't required, we can directly do unpack. If the initrd isn't an initramfs, we can remove the garbage. In my laptop, this saves 0.1s boot time. This patch penalizes non-initramfs initrd case, but nowadays, initramfs is the most widely used method for initrds. Signed-off-by: Shaohua Li <shaohua.li@intel.com> Acked-by: Arjan van de Ven <arjan@infradead.org> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- init/initramfs.c | 71 ++++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 56 insertions(+), 15 deletions(-) diff --git a/init/initramfs.c b/init/initramfs.c index d9c941c..d3c56fc 100644 --- a/init/initramfs.c +++ b/init/initramfs.c @@ -5,6 +5,7 @@ #include <linux/fcntl.h> #include <linux/delay.h> #include <linux/string.h> +#include <linux/dirent.h> #include <linux/syscalls.h> #include <linux/utime.h> @@ -166,8 +167,6 @@ static __initdata char *victim; static __initdata unsigned count; static __initdata loff_t this_header, next_header; -static __initdata int dry_run; - static inline void __init eat(unsigned n) { victim += n; @@ -229,10 +228,6 @@ static int __init do_header(void) parse_header(collected); next_header = this_header + N_ALIGN(name_len) + body_len; next_header = (next_header + 3) & ~3; - if (dry_run) { - read_into(name_buf, N_ALIGN(name_len), GotName); - return 0; - } state = SkipIt; if (name_len <= 0 || name_len > PATH_MAX) return 0; @@ -303,8 +298,6 @@ static int __init do_name(void) free_hash(); return 0; } - if (dry_run) - return 0; clean_path(collected, mode); if (S_ISREG(mode)) { int ml = maybe_link(); @@ -476,10 +469,9 @@ static void __init flush_window(void) outcnt = 0; } -static char * __init unpack_to_rootfs(char *buf, unsigned len, int check_only) +static char * __init unpack_to_rootfs(char *buf, unsigned len) { int written; - dry_run = check_only; header_buf = kmalloc(110, GFP_KERNEL); symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL); name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL); @@ -574,10 +566,57 @@ skip: initrd_end = 0; } +#define BUF_SIZE 1024 +static void __init clean_rootfs(void) +{ + int fd; + void *buf; + struct linux_dirent64 *dirp; + int count; + + fd = sys_open("/", O_RDONLY, 0); + WARN_ON(fd < 0); + if (fd < 0) + return; + buf = kzalloc(BUF_SIZE, GFP_KERNEL); + WARN_ON(!buf); + if (!buf) { + sys_close(fd); + return; + } + + dirp = buf; + count = sys_getdents64(fd, dirp, BUF_SIZE); + while (count > 0) { + while (count > 0) { + struct stat st; + int ret; + + ret = sys_newlstat(dirp->d_name, &st); + WARN_ON_ONCE(ret); + if (!ret) { + if (S_ISDIR(st.st_mode)) + sys_rmdir(dirp->d_name); + else + sys_unlink(dirp->d_name); + } + + count -= dirp->d_reclen; + dirp = (void *)dirp + dirp->d_reclen; + } + dirp = buf; + memset(buf, 0, BUF_SIZE); + count = sys_getdents64(fd, dirp, BUF_SIZE); + } + + sys_close(fd); + kfree(buf); +} + static int __init populate_rootfs(void) { char *err = unpack_to_rootfs(__initramfs_start, - __initramfs_end - __initramfs_start, 0); + __initramfs_end - __initramfs_start); if (err) panic(err); if (initrd_start) { @@ -585,13 +624,15 @@ static int __init populate_rootfs(void) int fd; printk(KERN_INFO "checking if image is initramfs..."); err = unpack_to_rootfs((char *)initrd_start, - initrd_end - initrd_start, 1); + initrd_end - initrd_start); if (!err) { printk(" it is\n"); - unpack_to_rootfs((char *)initrd_start, - initrd_end - initrd_start, 0); free_initrd(); return 0; + } else { + clean_rootfs(); + unpack_to_rootfs(__initramfs_start, + __initramfs_end - __initramfs_start); } printk("it isn't (%s); looks like an initrd\n", err); fd = sys_open("/initrd.image", O_WRONLY|O_CREAT, 0700); @@ -604,7 +645,7 @@ static int __init populate_rootfs(void) #else printk(KERN_INFO "Unpacking initramfs..."); err = unpack_to_rootfs((char *)initrd_start, - initrd_end - initrd_start, 0); + initrd_end - initrd_start); if (err) panic(err); printk(" done\n"); -- 1.6.0.6 -- Arjan van de Ven Intel Open Source Technology Centre For development, discussion and tips for power savings, visit http://www.lesswatts.org ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-03-15 18:26 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-08-13 6:07 [patch]fastboot: remove duplicate unpack_to_rootfs() Shaohua Li 2008-08-13 7:45 ` Ingo Molnar 2008-08-13 7:52 ` Ingo Molnar 2008-08-13 8:06 ` Andrew Morton 2008-08-13 9:14 ` Ingo Molnar 2008-08-13 8:11 ` Frans Meulenbroeks 2008-08-13 9:15 ` Ingo Molnar 2008-08-13 8:00 ` Li, Shaohua 2008-08-13 9:26 ` Li, Shaohua 2009-03-15 18:23 [2.6.30] What's in the async boot tree Arjan van de Ven 2009-03-15 18:25 ` [PATCH] fastboot: remove duplicate unpack_to_rootfs() Arjan van de Ven
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome