Re: [RFC v13][PATCH 05/14] x86 support for checkpoint/restart



Nathan Lynch wrote:
Hi, this is an old thread I guess, but I just noticed some issues while
looking at this code.

On Tue, 27 Jan 2009 12:08:03 -0500
Oren Laadan <orenl@xxxxxxxxxxxxxxx> wrote:
+static int cr_read_cpu_fpu(struct cr_ctx *ctx, struct task_struct *t)
+{
+ void *xstate_buf = cr_hbuf_get(ctx, xstate_size);
+ int ret;
+
+ ret = cr_kread(ctx, xstate_buf, xstate_size);
+ if (ret < 0)
+ goto out;
+
+ /* i387 + MMU + SSE */
+ preempt_disable();
+
+ /* init_fpu() also calls set_used_math() */
+ ret = init_fpu(current);
+ if (ret < 0)
+ return ret;

Several problems here:
* init_fpu can call kmem_cache_alloc(GFP_KERNEL), but is called here
with preempt disabled (init_fpu could use a might_sleep annotation?)
* if init_fpu returns an error, we get preempt imbalance
* if init_fpu returns an error, we "leak" the cr_hbuf_get for
xstate_buf

Fixed, thanks.


Speaking of cr_hbuf_get... I'd prefer to see that "allocator" go away
and its users converted to kmalloc/kfree (this is what I've done for
the powerpc C/R code, btw).

Using the slab allocator would:

* make the code less obscure and easier to review
* make the code more amenable to static analysis
* gain the benefits of slab debugging at runtime

But I think this has been pointed out before. If I understand the
justification for cr_hbuf_get correctly, the allocations it services
are somehow known to be bounded in size and nesting. But even if that
is the case, it's not much of a reason to avoid using kmalloc, is it?


The reason I want these wrappers (as opposed to allocators) in place is
allow an optimization that will reduce application downtime during checkpoint.

Since we freeze the container during checkpoint, the applications inside are
unresponsive. The idea is to minimize the downtime by buffering the checkpoint
data in the kernel while the applications are frozen, and defer the (slow)
write-back of the buffer until after the application is allowed to resume
execution. (Memory pages will be marked COW instead of a physical copy in the
kernel).

To that, we'll need the wrapper to not only allocate memory, but also track
all the pieces together as a long buffer. Actual implementation details are
not important now, but having a wrapper in place is.

Consequently, although I prefer not to change the current implementation of
cr_hbuf_get/put(), if you find it really helpful to change to kmalloc/kfree
I won't stand in the way. However, I do insist that the wrappers remain.

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/

Relevant Pages

  • Re: Scrolling through large view area.
    ... the spike was at the allocator. ... buffer has to be", so they called the allocator to allocate a buffer of the correct ... In a third case, I had put a "bubble sort" into my program, knowing I could replace it ... Ten minutes later, it was still sorting. ...
    (microsoft.public.vc.mfc)
  • Re: How to correctly buffer (FIFO) a number of samples to create a time-shifting filter ?
    ... If you do not have your own allocator you are at the mercy of downstream ... It seams that this will happen anyway also with transinplace filters ... real copy the samples and manage my own buffer:(... ...
    (microsoft.public.win32.programmer.directx.video)
  • Re: sort(1) memory usage
    ... The buffer size allocated in that case ... nbuf = realloc; ... With a good allocator - and depending to some extent on the memory usage ... I think you're bashing GNU just because you feel ...
    (freebsd-hackers)
  • Re: Custom filter gets "These filters cannot agree on a connection
    ... I have a very different kind of problem with my parser filter. ... Somhow when I am trying to set the Allocator for IAsyncReader to have above ... multiple of 512 for above mentioned size with buffer alignment value 512. ... Well documentation says output pin does't really have to agree on allocator ...
    (microsoft.public.win32.programmer.directx.video)
  • Re: Benchmark: STLs list vs. hand-coded one
    ... And having 64MB buffer allocated you will need to write your own new, ... or malloc() implementations to manage memory in the buffer and do ... unlikely that a custom memory allocator can be any faster than a generic ...
    (comp.arch.embedded)
Loading