[5/6] Hack to handle vmsplice fail: resolve later

Submitted by Abhishek Dubey on July 25, 2019, 1:14 a.m.

Details

Message ID 1564017256-8143-1-git-send-email-dubeyabhishek777@gmail.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Abhishek Dubey July 25, 2019, 1:14 a.m.
hack to handle failing vmsplice from user-buffer to
pipe: need to resolve

Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
---
 criu/page-pipe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/page-pipe.c b/criu/page-pipe.c
index a821696..d73f223 100644
--- a/criu/page-pipe.c
+++ b/criu/page-pipe.c
@@ -33,7 +33,7 @@  static int __ppb_resize_pipe(struct page_pipe_buf *ppb, unsigned long new_size)
 {
 	int ret;
 
-	ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE);
+	ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE + 1);
 	if (ret < 0)
 		return -1;
 
@@ -41,7 +41,7 @@  static int __ppb_resize_pipe(struct page_pipe_buf *ppb, unsigned long new_size)
 	BUG_ON(ret < ppb->pipe_size);
 
 	pr_debug("Grow pipe %x -> %x\n", ppb->pipe_size, ret);
-	ppb->pipe_size = ret;
+	ppb->pipe_size = ret -1;
 
 	return 0;
 }

Comments

Pavel Emelianov July 30, 2019, 12:41 p.m.
On 7/25/19 4:14 AM, Abhishek Dubey wrote:
> hack to handle failing vmsplice from user-buffer to
> pipe: need to resolve

Please, write more descriptive comment to this patch, these off-by-one-s
are not clear.

> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
> ---
>  criu/page-pipe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/criu/page-pipe.c b/criu/page-pipe.c
> index a821696..d73f223 100644
> --- a/criu/page-pipe.c
> +++ b/criu/page-pipe.c
> @@ -33,7 +33,7 @@ static int __ppb_resize_pipe(struct page_pipe_buf *ppb, unsigned long new_size)
>  {
>  	int ret;
>  
> -	ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE);
> +	ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE + 1);
>  	if (ret < 0)
>  		return -1;
>  
> @@ -41,7 +41,7 @@ static int __ppb_resize_pipe(struct page_pipe_buf *ppb, unsigned long new_size)
>  	BUG_ON(ret < ppb->pipe_size);
>  
>  	pr_debug("Grow pipe %x -> %x\n", ppb->pipe_size, ret);
> -	ppb->pipe_size = ret;
> +	ppb->pipe_size = ret -1;
>  
>  	return 0;
>  }
>
Abhishek Dubey Aug. 2, 2019, 6:15 a.m.
On 30/07/19 6:11 PM, Pavel Emelianov wrote:
> On 7/25/19 4:14 AM, Abhishek Dubey wrote:
>> hack to handle failing vmsplice from user-buffer to
>> pipe: need to resolve
> Please, write more descriptive comment to this patch, these off-by-one-s
> are not clear.

This is temporary change. I need to debug this code and eliminate it.

If this will be required in final code, I will add proper description.

>
>> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
>> ---
>>   criu/page-pipe.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/criu/page-pipe.c b/criu/page-pipe.c
>> index a821696..d73f223 100644
>> --- a/criu/page-pipe.c
>> +++ b/criu/page-pipe.c
>> @@ -33,7 +33,7 @@ static int __ppb_resize_pipe(struct page_pipe_buf *ppb, unsigned long new_size)
>>   {
>>   	int ret;
>>   
>> -	ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE);
>> +	ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE + 1);
>>   	if (ret < 0)
>>   		return -1;
>>   
>> @@ -41,7 +41,7 @@ static int __ppb_resize_pipe(struct page_pipe_buf *ppb, unsigned long new_size)
>>   	BUG_ON(ret < ppb->pipe_size);
>>   
>>   	pr_debug("Grow pipe %x -> %x\n", ppb->pipe_size, ret);
>> -	ppb->pipe_size = ret;
>> +	ppb->pipe_size = ret -1;
>>   
>>   	return 0;
>>   }
>>
-Abhishek
Abhishek Dubey Aug. 16, 2019, 11:13 a.m.
Hi,

I have a few observation regarding this issue:

If a ppb (page-pipe-buffer) requires corresponding pipe size < 512 pages,
it faces no issue.
But, all the test-cases which require pipe size to be 512 pages fails. In
such cases, vmsplice
while splicing from user-buffer, don't return any error, but splices few
bytes less than 512 pages.
So 512th page is incompletely spliced from user-buffer to pipe.

This hack bypasses this issue:
While expanding pipe, let it expand till 512 pages, but the stored pipe
size will be 1 less than
actual size. So, ppb->pipe_size will store 511 instead of 512. Each ppb can
point to 511 pages.

By doing so, vmsplice splices 511 pages completely, without issue for all
test cases. The 512th
page become part of next ppb.

I wrote a toy code to find the actual issue(vmsplice failing with pipe of
512 page size), but pipe was not expanding beyond 256 pages.

Have you faced this problem before?


