Re: [PATCH] INITRAMFS: Add option to preserve mtime from INITRAMFScpio images

[Nye Liu]

On Wed, Sep 03, 2008 at 03:48:40PM -0700, Andrew Morton wrote:
> On Wed, 3 Sep 2008 15:41:31 -0700
> Nye Liu <nyet@xxxxxxxx> wrote:
> 
> > > >  	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?  
> > > 
> > 
> > The main problem is that i need to save off the entire list for later
> > processing of the directory mtimes... if i process the directory mtimes
> > in the same pass as the file/link mtimes, touching the directory inode
> > when creating/modifying the file/links updates the directory mtime, and
> > overwrites whatever mtime i set the directory to when i created it.
> > 
> > The only solution is to do a two pass, which is why the list is
> > necessary. If there is a better way, i did not find it :(
> > 
> > It could be that i misunderstood your question though :)
> 
> I'm wondering whether this code need to use `struct utimbuf' at all. 
> Or at least, as much as it does.  utimbuf is more a userspace-facing
> thing whereas in-kernel timespecs and timevals are more common.
> 
> The code as you have it does a fair few conversions into utimbuf format
> (both directly and via the existing functions which it calls).  Would
> it be simpler if it dealt in terms of timespecs?
> 

It could be. Heck, it would be even simpler to just use a single time_t (since
cpio doesn't have timespecs OR seperate atime/mtimes):

static __initdata LIST_HEAD(dir_list);
struct dir_entry {
        struct list_head list;
        char *name;
        time_t mtime;
};

then make a simple wrapper for sys_utime() to convert the time_t into a the
utimbuf:

static void __init do_utime(char *name, time_t mtime)
{
        struct utimbuf time;
        time.actime=time.modtime=mtime;
        sys_utime(name, &time);
}

I can try it that way and resubmit. Gimme a bit to compose a patch

ALSO it disturbs me about the alloc problem you mentioned, especially since
i did NOT TEST on anything but our embedded (ppc/powerpc) target.

-- 
Nye Liu
nliu@xxxxxxx
(818) 772-6235x248
(818) 772-0576 fax

"Who would be stupid enough to quote a fictitious character?"
	-- Don Quixote
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/