[CRIU] [PATCH 6/7] read mode pre-dump implementation
Dmitry Safonov
0x7f454c46 at gmail.com
Thu Aug 29 14:20:14 MSK 2019
On 8/29/19 5:08 AM, abhishek dubey wrote:
> On 29/08/19 3:14 AM, Dmitry Safonov wrote:
[..]
>> It probably not what you want on arm64, ppc64 where page_size could be
>> different from 4Kb with Large Pages. Furthermore, you use it only in one
>> function as a temporary buffer - could we move it from .data section
>> into dynamic allocation with malloc()?
>
> is this a correct replacement? architecture independent one?
>
> char* userbuf = malloc(PIPE_MAX_SIZE * PAGE_SIZE);
I believe so. You may want to use criu's xmalloc() which will also print
an error on fail [not to print it yourself].
[..]
>>> +
>>> + /* Handling Case 3-Part 3.2*/
>>> + offset = (partial_read_bytes)? partial_read_bytes : PAGE_SIZE;
>>> +
>>> + dummy.iov_base = riov[faulty_index].iov_base + offset;
>>> + dummy.iov_len = riov[faulty_index].iov_len - offset;
>>> +
>>> + if (!partial_read_bytes)
>>> + cnt_sub(CNT_PAGES_WRITTEN, 1);
>>> +
>>> + while (dummy.iov_len) {
>>> +
>>> + bytes_read = process_vm_readv(pid, bufvec, 1, &dummy, 1, 0);
>> Could you check errno please?
>
> In the very first patch I had that check. In the review it was not found
> very useful, so omitted that.
Heh, wasn't aware of that.
Correct me if I'm wrong, but you expect that it may fail with EFAULT.
But other errors look to be unexpected here?
At least those three from `man 2 process_vm_readv`:
> ENOMEM Could not allocate memory for internal copies of the iovec
> structures.
> EPERM The caller does not have permission to access the address
> space ofthe process pid.
> ESRCH No process with ID pid exists.
>
> @Andrei @Xemul : how to go about it?
>
>>
[..]
>>> + ret = vmsplice(ppb->p[1], &bufvec, 1, SPLICE_F_NONBLOCK);
>>> +
>>> + if (ret == -1 || ret != bytes_read) {
>>> + pr_err("vmsplice: Failed to splice user buffer to pipe
>>> %ld\n", ret);
>> Probably you want pr_error()
>
> Yes, maybe. Other splicing failure also had pr_error().
>
> What is difference between two?
Sorry, misinformed you - the correct name is pr_perror().
It will print decoding for errno:
: #define pr_perror(fmt, ...) \
: pr_err(fmt ": %s\n", ##__VA_ARGS__, strerror(errno))
Thanks,
Dmitry
More information about the CRIU
mailing list