[CRIU] [PATCH 0/4] Split get_task_regs routines

[Pavel Emelyanov]

On 10/07/2016 06:59 PM, Laurent Dufour wrote:
> On 06/10/2016 20:23, Pavel Emelyanov wrote:
>> On 10/06/2016 06:47 PM, Laurent Dufour wrote:
>>> On 26/09/2016 10:23, Pavel Emelyanov wrote:
>>>> Hi,
>>>>
>>>> We're trying to detach the parasite code injection functionality of CRIU
>>>> into a standalone library called compel. In order to do so we need to
>>>> (among other things) split getting task registers code from saving the
>>>> info into protobuf formats. The most intrusive part of this work is
>>>> splitting the get_task_regs() routine.
>>>>
>>>> So this set introduces the user_fpregs_struct_t for all the arches and
>>>> splits the get_task_regs() into two -- first part is getting user_regs_t 
>>>> and (!) the new user_fpregs_t structures, and the second is saving this
>>>> info into the CoreEntry.
>>>>
>>>> I've tested the x86 part and compile-tested the ppc64 and arm/aarch64.
>>>> So Laurent and Christopher, would you please help us and review patches 
>>>> 1 and 2?
>>>
>>> Hi Pavel,
>>>
>>> Sorry for the very late answer, but I had some hot stuff to fix.
>>>
>>> Quick build and test showed a major regression, all tests are now
>>> failing on ppc64le.
>>
>> :( Sorry about that...
>>
>>> I'll investigate that deeper tomorrow.
>>
>> Yes, that'd be very much appreciated!
> 
> I think I'd have to rewrite the ppc64 your patch because it is not
> handling well the TM case.

:(

> But to do so, I'd like to be sure I understand correctly the final point.
> The idea is to have on one side the function that get and set the
> registers of a process. 

The idea is to split the get_task_regs() routine into two parts -- the first
part fills in the user_regs_struct_t and the new user_fpregs_struct_t and
the 2nd part then copies these two variables into CoreEntry.

Opposite to this, right now the ppc's get_task_regs() fills the CoreEntry
with data eventually, reading the registers set on demand with the ptrace().

Once this split is done, I will be able to have two separate routines -- the
get_task_regs() that gets the registers into user_(fp)?regs_struct_t-s and
the save_task_regs() that is called by the former and that copies the contents
of the foo_struct_t-s into the CoreEntry.

And having _this_ done, I will be able to make get_task_regs() forget about
CoreEntry, by passing to it two pointers:

- pointer to the function that is to be called instead of save_task_regs()
- void * pointer that will be meaningful only to the former callback.

> This service rely on architecture specific type
> but will expose 2 global one user_regs_struct_t and user_fpregs_struct_t.
> 
> The user_regs_struct_t will remain manage by the common
> criu/parasite-syscall.c file.
> 
> Since we need to write back the registers to the process once the
> parasite injection is made, I guess we need a per architecture exported
> function which put back the content of user_fpregs_struct_t.

Yes, that's the 2nd half of the story, but it's slightly simpler that the
first one, as the construct_sigframe() already does this and I don't need
to split this routine into two :)

> But today we rely on the calls made by construct_sigframe to create the
> jumping back signal frame, and this one is based on the protobuf data.
> I guess at one point we should tend to the following flow:
> 
> Checkpoint:
> 1. get user_regs_struct_t
> 2. get user_fpregs_struct_t
> 3. write user_regs_struct_t in proto format
> 4. write user_fpregs_struct_t in proto format
> 5. dump proto data to disk
> 6. build signal frame using user_regs_struct_t and user_fpregs_struct_t
> 7. release process
> 
> Restore:
> 1. read proto data
> 2. build user_regs_struct_t from proto data
> 3. build user_fpregs_struct_t from proto data
> 4. build signal frame using user_regs_struct_t and user_fpregs_struct_t
> 5. release process
> 
> So construct_sigframe() will change to no more manipulate proto data,
> isn't it ?

No, the construct_sigframe() will keep messing with the CoreEntry. My
goal it to make get_task_regs() manipulate only the user_(fp)?regs_struct_t-s
and call the callbacks.

-- Pavel