On Fri, Aug 2, 2019 at 11:44 AM abhishek dubey <dubeyabhishek777@gmail.com>
wrote:

>
> On 30/07/19 6:11 PM, Pavel Emelianov wrote:
> > On 7/25/19 4:14 AM, Abhishek Dubey wrote:
> >> hack to handle failing vmsplice from user-buffer to
> >> pipe: need to resolve
> > Please, write more descriptive comment to this patch, these off-by-one-s
> > are not clear.
>
> This is temporary change. I need to debug this code and eliminate it.
>
> If this will be required in final code, I will add proper description.
>
> >
> >> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
> >> ---
> >>   criu/page-pipe.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/criu/page-pipe.c b/criu/page-pipe.c
> >> index a821696..d73f223 100644
> >> --- a/criu/page-pipe.c
> >> +++ b/criu/page-pipe.c
> >> @@ -33,7 +33,7 @@ static int __ppb_resize_pipe(struct page_pipe_buf
> *ppb, unsigned long new_size)
> >>   {
> >>      int ret;
> >>
> >> -    ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE);
> >> +    ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE + 1);
> >>      if (ret < 0)
> >>              return -1;
> >>
> >> @@ -41,7 +41,7 @@ static int __ppb_resize_pipe(struct page_pipe_buf
> *ppb, unsigned long new_size)
> >>      BUG_ON(ret < ppb->pipe_size);
> >>
> >>      pr_debug("Grow pipe %x -> %x\n", ppb->pipe_size, ret);
> >> -    ppb->pipe_size = ret;
> >> +    ppb->pipe_size = ret -1;
> >>
> >>      return 0;
> >>   }
> >>
> -Abhishek
>
Abhishek Dubey Sept. 27, 2019, 9:48 a.m.
This is elaborate description of the issue which is bypassed by this patch.

On 16/08/19 4:43 PM, Abhishek Dubey wrote:
> Hi,
>
> I have a few observation regarding this issue:
>
> If a ppb (page-pipe-buffer) requires corresponding pipe size < 512 
> pages, it faces no issue.
> But, all the test-cases which require pipe size to be 512 pages fails. 
> In such cases, vmsplice
> while splicing from user-buffer, don't return any error, but splices 
> few bytes less than 512 pages.
> So 512th page is incompletely spliced from user-buffer to pipe.
>
> This hack bypasses this issue:
> While expanding pipe, let it expand till 512 pages, but the stored 
> pipe size will be 1 less than
> actual size. So, ppb->pipe_size will store 511 instead of 512. Each 
> ppb can point to 511 pages.
>
> By doing so, vmsplice splices 511 pages completely, without issue for 
> all test cases. The 512th
> page become part of next ppb.
>
> I wrote a toy code to find the actual issue(vmsplice failing with pipe 
> of 512 page size), but pipe was not expanding beyond 256 pages.
>
> Have you faced this problem before?
>
>
> On Fri, Aug 2, 2019 at 11:44 AM abhishek dubey 
> <dubeyabhishek777@gmail.com <mailto:dubeyabhishek777@gmail.com>> wrote:
>
>
>     On 30/07/19 6:11 PM, Pavel Emelianov wrote:
>     > On 7/25/19 4:14 AM, Abhishek Dubey wrote:
>     >> hack to handle failing vmsplice from user-buffer to
>     >> pipe: need to resolve
>     > Please, write more descriptive comment to this patch, these
>     off-by-one-s
>     > are not clear.
>
>     This is temporary change. I need to debug this code and eliminate it.
>
>     If this will be required in final code, I will add proper description.
>
>     >
>     >> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com
>     <mailto:dubeyabhishek777@gmail.com>>
>     >> ---
>     >>   criu/page-pipe.c | 4 ++--
>     >>   1 file changed, 2 insertions(+), 2 deletions(-)
>     >>
>     >> diff --git a/criu/page-pipe.c b/criu/page-pipe.c
>     >> index a821696..d73f223 100644
>     >> --- a/criu/page-pipe.c
>     >> +++ b/criu/page-pipe.c
>     >> @@ -33,7 +33,7 @@ static int __ppb_resize_pipe(struct
>     page_pipe_buf *ppb, unsigned long new_size)
>     >>   {
>     >>      int ret;
>     >>
>     >> -    ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE);
>     >> +    ret = fcntl(ppb->p[0], F_SETPIPE_SZ, new_size * PAGE_SIZE
>     + 1);
>     >>      if (ret < 0)
>     >>              return -1;
>     >>
>     >> @@ -41,7 +41,7 @@ static int __ppb_resize_pipe(struct
>     page_pipe_buf *ppb, unsigned long new_size)
>     >>      BUG_ON(ret < ppb->pipe_size);
>     >>
>     >>      pr_debug("Grow pipe %x -> %x\n", ppb->pipe_size, ret);
>     >> -    ppb->pipe_size = ret;
>     >> +    ppb->pipe_size = ret -1;
>     >>
>     >>      return 0;
>     >>   }
>     >>
>     -Abhishek
>