Re: [PATCH v2] mm/madvise: zap markers from guard regions

["David Hildenbrand (Arm)" <david@kernel.org>]

>>>  mm/madvise.c | 28 +++++++++++++++++++---------
>>>  1 file changed, 19 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/mm/madvise.c b/mm/madvise.c
>>> index 0214efd2a014..4f7728583207 100644
>>> --- a/mm/madvise.c
>>> +++ b/mm/madvise.c
>>> @@ -850,6 +850,19 @@ static int madvise_free_single_vma(struct madvise_behavior *madv_behavior)
>>>  	return 0;
>>>  }
>>>
>>> +static void zap_range(struct madvise_behavior *madv_behavior,
>>> +		      zap_flags_t flags)
>>
>> Two ... ;) But maybe it's consistent like that in the file, didn't check.
> 
> I'm a bit exhausted, can you or Andrew just fix up whitespace like this on
> application?
> 
> It'll be easier when I have the VMA tree.

Yeah, absolutely. I'll let Andrew pick this one.

> 
>>
>>> +{
>>> +	const struct madvise_behavior_range *range = &madv_behavior->range;
>>> +	struct zap_details details = {
>>> +		.reclaim_pt = true,
>>> +		.zap_flags = flags,
>>> +	};
>>> +
>>> +	zap_vma_range_batched(madv_behavior->tlb, madv_behavior->vma,
>>> +			      range->start, range->end - range->start, &details);
>>> +}
>>> +
>>>  /*
>>>   * Application no longer needs these pages.  If the pages are dirty,
>>>   * it's OK to just throw them away.  The app will be more careful about
>>> @@ -872,13 +885,7 @@ static int madvise_free_single_vma(struct madvise_behavior *madv_behavior)
>>>  static long madvise_dontneed_single_vma(struct madvise_behavior *madv_behavior)
>>>
>>>  {
>>> -	struct madvise_behavior_range *range = &madv_behavior->range;
>>> -	struct zap_details details = {
>>> -		.reclaim_pt = true,
>>> -	};
>>> -
>>> -	zap_vma_range_batched(madv_behavior->tlb, madv_behavior->vma,
>>> -			      range->start, range->end - range->start, &details);
>>> +	zap_range(madv_behavior, 0);
>>>  	return 0;
>>>  }
>>>
>>> @@ -1217,9 +1224,11 @@ static long madvise_guard_install(struct madvise_behavior *madv_behavior)
>>>
>>>  		/*
>>>  		 * OK some of the range have non-guard pages mapped, zap
>>> -		 * them. This leaves existing guard pages in place.
>>> +		 * them. This also drops any PTE markers in the range,
>>> +		 * including existing guard markers, which we reinstall on
>>> +		 * the next attempt.
>>>  		 */
>>> -		zap_vma_range(vma, range->start, range->end - range->start);
>>> +		zap_range(madv_behavior, ZAP_FLAG_DROP_MARKER);
>>>  	}
>>>
>>>  	/*
>>> @@ -1845,6 +1854,7 @@ static bool madvise_batch_tlb_flush(int behavior)
>>>  	case MADV_DONTNEED:
>>>  	case MADV_DONTNEED_LOCKED:
>>>  	case MADV_FREE:
>>> +	case MADV_GUARD_INSTALL:
>>
>> Huh why that change? Not spelled out above.
> 
> It's just the established way madvise interacts with TLB maintenance.
> 
> Anything that uses madv_behaviour->tlb has to return true here so
> madvise_init_tlb() and madvise_finish_tlb() do the right thing.

Ohhh, I see now.

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David