[CRIU] [PATCH 3/7] page-read: Drop get_remote_pages

[Pavel Emelyanov]

On 11/16/2016 01:42 PM, Mike Rapoport wrote:
> On Wed, Nov 16, 2016 at 11:39 AM, Pavel Emelyanov <xemul at virtuozzo.com> wrote:
>> We already have routines that do send-req, recv-info
>> and recv-page, so no need in yet another one.
>>
>> Signed-off-by: Pavel Emelyanov <xemul at virtuozzo.com>
>> ---
>>  criu/include/page-xfer.h |  3 ---
>>  criu/page-xfer.c         | 34 ++++------------------------------
>>  criu/pagemap.c           | 21 ++++++++++++---------
>>  3 files changed, 16 insertions(+), 42 deletions(-)
>>
>> diff --git a/criu/include/page-xfer.h b/criu/include/page-xfer.h
>> index 7cd0055..5b7ec21 100644
>> --- a/criu/include/page-xfer.h
>> +++ b/criu/include/page-xfer.h
>> @@ -52,9 +52,6 @@ extern int check_parent_page_xfer(int fd_type, long id);
>>   * - dump-side page server sends the raw page data
>>   */
>>
>> -/* sync receive of remote pages */
>> -extern int get_remote_pages(int pid, unsigned long addr, int nr_pages, void *dest);
>> -
>>  /* async request/receive of remote pages */
>>  extern int request_remote_pages(int pid, unsigned long addr, int nr_pages);
>>  extern int receive_remote_pages_info(int *nr_pages, unsigned long *addr, int *pid);
>> diff --git a/criu/page-xfer.c b/criu/page-xfer.c
>> index 7f1d0f7..2d878e2 100644
>> --- a/criu/page-xfer.c
>> +++ b/criu/page-xfer.c
>> @@ -887,36 +887,6 @@ out:
>>         return ret ? : status;
>>  }
>>
>> -int get_remote_pages(int pid, unsigned long addr, int nr_pages, void *dest)
>> -{
>> -       int ret;
>> -       int len = PAGE_SIZE * nr_pages;
>> -
>> -       struct page_server_iov pi;
>> -
>> -       if (send_psi(page_server_sk, PS_IOV_GET, nr_pages, addr, pid))
>> -               return -1;
>> -
>> -       tcp_nodelay(page_server_sk, true);
>> -
>> -       ret = recv(page_server_sk, &pi, sizeof(pi), MSG_WAITALL);
>> -       if (ret != sizeof(pi))
>> -               return -1;
>> -
>> -       /* zero page */
>> -       if (pi.cmd == PS_IOV_ZERO)
>> -               return 0;
>> -
>> -       if (pi.nr_pages > nr_pages)
>> -               return -1;
>> -
>> -       ret = recv(page_server_sk, dest, len, MSG_WAITALL);
>> -       if (ret != len)
>> -               return -1;
>> -
>> -       return 1;
>> -}
>> -
>>  int request_remote_pages(int pid, unsigned long addr, int nr_pages)
>>  {
>>         struct page_server_iov pi = {
>> @@ -945,6 +915,10 @@ int receive_remote_pages_info(int *nr_pages, unsigned long *addr, int *pid)
>>                 return -1;
>>         }
>>
>> +       if (pi.cmd == PS_IOV_ZERO)
>> +               pr_warn("Unexpected ZERO page received for %d.%lx\n",
>> +                               (int)pi.dst_id, (unsigned long)pi.vaddr);
>> +
>>         *nr_pages = pi.nr_pages;
>>         *addr = pi.vaddr;
>>         *pid = pi.dst_id;
>> diff --git a/criu/pagemap.c b/criu/pagemap.c
>> index e05a468..86791d7 100644
>> --- a/criu/pagemap.c
>> +++ b/criu/pagemap.c
>> @@ -397,16 +397,19 @@ static int maybe_read_page_local(struct page_read *pr, unsigned long vaddr,
>>  static int maybe_read_page_remote(struct page_read *pr, unsigned long vaddr,
>>                 int nr, void *buf, unsigned flags)
>>  {
>> -       int ret;
>> +       int ret, pid;
>>
>> -       if (flags & PR_ASYNC)
>> -               /*
>> -                * Note, that for async remote page_read, the actual
>> -                * transfer happens in the lazy-pages daemon
>> -                */
>> -               ret = request_remote_pages(pr->pid, vaddr, nr);
>> -       else
>> -               ret = get_remote_pages(pr->pid, vaddr, nr, buf);
>> +       ret = request_remote_pages(pr->pid, vaddr, nr);
>> +       if ((ret < 0) || (flags & PR_ASYNC))
>> +               return ret;
> 
> request_remote_pages uses send(..., MSG_DONTWAIT)
> Are you sure that for synchronous case we won't get some weird effects here?

Well, "weird effects" no :) In the worst case we'll receive the exit
code that != sizeof(something) and will abort the operation.

But the same would anyway be a problem even for synchronous case, so
this particular MSG_DONTWAIT seem to be incorrect by itself.

-- Pavel