RE: [PATCH 09/13] unicore: mmu_gather rework

[Guan Xuetao]

> -----Original Message-----
> From: Peter Zijlstra [mailto:a.p.zijlstra@xxxxxxxxx]
> Sent: Friday, March 04, 2011 8:17 PM
> To: Guan Xuetao
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-arch@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; 'Benjamin Herrenschmidt'; 'David Miller'; 'Hugh
> Dickins'; 'Mel Gorman'; 'Nick Piggin'; 'Paul McKenney'; 'Yanmin Zhang'; 'Andrea Arcangeli'; 'Avi Kivity'; 'Thomas Gleixner'; 'Rik van Riel';
> 'Ingo Molnar'; akpm@xxxxxxxxxxxxxxxxxxxx; 'Linus Torvalds'; Arnd Bergmann
> Subject: RE: [PATCH 09/13] unicore: mmu_gather rework
> 
> On Fri, 2011-03-04 at 19:56 +0800, Guan Xuetao wrote:
> 
> > Thanks Peter.
> > It looks good to me, though it is dependent on your patch set "mm: Preemptible mmu_gather"
> 
> It is indeed, the split-out per arch is purely to ease review. The final
> commit should be a merge of the first 10 patches so as not to break
> bisection.
> 
> > While I have another look to include/asm-generic/tlb.h, I found it is also suitable for unicore32.
> > And so, I rewrite the tlb.h to use asm-generic version, and then your patch set will also work for me.
> 
> Awesome, I notice you're loosing flush_tlb_range() support for this, if
> you're fine with that I'm obviously not going to argue, but if its
> better for your platform to keep doing this we can work on that as well
> as I'm trying to add generic support for range tracking into the generic
> tlb code.
Yes, I think flush_tlb_range() have no effect in unicore32 architecture.
Or perhaps, it is because no optimization, just as you point it below.

> 
> More importantly, you seem to loose your call to flush_cache_range()
> which isn't a NOP on your platform.
IMO, flush_cache_range() is only used in self-modified codes when cachetype is vipt.
So, it could be neglected here.
Perhaps it's wrong.
 
> 
> Furthermore, while arch/unicore32/mm/tlb-ucv2.S is mostly magic to me, I
> see unicore32 is one of the few architectures that actually uses
> vm_flags in flush_tlb_range(). Do you have independent I/D-TLB flushes
> or are you flushing I-cache on VM_EXEC?
We have both independent and global I/D TLB flushes.
And flushing I-cache on VM_EXEC is also needed in self-modified codes, IMO.

> 
> Also, I notice your flush_tlb_range() implementation looks to be a loop
> invalidating individual pages, which I can imagine is cheaper for small
> ranges but large ranges might be better of with a full invalidate. Did
> you think about this?
> 
Yes, it should be optimized.
However, I doubt its effect in unicore32 which has no asid support.

Thanks & Regards.

Guan Xuetao

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/