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

[Mike Rapoport <rppt@linux.ibm.com>]

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' ?

-- 
Sincerely yours,
Mike.