Re: [RFC v13][PATCH 05/14] x86 support for checkpoint/restart
flat view
From: Nathan Lynch <hidden>
Date: 2009-02-24 07:48:10
Also in:
linux-mm, lkml
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 [off-list ref] wrote:
quoted hunk ↗ jump to hunk
+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 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? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>