[2/7] Skip generating iov for non PROT_READ region

Submitted by abhishek dubey on Aug. 20, 2019, 11:06 p.m.

Details

Message ID 1566342422-18459-3-git-send-email-dubeyabhishek777@gmail.com
State New
Series "GSoC 19: Optimizing the Pre-dump Algorithm"
Headers show

Commit Message

abhishek dubey Aug. 20, 2019, 11:06 p.m.
skip iov-generation corresponding to regions
not having PROT_READ, since process_vm_readv
syscall can't process them during pre-dumping
in "read" mode.

Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
---
 criu/mem.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/mem.c b/criu/mem.c
index e19688d..740992d 100644
--- a/criu/mem.c
+++ b/criu/mem.c
@@ -351,7 +351,7 @@  static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
 			     struct page_pipe *pp, struct page_xfer *xfer,
 			     struct parasite_dump_pages_args *args,
 			     struct parasite_ctl *ctl, pmc_t *pmc,
-			     bool has_parent, bool pre_dump)
+			     bool has_parent, struct mem_dump_ctl *mdc)
 {
 	u64 off = 0;
 	u64 *map;
@@ -361,8 +361,15 @@  static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
 				!vma_area_is(vma, VMA_ANON_SHARED))
 		return 0;
 
+	if (!(vma->e->prot & PROT_READ)) {
+		if (mdc->pre_dump == PRE_DUMP_READ)
+			return 0;
+		if (!mdc->pre_dump)
+			has_parent = false;
+	}
+
 	if (vma_entry_is(vma->e, VMA_AREA_AIORING)) {
-		if (pre_dump)
+		if (mdc->pre_dump)
 			return 0;
 		has_parent = false;
 	}
@@ -474,7 +481,7 @@  static int __parasite_dump_pages_seized(struct pstree_item *item,
 	has_parent = !!xfer.parent && !possible_pid_reuse;
 	list_for_each_entry(vma_area, &vma_area_list->h, list) {
 		ret = generate_vma_iovs(item, vma_area, pp, &xfer, args, ctl,
-					&pmc, has_parent, mdc->pre_dump);
+					&pmc, has_parent, mdc);
 		if (ret < 0)
 			goto out_xfer;
 	}

Comments

Andrei Vagin Aug. 22, 2019, 3:06 p.m.
On Wed, Aug 21, 2019 at 04:36:57AM +0530, Abhishek Dubey wrote:
> skip iov-generation corresponding to regions
> not having PROT_READ, since process_vm_readv
> syscall can't process them during pre-dumping
> in "read" mode.
> 
> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
> ---
>  criu/mem.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/criu/mem.c b/criu/mem.c
> index e19688d..740992d 100644
> --- a/criu/mem.c
> +++ b/criu/mem.c
> @@ -351,7 +351,7 @@ static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
>  			     struct page_pipe *pp, struct page_xfer *xfer,
>  			     struct parasite_dump_pages_args *args,
>  			     struct parasite_ctl *ctl, pmc_t *pmc,
> -			     bool has_parent, bool pre_dump)
> +			     bool has_parent, struct mem_dump_ctl *mdc)
>  {
>  	u64 off = 0;
>  	u64 *map;
> @@ -361,8 +361,15 @@ static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
>  				!vma_area_is(vma, VMA_ANON_SHARED))
>  		return 0;
>  
> +	if (!(vma->e->prot & PROT_READ)) {
> +		if (mdc->pre_dump == PRE_DUMP_READ)
> +			return 0;
> +		if (!mdc->pre_dump)
> +			has_parent = false;

What does this mean?

> +	}
> +
>  	if (vma_entry_is(vma->e, VMA_AREA_AIORING)) {
> -		if (pre_dump)
> +		if (mdc->pre_dump)
>  			return 0;
>  		has_parent = false;
>  	}
> @@ -474,7 +481,7 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
>  	has_parent = !!xfer.parent && !possible_pid_reuse;
>  	list_for_each_entry(vma_area, &vma_area_list->h, list) {
>  		ret = generate_vma_iovs(item, vma_area, pp, &xfer, args, ctl,
> -					&pmc, has_parent, mdc->pre_dump);
> +					&pmc, has_parent, mdc);
>  		if (ret < 0)
>  			goto out_xfer;
>  	}
> -- 
> 2.7.4
>
abhishek dubey Aug. 22, 2019, 9:20 p.m.
On 22/08/19 8:36 PM, Andrei Vagin wrote:
> On Wed, Aug 21, 2019 at 04:36:57AM +0530, Abhishek Dubey wrote:
>> skip iov-generation corresponding to regions
>> not having PROT_READ, since process_vm_readv
>> syscall can't process them during pre-dumping
>> in "read" mode.
>>
>> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
>> ---
>>   criu/mem.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/criu/mem.c b/criu/mem.c
>> index e19688d..740992d 100644
>> --- a/criu/mem.c
>> +++ b/criu/mem.c
>> @@ -351,7 +351,7 @@ static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
>>   			     struct page_pipe *pp, struct page_xfer *xfer,
>>   			     struct parasite_dump_pages_args *args,
>>   			     struct parasite_ctl *ctl, pmc_t *pmc,
>> -			     bool has_parent, bool pre_dump)
>> +			     bool has_parent, struct mem_dump_ctl *mdc)
>>   {
>>   	u64 off = 0;
>>   	u64 *map;
>> @@ -361,8 +361,15 @@ static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
>>   				!vma_area_is(vma, VMA_ANON_SHARED))
>>   		return 0;
>>   
>> +	if (!(vma->e->prot & PROT_READ)) {
>> +		if (mdc->pre_dump == PRE_DUMP_READ)
>> +			return 0;

For "read" mode pre-dump, iov for non-PROT_READ regions must not be 
formed. If they are formed, they will fail at process_vm_readv calls 
with permission error.

This is optimization to reduce the count of failing syscalls.

>> +		if (!mdc->pre_dump)
>> +			has_parent = false;
Subsequent dump stage have to dump non-PROT_READ regions(as they are not 
dumped in any "read" mode pre-dumps). So, mark "has_parent=false" for 
such regions at dump time.
> What does this mean?

I found one error in my code, I was setting "has_parent=false" for both 
modes of pre-dump. Given below is correct 
implementation("has_parent=false" just for "read" pre-dump)

I think this is correct version:

if (!(vma->e->prot & PROT_READ)) {
	if (opts.pre_dump_mode == PRE_DUMP_READ)
		return 0;
	if (!mdc->pre_dump && opts.pre_dump_mode == PRE_DUMP_READ)
		has_parent = false;

>
>> +	}
>> +
>>   	if (vma_entry_is(vma->e, VMA_AREA_AIORING)) {
>> -		if (pre_dump)
>> +		if (mdc->pre_dump)
>>   			return 0;
>>   		has_parent = false;
>>   	}
>> @@ -474,7 +481,7 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
>>   	has_parent = !!xfer.parent && !possible_pid_reuse;
>>   	list_for_each_entry(vma_area, &vma_area_list->h, list) {
>>   		ret = generate_vma_iovs(item, vma_area, pp, &xfer, args, ctl,
>> -					&pmc, has_parent, mdc->pre_dump);
>> +					&pmc, has_parent, mdc);
>>   		if (ret < 0)
>>   			goto out_xfer;
>>   	}
>> -- 
>> 2.7.4
>>
-Abhishek
abhishek dubey Aug. 23, 2019, 6:26 p.m.
Hi Andrei,

