[PATCH 2/3] Quiesce all threads of a process

[Jim Keniston]

On Thu, 2009-07-30 at 15:26 +0530, Ananth N Mavinakayanahalli wrote:

> +static u32 core_clone(enum utrace_resume_action action,
...
> +		/*
> +		 * -EINPROGRESS => thread on the way to quiesce
> +		 * -EALREADY => we may be racing with attach_utrace_engines
> +		 *  and it may already have attached an engine for us.
> +		 */
> +		if (ret && ((ret != -EINPROGRESS) || (ret != -EALREADY))) {

((ret != -EINPROGRESS) || (ret != -EALREADY)) ???
Assuming EINPROGRESS != EALREADY, this test is always true.  I think you
want && here, in which case you can dispense with most of the parens.

...
> +/*
> + * Process exec()ing. Abort and exit from here
> + *
> + * XXX:
> + * a. Is this necessary?

I think it's a good idea to handle the exec case.  But since dumping
core for an exec-ing process is a rather bizarre thing to attempt, you
can probably handle it however is most convenient for you.

> + * b. Are we safe here wrt the core dump being in progress?

Assuming all threads are quiesced before you start your actual core
dump, there's no way one of them can exec while you're dumping.

> + */
> +static u32 core_exec(enum utrace_resume_action action,
> +		struct utrace_engine *engine, struct task_struct *task,
> +		const struct linux_binfmt *fmt, const struct linux_binprm *bprm,
> +		struct pt_regs *regs)
> +...
> +
> +static struct core_proc *attach_utrace_engines(struct task_struct *task)
> +{
...
> +		/*
> +		 * -EINPROGRESS => we are on our way to quiesce.
> +		 * -EEXIST => we may be racing with core_clone and it has
> +		 *  already attached an engine for us.
> +		 */
> +		if (ret && ((ret != EINPROGRESS) || (ret != -EALREADY)))

s/||/&&/ -- as above.

Jim