Re: [RFC v5][PATCH 5/8] Restore memory address space

Dave Hansen
2008-09-15T19:14:37Z

On Sat, 2008-09-13 at 19:06 -0400, Oren Laadan wrote:
> +struct file *cr_read_open_fname(struct cr_ctx *ctx, int flags, int mode)
> +{
> + struct file *file;
> + char *fname;
> + int flen, ret;
> +
> + flen = PATH_MAX;
> + fname = kmalloc(flen, GFP_KERNEL);
> + if (!fname)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = cr_read_fname(ctx, fname, flen);
> + cr_debug("fname '%s' flags %#x mode %#x\n", fname, flags, mode);
> + if (ret >= 0)
> + file = filp_open(fname, flags, mode);
> + else
> + file = ERR_PTR(ret);
> +
> + kfree(fname);
> + return file;
> +}

PATH_MAX is about as short of a global macro name as you're going to
get. It should just be used directly. Please kill 'flen'.

> +static int cr_read_pages_vaddrs(struct cr_ctx *ctx, unsigned long nr_pages)
> +{
> + struct cr_pgarr *pgarr;
> + unsigned long *vaddrp;
> + int nr, ret;
> +
> + while (nr_pages) {
> + pgarr = cr_pgarr_current(ctx);
> + if (!pgarr)
> + return -ENOMEM;
> + nr = cr_pgarr_nr_free(pgarr);
> + if (nr > nr_pages)
> + nr = nr_pages;
> + vaddrp = &pgarr->vaddrs[pgarr->nr_used];
> + ret = cr_kread(ctx, vaddrp, nr * sizeof(unsigned long));
> + if (ret < 0)
> + return ret;
> + pgarr->nr_used += nr;
> + nr_pages -= nr;
> + }
> + return 0;
> +}
> +
> +static int cr_page_read(struct cr_ctx *ctx, struct page *page, char *buf)
> +{
> + void *ptr;
> + int ret;
> +
> + ret = cr_kread(ctx, buf, PAGE_SIZE);
> + if (ret < 0)
> + return ret;
> +
> + ptr = kmap_atomic(page, KM_USER1);
> + memcpy(ptr, buf, PAGE_SIZE);
> + kunmap_atomic(page, KM_USER1);
> +
> + return 0;
> +}
> +
> +/**
> + * cr_read_pages_contents - read in data of pages in page-array chain
> + * @ctx - restart context
> + */
> +static int cr_read_pages_contents(struct cr_ctx *ctx)
> +{
> + struct mm_struct *mm = current->mm;
> + struct cr_pgarr *pgarr;
> + unsigned long *vaddrs;
> + char *buf;
> + int i, ret = 0;
> +
> + buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + down_read(&mm->mmap_sem);
> + list_for_each_entry_reverse(pgarr, &ctx->pgarr_list, list) {
> + vaddrs = pgarr->vaddrs;
> + for (i = 0; i < pgarr->nr_used; i++) {
> + struct page *page;
> +
> + ret = get_user_pages(current, mm, vaddrs[i],
> + 1, 1, 1, &page, NULL);
> + if (ret < 0)
> + goto out;
> +
> + ret = cr_page_read(ctx, page, buf);
> + page_cache_release(page);
> +
> + if (ret < 0)
> + goto out;
> + }
> + }
> +
> + out:
> + up_read(&mm->mmap_sem);
> + kfree(buf);
> + return 0;
> +}
> +
> +/**
> + * cr_read_private_vma_contents - restore contents of a VMA with private memory
> + * @ctx - restart context
> + *
> + * Reads a header that specifies how many pages will follow, then reads
> + * a list of virtual addresses into ctx->pgarr_list page-array chain,
> + * followed by the actual contents of the corresponding pages. Iterates
> + * these steps until reaching a header specifying "0" pages, which marks
> + * the end of the contents.
> + */
> +static int cr_read_private_vma_contents(struct cr_ctx *ctx)
> +{
> + struct cr_hdr_pgarr *hh;
> + unsigned long nr_pages;
> + int parent, ret = 0;
> +
> + while (!ret) {
> + hh = cr_hbuf_get(ctx, sizeof(*hh));
> + parent = cr_read_obj_type(ctx, hh, sizeof(*hh), CR_HDR_PGARR);
> + if (parent < 0)
> + return parent;
> + else if (parent != 0)
> + return -EINVAL;
> +
> + cr_debug("nr_pages %ld\n", (unsigned long) hh->nr_pages);
> +
> + nr_pages = hh->nr_pages;
> + cr_hbuf_put(ctx, sizeof(*hh));
> +
> + if (!nr_pages)
> + break;
> +
> + ret = cr_read_pages_vaddrs(ctx, nr_pages);
> + if (ret < 0)
> + break;
> + ret = cr_read_pages_contents(ctx);
> + if (ret < 0)
> + break;
> + cr_pgarr_reset_all(ctx);
> + }
> +
> + return ret;
> +}

That's an interesting loop condition, especially since it will never
actually get triggered. while(1) would give the same functionality,
right?

-- Dave