[PATCH V3 2/2] ARM: Handle user space mapped pages in flush_kernel_dcache_page

Russell King - ARM Linux linux at arm.linux.org.uk
Wed May 8 14:43:54 EDT 2013


On Wed, May 08, 2013 at 04:08:00PM +0100, Catalin Marinas wrote:
> On Sat, May 04, 2013 at 09:21:27AM +0100, Ming Lei wrote:
> > On Fri, May 3, 2013 at 6:02 PM, Catalin Marinas <catalin.marinas at arm.com> wrote:
> > >>
> > >> I assume that you inhibited the call to flush_dcache_page() in
> > >> __get_user_pages() for anon pages.  Otherwise, you will be flooded
> > >> with warnings.
> > >
> > > I haven't done any stress testing so I don't think I hit this code path,
> > > so no warning. But yes, it should have triggered. Anyway, in this case
> > > flush_dcache_page() should have just ignored (clearing PG_arch_1 is
> > > harmless anyway if we also ignore this bit in __sync_icache_dcache for
> > > non-aliasing caches).
> > 
> > Yes, maybe we can do a little optimization for O_DIRECT since no
> > dcache alias and I/D coherency problem in this case on ARMv7, how
> > about below change?
> > 
> > diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
> > index 1c8f7f5..962a657 100644
> > --- a/arch/arm/mm/flush.c
> > +++ b/arch/arm/mm/flush.c
> > @@ -287,6 +287,8 @@ void flush_dcache_page(struct page *page)
> >  	    mapping && !mapping_mapped(mapping))
> >  		clear_bit(PG_dcache_clean, &page->flags);
> >  	else {
> > +		if (!mapping && cache_is_vipt_nonaliasing())
> > +			return;
> >  		__flush_dcache_page(mapping, page);
> >  		if (mapping && cache_is_vivt())
> >  			__flush_dcache_aliases(mapping, page);
> 
> I wonder whether we could move the:
> 
> 	if (!mapping)
> 		return
> 
> at the top of this function, IOW don't touch any anonymous pages. Would
> anything be broken (apart from wrong API use)?

I suspect you may be missing something - and I suggest reading the Sparc
version, which is where this PG_arch_1 stuff was first dreamed up (by
DaveM).

Are you positive that this path can be eliminated?  Totally sure?  Have
you checked the arg/env pages that fs/exec.c and the binfmt's insert into
the new process?

The comments against the Sparc version suggest that the "else" clause
is necessary ensure that case is adequately covered.



More information about the linux-arm-kernel mailing list