[Devel] Re: [PATCH 1/1] checkpoint: Note checkpointability of mm_struct (v2)

[Serge E. Hallyn]

Quoting Dan Smith (danms at us.ibm.com):
> SH> +static inline void task_show_checkpointable(struct seq_file *m,
> SH> +						struct task_struct *p)
> SH> +{
> SH> +	if (test_bit(0, &p->mm->may_checkpoint))
> SH> +		seq_printf(m,	"mm is checkpointable\n");
>                              ^^^
> You have a hard tab in the middle of the line.  Was that intentional?
> 
> SH> +	else
> SH> +		seq_printf(m,	"mm is not checkpointable\n");
> SH> +
> SH> +	if (test_bit(0, &p->files->may_checkpoint))
> SH> +		seq_printf(m,	"files are checkpointable\n");
> SH> +	else
> SH> +		seq_printf(m,	"files are not checkpointable\n");
> SH> +}
> 
> These too.
> 
> SH> +static inline void __mm_deny_checkpointing(struct mm_struct *mm,
> SH> +		char *file, int line)
> SH> +{
> SH> +	if (!test_and_clear_bit(0, &mm->may_checkpoint))
> SH> +		return;
> SH> +	printk(KERN_INFO "process performed an (mm) action that can not be "
> SH> +			"checkpointed at: %s:%d\n", file, line);
> SH> +}
> SH> +#define mm_deny_checkpointing(f) \
> SH> +	__mm_deny_checkpointing(f, __FILE__, __LINE__)
> 
> There is no definition of mm_deny_checkpointing() outside of
> CONFIG_CHECKPOINT_RESTART, which means that you get a build error
> because mm->may_checkpoint is only present when CR is enabled.
> 
> SH> -	WARN_ON(1);
> SH> +	//WARN_ON(1);
> 
> I assume commenting this out wasn't intended to be in this patch.

Yeah that's why I said I would clean it up before sending to
lkml :)

I was especially curious whether using vm_stat_account to catch the
offending mmaps seemed sensible.

thanks,
-serge
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers