[CRIU] [PATCH 3/7] Skip adding PROT_READ to non-PROT_READ mappings

[abhishek dubey]

On 28/08/19 10:26 PM, Dmitry Safonov wrote:
> On 8/25/19 2:58 AM, Abhishek Dubey wrote:
> [..]
>> -	pargs->add_prot = PROT_READ;
>> -	ret = compel_rpc_call_sync(PARASITE_CMD_MPROTECT_VMAS, ctl);
>> -	if (ret) {
>> -		pr_err("Can't dump unprotect vmas with parasite\n");
>> -		return ret;
>> -	}
>> +	if (!mdc->pre_dump || opts.pre_dump_mode == PRE_DUMP_SPLICE) {
>> +		pargs->add_prot = PROT_READ;
>> +		ret = compel_rpc_call_sync(PARASITE_CMD_MPROTECT_VMAS, ctl);
>> +		if (ret) {
>> +			pr_err("Can't dump unprotect vmas with parasite\n");
>> +			return ret;
>> +		}
> Nit: could we separate it into:
> static int parasite_mprotect_vmas(struct parasite_ctl *ctl, int prot)
> {
>       struct parasite_dump_pages_args *pargs;
>       int ret;
>
>       ret = compel_rpc_call_sync(PARASITE_CMD_MPROTECT_VMAS, ctl);
>       /*  ... */
> }
>
>>   
>> -	if (fault_injected(FI_DUMP_PAGES)) {
>> -		pr_err("fault: Dump VMA pages failure!\n");
>> -		return -1;
>> +		if (fault_injected(FI_DUMP_PAGES)) {
>> +			pr_err("fault: Dump VMA pages failure!\n");
>> +			return -1;
>> +		}
> Hmm, I might be mistaken, but it may make sense to leave it outside "if"?
Yes, this is logically correct and can be done!
>
>>   	}
>>   
>>   	ret = __parasite_dump_pages_seized(item, pargs, vma_area_list, mdc, ctl);
>> @@ -574,10 +607,12 @@ int parasite_dump_pages_seized(struct pstree_item *item,
>>   		return ret;
>>   	}
>>   
>> -	pargs->add_prot = 0;
>> -	if (compel_rpc_call_sync(PARASITE_CMD_MPROTECT_VMAS, ctl)) {
>> -		pr_err("Can't rollback unprotected vmas with parasite\n");
>> -		ret = -1;
>> +	if (!mdc->pre_dump || opts.pre_dump_mode == PRE_DUMP_SPLICE) {
>> +		pargs->add_prot = 0;
>> +		if (compel_rpc_call_sync(PARASITE_CMD_MPROTECT_VMAS, ctl)) {
>> +			pr_err("Can't rollback unprotected vmas with parasite\n");
>> +			ret = -1;
>> +		}
> And reuse it here?
>
> Thanks,
>            Dmitry
-Abhishek