Re: [RFC 1/2] arm: cacheflush syscall: process only pages that are in the memory

[Marek Szyprowski]

Hi Russell,

On 2018-01-26 12:32, Russell King - ARM Linux wrote:
> On Fri, Jan 26, 2018 at 12:14:40PM +0100, Marek Szyprowski wrote:
> > glibc in calls cacheflush syscall on the whole textrels section of the
> > relocated binaries. However, relocation usually doesn't touch all pages
> > of that section, so not all of them are read to memory when calling this
> > syscall. However flush_cache_user_range() function will unconditionally
> > touch all pages from the provided range, resulting additional overhead
> > related to reading all clean pages. Optimize this by calling
> > flush_cache_user_range() only on the pages that are already in the
> > memory.
> What ensures that another CPU doesn't remove a page while we're
> flushing it?  That will trigger a data abort, which will want to
> take the mmap_sem, causing a deadlock.

I thought that taking mmap_sem will prevent pages from being removed.
mmap_sem has been already taken in the previous implementation of that
syscall, until code simplification done by commit 97c72d89ce0e ("ARM:
cacheflush: don't bother rounding to nearest vma").

> > Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> > ---
> >   arch/arm/kernel/traps.c | 25 +++++++++++++++++++------
> >   1 file changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> > index 5e3633c24e63..a5ec262ab30e 100644
> > --- a/arch/arm/kernel/traps.c
> > +++ b/arch/arm/kernel/traps.c
> > @@ -564,23 +564,36 @@ static int bad_syscall(int n, struct pt_regs *regs)
> >   static inline int
> >   __do_cache_op(unsigned long start, unsigned long end)
> >   {
> > -	int ret;
> > +	struct vm_area_struct *vma = NULL;
> > +	int ret = 0;
> >   
> > +	down_read(&current->mm->mmap_sem);
> >   	do {
> >   		unsigned long chunk = min(PAGE_SIZE, end - start);
> >   
> > +		if (!vma || vma->vm_end <= start) {
> > +			vma = find_vma(current->mm, start);
> > +			if (!vma) {
> > +				ret = -EFAULT;
> > +				goto done;
> > +			}
> > +		}
> > +
> >   		if (fatal_signal_pending(current))
> >   			return 0;
> >   
> > -		ret = flush_cache_user_range(start, start + chunk);
> > -		if (ret)
> > -			return ret;
> > +		if (follow_page(vma, start, 0)) {
> > +			ret = flush_cache_user_range(start, start + chunk);
> > +			if (ret)
> > +				goto done;
> > +		}
> >   
> >   		cond_resched();
> >   		start += chunk;
> >   	} while (start < end);
> > -
> > -	return 0;
> > +done:
> > +	up_read(&current->mm->mmap_sem);
> > +	return ret;
> >   }
> >   
> >   static inline int
> > --
> > 2.15.0

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland