[CRIU] [PATCH 01/12] pstree: Bind CoreEntry to pstree and fill it with registers early

[Cyrill Gorcunov]

On Fri, Mar 01, 2013 at 10:44:27PM +0400, Pavel Emelyanov wrote:
> > -	if (ctl)
> > -		regs = ctl->regs_orig;
> > -	else {
> > -		if (ptrace(PTRACE_GETREGS, pid, NULL, &regs)) {
> > -			pr_err("Can't obtain GP registers for %d\n", pid);
> > -			goto err;
> > -		}
> > +	if (ptrace(PTRACE_GETREGS, pid, NULL, &regs)) {
> > +		pr_err("Can't obtain GP registers for %d\n", pid);
> > +		goto err;
> >  	}
> 
> So, do we still have regs duplication?

We simply _can't_ live without regs duplication for thread leader.
parasite_ctl must have own copy in cpu _native_ format, while
CoreEntry'ies carry registers converted to protobuf format.

I though about to use CoreEntry instead but this implies adding
a new routines which would convert pb registers to native format,
which eventually only complicates the code. That's why I left it
in this way.

> > +static int collect_regs_seized(struct pstree_item *item)
> > +{
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	if (pstree_alloc_cores(item))
> > +		return -1;
> > +
> > +	for (i = 0; i < item->nr_threads; i++) {
> > +		pid_t pid = item->threads[i].real;
> > +		ret = get_task_regs(pid, item->core[i]);
> > +		if (ret) {
> > +			pr_err("Can't obtain regs for thread %d\n", pid);
> > +			return -1;
> > +		}
> > +
> > +		if (item->threads[i].real == item->pid.real)
> > +			item->this_core = item->core[i];
> 
> We've already done this in pstree_alloc_cores()

Indeed, this is harmless leftover, thanks!

> > +	}
> > +
> > +	return 0;
> > +}
> 
> > +int pstree_alloc_cores(struct pstree_item *item)
> > +{
> > +	unsigned int i;
> > +
> > +	item->core = xzalloc(sizeof(*item->core) * item->nr_threads);
> > +	if (!item->core)
> > +		return -1;
> > +
> > +	for (i = 0; i < item->nr_threads; i++) {
> > +		int is_leader = (item->threads[i].real == item->pid.real);
> > +
> > +		item->core[i] = core_entry_alloc(item->state != TASK_DEAD, is_leader);
> > +		if (!item->core[i])
> > +			goto err;
> > +
> > +		if (is_leader)
> > +			item->this_core = item->core[i];
> 
> The version in collect_regs_seized() without is_leader looks nicer.

I wanted to distinguish pstree functions with anything else, ie we
have pstree_alloc_cores/pstree_free_cores pair which doesn't care
about the data present in CoreEntry but only serves allocation/frees
of CoreEntry, and collect_regs_seized which fills core entries with
registers.

In first versions of this patch collect_regs_seized didn't allocate
Cores but you don't like it as far as i remember, that's why I moved
pstree_alloc_cores into collect_regs_seized.

	Cyrill