Re: [RFC 1/4] mm: Introduce lazy exec permission setting on a page

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 02/14/2019 02:36 PM, Mike Rapoport wrote:
> On Wed, Feb 13, 2019 at 07:23:18PM +0530, Anshuman Khandual wrote:
>>
>>
>> On 02/13/2019 06:47 PM, Matthew Wilcox wrote:
>>> On Wed, Feb 13, 2019 at 01:36:28PM +0530, Anshuman Khandual wrote:
>>>> +#ifdef CONFIG_ARCH_SUPPORTS_LAZY_EXEC
>>>> +static inline pte_t maybe_mkexec(pte_t entry, struct vm_area_struct *vma)
>>>> +{
>>>> +	if (unlikely(vma->vm_flags & VM_EXEC))
>>>> +		return pte_mkexec(entry);
>>>> +	return entry;
>>>> +}
>>>> +#else
>>>> +static inline pte_t maybe_mkexec(pte_t entry, struct vm_area_struct *vma)
>>>> +{
>>>> +	return entry;
>>>> +}
>>>> +#endif
>>>
>>>> +++ b/mm/memory.c
>>>> @@ -2218,6 +2218,8 @@ static inline void wp_page_reuse(struct vm_fault *vmf)
>>>>  	flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
>>>>  	entry = pte_mkyoung(vmf->orig_pte);
>>>>  	entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>>>> +	if (vmf->flags & FAULT_FLAG_INSTRUCTION)
>>>> +		entry = maybe_mkexec(entry, vma);
>>>
>>> I don't understand this bit.  We have a fault based on an instruction
>>> fetch.  But we're only going to _maybe_ set the exec bit?  Why not call
>>> pte_mkexec() unconditionally?
>>
>> Because the arch might not have subscribed to this in which case the fall
>> back function does nothing and return the same entry. But in case this is
>> enabled it also checks for VMA exec flag (VM_EXEC) before calling into
>> pte_mkexec() something similar to existing maybe_mkwrite().
> 
> Than why not pass vmf->flags to maybe_mkexec() so that only arches
> subscribed to this will have the check for 'flags & FAULT_FLAG_INSTRUCTION' ?

Right it can help remove couple of instructions from un-subscribing archs.