This is clean version for adding --pre-dump-mode option.

Please review it, so that I can send final patch series on mailing list.

https://github.com/dubeyabhishek/criu/commit/756410ae4313770a6f5260eb749da0b578f50e35

On 22/08/19 8:36 PM, Andrei Vagin wrote:
> On Wed, Aug 21, 2019 at 04:36:57AM +0530, Abhishek Dubey wrote:
>> skip iov-generation corresponding to regions
>> not having PROT_READ, since process_vm_readv
>> syscall can't process them during pre-dumping
>> in "read" mode.
>>
>> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
>> ---
>>   criu/mem.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/criu/mem.c b/criu/mem.c
>> index e19688d..740992d 100644
>> --- a/criu/mem.c
>> +++ b/criu/mem.c
>> @@ -351,7 +351,7 @@ static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
>>   			     struct page_pipe *pp, struct page_xfer *xfer,
>>   			     struct parasite_dump_pages_args *args,
>>   			     struct parasite_ctl *ctl, pmc_t *pmc,
>> -			     bool has_parent, bool pre_dump)
>> +			     bool has_parent, struct mem_dump_ctl *mdc)
>>   {
>>   	u64 off = 0;
>>   	u64 *map;
>> @@ -361,8 +361,15 @@ static int generate_vma_iovs(struct pstree_item *item, struct vma_area *vma,
>>   				!vma_area_is(vma, VMA_ANON_SHARED))
>>   		return 0;
>>   
>> +	if (!(vma->e->prot & PROT_READ)) {
>> +		if (mdc->pre_dump == PRE_DUMP_READ)
>> +			return 0;
>> +		if (!mdc->pre_dump)
>> +			has_parent = false;
> What does this mean?
>
>> +	}
>> +
>>   	if (vma_entry_is(vma->e, VMA_AREA_AIORING)) {
>> -		if (pre_dump)
>> +		if (mdc->pre_dump)
>>   			return 0;
>>   		has_parent = false;
>>   	}
>> @@ -474,7 +481,7 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
>>   	has_parent = !!xfer.parent && !possible_pid_reuse;
>>   	list_for_each_entry(vma_area, &vma_area_list->h, list) {
>>   		ret = generate_vma_iovs(item, vma_area, pp, &xfer, args, ctl,
>> -					&pmc, has_parent, mdc->pre_dump);
>> +					&pmc, has_parent, mdc);
>>   		if (ret < 0)
>>   			goto out_xfer;
>>   	}
>> -- 
>> 2.7.4
>>
-Abhishek