Re: [RFC v5][PATCH 9/9] Restore open file descriprtors

[Oren Laadan]

Serge E. Hallyn wrote:
> Quoting Oren Laadan (orenl@xxxxxxxxxxxxxxx):
> > Restore open file descriptors: for each FD read 'struct cr_hdr_fd_ent'
> > and lookup objref in the hash table; if not found (first occurence), read
> > in 'struct cr_hdr_fd_data', create a new FD and register in the hash.
> > Otherwise attach the file pointer from the hash as an FD.
> >
> > This patch only handles basic FDs - regular files, directories and also
> > symbolic links.
> >
> > Signed-off-by: Oren Laadan <orenl@xxxxxxxxxxxxxxx>
> > ---
> >  checkpoint/Makefile        |    2 +-
> >  checkpoint/restart.c       |    4 +
> >  checkpoint/rstr_file.c     |  202 ++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/checkpoint.h |    1 +
> >  4 files changed, 208 insertions(+), 1 deletions(-)
> >  create mode 100644 checkpoint/rstr_file.c
> >
> > diff --git a/checkpoint/Makefile b/checkpoint/Makefile
> > index 7496695..88bbc10 100644
> > --- a/checkpoint/Makefile
> > +++ b/checkpoint/Makefile
> > @@ -3,4 +3,4 @@
> >  #
> >
> >  obj-$(CONFIG_CHECKPOINT_RESTART) += sys.o checkpoint.o restart.o objhash.o \
> > -		ckpt_mem.o rstr_mem.o ckpt_file.o
> > +		ckpt_mem.o rstr_mem.o ckpt_file.o rstr_file.o
> > diff --git a/checkpoint/restart.c b/checkpoint/restart.c
> > index a0d5e60..956e274 100644
> > --- a/checkpoint/restart.c
> > +++ b/checkpoint/restart.c
> > @@ -212,6 +212,10 @@ static int cr_read_task(struct cr_ctx *ctx)
> >  	cr_debug("memory: ret %d\n", ret);
> >  	if (ret < 0)
> >  		goto out;
> > +	ret = cr_read_files(ctx);
> > +	cr_debug("files: ret %d\n", ret);
> > +	if (ret < 0)
> > +		goto out;
> >  	ret = cr_read_thread(ctx);
> >  	cr_debug("thread: ret %d\n", ret);
> >  	if (ret < 0)
> > diff --git a/checkpoint/rstr_file.c b/checkpoint/rstr_file.c
> > new file mode 100644
> > index 0000000..780c0fc
> > --- /dev/null
> > +++ b/checkpoint/rstr_file.c
> > @@ -0,0 +1,202 @@
> > +/*
> > + *  Checkpoint file descriptors
> > + *
> > + *  Copyright (C) 2008 Oren Laadan
> > + *
> > + *  This file is subject to the terms and conditions of the GNU General Public
> > + *  License.  See the file COPYING in the main directory of the Linux
> > + *  distribution for more details.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/sched.h>
> > +#include <linux/fs.h>
> > +#include <linux/file.h>
> > +#include <linux/fdtable.h>
> > +#include <linux/fsnotify.h>
> > +#include <linux/syscalls.h>
> > +#include <linux/checkpoint.h>
> > +#include <linux/checkpoint_hdr.h>
> > +
> > +#include "checkpoint_file.h"
> > +
> > +static int cr_close_all_fds(struct files_struct *files)
> > +{
> > +	int *fdtable;
> > +	int nfds;
> > +
> > +	nfds = cr_scan_fds(files, &fdtable);
> > +	if (nfds < 0)
> > +		return nfds;
> > +	while (nfds--)
> > +		sys_close(fdtable[nfds]);
> > +	kfree(fdtable);
> > +	return 0;
> > +}
> > +
> > +/**
> > + * cr_attach_file - attach a lonely file ptr to a file descriptor
> > + * @file: lonely file pointer
> > + */
> > +static int cr_attach_file(struct file *file)
> > +{
> > +	int fd = get_unused_fd_flags(0);
> > +
> > +	if (fd >= 0) {
> > +		fsnotify_open(file->f_path.dentry);
> > +		fd_install(fd, file);
> > +	}
> > +	return fd;
> > +}
> > +
> > +#define CR_SETFL_MASK (O_APPEND|O_NONBLOCK|O_NDELAY|FASYNC|O_DIRECT|O_NOATIME)
> > +
> > +/* cr_read_fd_data - restore the state of a given file pointer */
> > +static int
> > +cr_read_fd_data(struct cr_ctx *ctx, struct files_struct *files, int parent)
> > +{
> > +	struct cr_hdr_fd_data *hh = cr_hbuf_get(ctx, sizeof(*hh));
>
> You're leaking hh in a whole slew of error paths.

No. (this was discussed earlier already).

cr_hbuf_get() "allocates" space inside a dedicated buffer for headers
in the checkpoint context (ctx->hbuf). It does not allocate new kernel
memory. Instead, it returns the current position in that buffer
ctx->hbuf[ctx->hpos], and advances ctx->hpos appropriately. On the
other side, cr_hbuf_put() reverses that effect, reducing ctx->hpos
accordingly.

If an error occurs, the checkpoint (or restart) operation is aborted,
and eventually the context (ctx) will be cleaned up; at that point the
special purpose buffer will be freed.

[...]

Oren.


--
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/