On Wed, Aug 1, 2018 at 7:48 AM, Waldemar Kozaczuk <[email protected]> wrote:
> This patch optimizes RAM utilization of bootfs by > eliminating unnecessary copy of data. It does so by > pointing created file nodes to existing data offset in memory > which is part of area where kernel is copied after decompression. > > In essence we add new flag rn_owns_buf to RAMFS ramfs_node to track > if we can actually free memory wherever it happens. By default > rn_owns_buf is set to true but new function ramfs_set_file_data called > from unpack_bootfs() sets it to false. > > The savings of RAM are equal to the size of build/release/bootfs.bin > which means we save around 700K with ZFS images, 0 with ROSF and > as much as application code size with RAMFS where the improvement > is most significant especially with for example Java images. > > Please note that file data referenced by nodes created during > unpack_bootfs() would point to wherever bootfs.bin data is in > the uncompressed kernel area which means it is most likely not aligned > which possibly means slower access. This could be improved by > making individual files aligned in bootfs.bin. > > Fixes #977 > > Signed-off-by: Waldemar Kozaczuk <[email protected]> > --- > fs/ramfs/ramfs.h | 1 + > fs/ramfs/ramfs_vnops.cc | 38 ++++++++++++++++++++++++++++++++++---- > fs/vfs/main.cc | 26 ++++++++++++++++++++++++++ > loader.cc | 8 ++++++++ > 4 files changed, 69 insertions(+), 4 deletions(-) > > diff --git a/fs/ramfs/ramfs.h b/fs/ramfs/ramfs.h > index f21ff4bd..88ccbe72 100644 > --- a/fs/ramfs/ramfs.h > +++ b/fs/ramfs/ramfs.h > @@ -58,6 +58,7 @@ struct ramfs_node { > struct timespec rn_atime; > struct timespec rn_mtime; > int rn_mode; > + bool rn_owns_buf; > }; > > struct ramfs_node *ramfs_allocate_node(const char *name, int type); > diff --git a/fs/ramfs/ramfs_vnops.cc b/fs/ramfs/ramfs_vnops.cc > index cf1ae272..9291a4ec 100644 > --- a/fs/ramfs/ramfs_vnops.cc > +++ b/fs/ramfs/ramfs_vnops.cc > @@ -94,6 +94,7 @@ ramfs_allocate_node(const char *name, int type) > np->rn_mode = S_IFREG|0777; > > set_times_to_now(&(np->rn_ctime), &(np->rn_atime), &(np->rn_mtime)); > + np->rn_owns_buf = true; > > return np; > } > @@ -101,7 +102,7 @@ ramfs_allocate_node(const char *name, int type) > void > ramfs_free_node(struct ramfs_node *np) > { > - if (np->rn_buf != NULL) > + if (np->rn_buf != NULL && np->rn_owns_buf) > free(np->rn_buf); > > free(np->rn_name); > @@ -336,7 +337,8 @@ ramfs_truncate(struct vnode *vp, off_t length) > > if (length == 0) { > if (np->rn_buf != NULL) { > - free(np->rn_buf); > + if(np->rn_owns_buf) > + free(np->rn_buf); > np->rn_buf = NULL; > np->rn_bufsize = 0; > } > @@ -348,10 +350,12 @@ ramfs_truncate(struct vnode *vp, off_t length) > return EIO; > if (np->rn_size != 0) { > memcpy(new_buf, np->rn_buf, vp->v_size); > - free(np->rn_buf); > + if(np->rn_owns_buf) > + free(np->rn_buf); > } > np->rn_buf = (char *) new_buf; > np->rn_bufsize = new_size; > + np->rn_owns_buf = true; > } > np->rn_size = length; > vp->v_size = length; > @@ -413,6 +417,30 @@ ramfs_read(struct vnode *vp, struct file *fp, struct > uio *uio, int ioflag) > return uiomove(np->rn_buf + uio->uio_offset, len, uio); > } > > +int > +ramfs_set_file_data(struct vnode *vp, const void *data, size_t size) > +{ > + struct ramfs_node *np = (ramfs_node *) vp->v_data; > + > + if (vp->v_type == VDIR) { > + return EISDIR; > + } > + if (vp->v_type != VREG) { > + return EINVAL; > + } > + if (np->rn_buf) { > + return EINVAL; > + } > + > + np->rn_buf = (char *) data; > + np->rn_bufsize = size; > + np->rn_size = size; > + vp->v_size = size; > + np->rn_owns_buf = false; > + > + return 0; > +} > + > static int > ramfs_write(struct vnode *vp, struct uio *uio, int ioflag) > { > @@ -448,13 +476,15 @@ ramfs_write(struct vnode *vp, struct uio *uio, int > ioflag) > return EIO; > if (np->rn_size != 0) { > memcpy(new_buf, np->rn_buf, vp->v_size); > - free(np->rn_buf); > + if(np->rn_owns_buf) > + free(np->rn_buf); > } > np->rn_buf = (char *) new_buf; > np->rn_bufsize = new_size; > } > np->rn_size = end_pos; > vp->v_size = end_pos; > + np->rn_owns_buf = true; > } > > set_times_to_now(&(np->rn_mtime), &(np->rn_ctime)); > diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc > index c41c6947..51980a4b 100644 > --- a/fs/vfs/main.cc > +++ b/fs/vfs/main.cc > @@ -2132,11 +2132,14 @@ struct bootfs_metadata { > > extern char bootfs_start; > > +#define ONE_MB (1024 * 1024.0) > +int ramfs_set_file_data(struct vnode *vp, const void *data, size_t size); > void unpack_bootfs(void) > { > struct bootfs_metadata *md = (struct bootfs_metadata *)&bootfs_start; > int fd, i; > > + size_t saved_ram_in_bytes = 0; > for (i = 0; md[i].name[0]; i++) { > int ret; > char *p; > @@ -2173,15 +2176,38 @@ void unpack_bootfs(void) > sys_panic("unpack_bootfs failed"); > } > > +#if 0 > If you want to leave this #if 0 behind, maybe write a comment saying what this case did? ret = write(fd, &bootfs_start + md[i].offset, md[i].size); > if (ret != md[i].size) { > kprintf("write failed, ret = %d, errno = %d\n", > ret, errno); > sys_panic("unpack_bootfs failed"); > } > +#endif > +#if 1 > If this is the other implementation of the #if 0 and they are exclusive, it can be a #else instead if a separate #if. + struct file *fp; > + int error = fget(fd, &fp); > + if (error) { > + kprintf("couldn't fget %s: %d\n", > + md[i].name, error); > + sys_panic("unpack_bootfs failed"); > + } > + > + struct vnode *vp = fp->f_dentry->d_vnode; > + ret = ramfs_set_file_data(vp, &bootfs_start + md[i].offset, > md[i].size); > + if (ret) { > + kprintf("ramfs_set_file_data failed, ret = %d\n", ret); > + sys_panic("unpack_bootfs failed"); > + } > > + fdrop(fp); > +#endif > + saved_ram_in_bytes += md[i].size; > This shouldn't be inside the #if 1 above? close(fd); > } > + > + printf("Saved %0.2f MB of RAM which should be equal to size of > ./build/release/bootfs.bin\n", > + saved_ram_in_bytes / ONE_MB); > Isn't "printf" (so it's always printed) too drastic? Maybe debug() or remove it completely? } > > void mount_rootfs(void) > diff --git a/loader.cc b/loader.cc > index 3f88ebda..25a403c3 100644 > --- a/loader.cc > +++ b/loader.cc > @@ -55,6 +55,9 @@ > #include "drivers/null.hh" > > #include "libc/network/__dns.hh" > +#include <sys/sysinfo.h> > + > +#define ONE_MB (1024 * 1024.0) > > using namespace osv; > using namespace osv::clock::literals; > @@ -436,6 +439,11 @@ void* do_main_thread(void *_main_args) > boot_time.print_chart(); > } > > + struct sysinfo info; > + sysinfo(&info); > + long unsigned int used_ram = info.totalram - info.freeram; > + printf("Used %0.2f MB of RAM out of %0.2f MB\n", used_ram / ONE_MB, > info.totalram / ONE_MB); > Again, do we want OSv to always print this? Maybe just in verbose mode (-V or --verbose), so use debug() here? + > if (!opt_redirect.empty()) { > // redirect stdout and stdin to the given file, instead of the > console > // use ">>filename" to append, instead of replace, to a file. > -- > 2.17.1 > > -- > You received this message because you are subscribed to the Google Groups > "OSv Development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
