lkml.org 
[lkml]   [2008]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] INITRAMFS: Add option to preserve mtime from INITRAMFS cpio images
    On Wed, 3 Sep 2008 13:29:38 -0700
    Nye Liu <nyet@nyet.org> wrote:

    > Resubmission of initramfs preserve mtime patch following Andrew Morton's
    > suggestions.

    It's nice to provide some sort of accounting for the reviewer's
    individual comments. But in this case I only had three or fopur and I
    see how they were addressed.

    > From: Nye Liu <nyet@nyet.org>
    >
    > When unpacking the cpio into the initramfs, mtimes are not preserved by
    > default. This patch adds an INITRAMFS_PRESERVE_MTIME option that allows mtimes
    > stored in the cpio image to be used when constructing the initramfs. For
    > embedded applications that run exclusively out of the initramfs, this is
    > invaluable.

    Why is it "invlauable". Please explain this value in full detail -
    it's the whole reason for merging the patch!

    > Signed-off-by: Nye Liu <nyet@nyet.org>
    >
    > diff --git a/init/initramfs.c b/init/initramfs.c
    > index 644fc01..ebfc049 100644
    > --- a/init/initramfs.c
    > +++ b/init/initramfs.c
    > @@ -6,6 +6,7 @@
    > #include <linux/delay.h>
    > #include <linux/string.h>
    > #include <linux/syscalls.h>
    > +#include <linux/utime.h>
    >
    > static __initdata char *message;
    > static void __init error(char *x)
    > @@ -72,6 +73,38 @@ static void __init free_hash(void)
    > }
    > }
    >
    > +static __initdata LIST_HEAD(dir_list);
    > +struct dir_entry {
    > + struct list_head list;
    > + char *name;
    > + struct utimbuf mtime;
    > +};
    > +
    > +static void __init dir_add(const char *name, struct utimbuf mtime)
    > +{
    > + struct dir_entry *de = kmalloc(sizeof(struct dir_entry), GFP_KERNEL);
    > + if (!de)
    > + panic("can't allocate dir_entry buffer");
    > + INIT_LIST_HEAD(&de->list);
    > + de->name = kstrdup(name, GFP_KERNEL);
    > + de->mtime = mtime;
    > + list_add(&de->list, &dir_list);
    > +}
    > +
    > +static void __init dir_utime(void)
    > +{
    > + struct list_head *e, *tmp;
    > + list_for_each_safe(e, tmp, &dir_list) {
    > + struct dir_entry *de = list_entry(e, struct dir_entry, list);
    > + list_del(e);
    > + sys_utime(de->name, &de->mtime);

    gargh. Why does this work? It's normally a big fail to pass a kernel
    address into a system call. I guess we're running under KERNEL_DS here
    and getname() and strncpy_from_user() did the right thing.

    On what CPU architecture was this tested?

    Wouldn't it be simpler to put a timespec into struct dir_entry then go
    direct to do_utimes() here?

    > + kfree(de->name);
    > + kfree(de);
    > + }
    > +}
    > +
    > +static __initdata struct utimbuf mtime;
    > +
    > /* cpio header parsing */
    >
    > static __initdata unsigned long ino, major, minor, nlink;
    > @@ -97,6 +130,7 @@ static void __init parse_header(char *s)
    > uid = parsed[2];
    > gid = parsed[3];
    > nlink = parsed[4];
    > + mtime.actime = mtime.modtime = parsed[5];
    > body_len = parsed[6];
    > major = parsed[7];
    > minor = parsed[8];
    > @@ -130,6 +164,7 @@ static inline void __init eat(unsigned n)
    > count -= n;
    > }
    >
    > +static __initdata char *vcollected;
    > static __initdata char *collected;
    > static __initdata int remains;
    > static __initdata char *collect;
    > @@ -271,6 +306,7 @@ static int __init do_name(void)
    > if (wfd >= 0) {
    > sys_fchown(wfd, uid, gid);
    > sys_fchmod(wfd, mode);
    > + vcollected = kstrdup(collected, GFP_KERNEL);
    > state = CopyFile;
    > }
    > }
    > @@ -278,12 +314,14 @@ static int __init do_name(void)
    > sys_mkdir(collected, mode);
    > sys_chown(collected, uid, gid);
    > sys_chmod(collected, mode);
    > + dir_add(collected, mtime);
    > } else if (S_ISBLK(mode) || S_ISCHR(mode) ||
    > S_ISFIFO(mode) || S_ISSOCK(mode)) {
    > if (maybe_link() == 0) {
    > sys_mknod(collected, mode, rdev);
    > sys_chown(collected, uid, gid);
    > sys_chmod(collected, mode);
    > + sys_utime(collected, &mtime);
    > }
    > }
    > return 0;
    > @@ -294,6 +332,8 @@ static int __init do_copy(void)
    > if (count >= body_len) {
    > sys_write(wfd, victim, body_len);
    > sys_close(wfd);
    > + sys_utime(vcollected, &mtime);

    and here?

    > + kfree(vcollected);
    > eat(body_len);
    > state = SkipIt;
    > return 0;
    > @@ -305,12 +345,26 @@ static int __init do_copy(void)
    > }
    > }
    >
    > +static long __init do_lutime(char __user *filename,
    > + struct utimbuf __user *times)
    > +{
    > + struct timespec t[2];
    > +
    > + t[0].tv_sec = times->actime;
    > + t[0].tv_nsec = 0;
    > + t[1].tv_sec = times->modtime;
    > + t[1].tv_nsec = 0;
    > +
    > + return do_utimes(AT_FDCWD, filename, t, AT_SYMLINK_NOFOLLOW);
    > +}
    > +
    > static int __init do_symlink(void)
    > {
    > collected[N_ALIGN(name_len) + body_len] = '\0';
    > clean_path(collected, 0);
    > sys_symlink(collected + N_ALIGN(name_len), collected);
    > sys_lchown(collected, uid, gid);
    > + do_lutime(collected, &mtime);
    > state = SkipIt;
    > next_state = Reset;
    > return 0;
    > @@ -466,6 +520,7 @@ static char * __init unpack_to_rootfs(char *buf, unsigned len, int check_only)
    > buf += inptr;
    > len -= inptr;
    > }
    > + dir_utime();

    Perhaps this is the simplest implementation - I didn't check the fine
    details. What's your thinking here?


    \
     
     \ /
      Last update: 2008-09-04 00:25    [W:0.030 / U:1.612 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site