On 14/03/2018 09:48, Peter Zijlstra wrote: > On Tue, Mar 13, 2018 at 06:59:47PM +0100, Laurent Dufour wrote: >> This change is inspired by the Peter's proposal patch [1] which was >> protecting the VMA using SRCU. Unfortunately, SRCU is not scaling well in >> that particular case, and it is introducing major performance degradation >> due to excessive scheduling operations. > > Do you happen to have a little more detail on that? This has been reported by kemi who find bad performance when running some benchmarks on top of the v5 series: https://patchwork.kernel.org/patch/9999687/ It appears that SRCU is generating a lot of additional scheduling to manage the freeing of the VMA structure. SRCU is dealing through per cpu ressources but the SRCU callback is And since we are handling this way a per process ressource (VMA) through a global resource (SRCU) this leads to a lot of overhead when scheduling the SRCU callback. >> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h >> index 34fde7111e88..28c763ea1036 100644 >> --- a/include/linux/mm_types.h >> +++ b/include/linux/mm_types.h >> @@ -335,6 +335,7 @@ struct vm_area_struct { >> struct vm_userfaultfd_ctx vm_userfaultfd_ctx; >> #ifdef CONFIG_SPECULATIVE_PAGE_FAULT >> seqcount_t vm_sequence; >> + atomic_t vm_ref_count; /* see vma_get(), vma_put() */ >> #endif >> } __randomize_layout; >> >> @@ -353,6 +354,9 @@ struct kioctx_table; >> struct mm_struct { >> struct vm_area_struct *mmap; /* list of VMAs */ >> struct rb_root mm_rb; >> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT >> + rwlock_t mm_rb_lock; >> +#endif >> u32 vmacache_seqnum; /* per-thread vmacache */ >> #ifdef CONFIG_MMU >> unsigned long (*get_unmapped_area) (struct file *filp, > > When I tried this, it simply traded contention on mmap_sem for > contention on these two cachelines. > > This was for the concurrent fault benchmark, where mmap_sem is only ever > acquired for reading (so no blocking ever happens) and the bottle-neck > was really pure cacheline access. I'd say that this expected if multiple threads are dealing on the same VMA, but if the VMA differ then this contention is disappearing while it is remaining when using the mmap_sem. This being said, test I did on PowerPC using will-it-scale/page_fault1_threads showed that the number of caches-misses generated in get_vma() are very low (less than 5%). Am I missing something ? > Only by using RCU can you avoid that thrashing. I agree, but this kind of test is the best use case for SRCU because there are not so many updates, so not a lot of call to the SRCU asynchronous callback. Honestly, I can't see an ideal solution here, RCU is not optimal when there is a high number of updates, and using a rwlock may introduced a bottleneck there. I get better results when using the rwlock than using SRCU in that case, but if you have another proposal, please advise, I'll give it a try. > Also note that if your database allocates the one giant mapping, it'll > be _one_ VMA and that vm_ref_count gets _very_ hot indeed. In the case of the database product I mentioned in the series header, that's the opposite, the VMA number is very high so this doesn't happen. But in the case of one VMA, it's clear that there will be a contention on vm_ref_count, but this would be better than blocking on the mmap_sem. Laurent.