| From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> |
| Subject: mm: move mm_count into its own cache line |
| Date: Mon, 15 May 2023 10:35:36 -0400 |
| |
| The mm_struct mm_count field is frequently updated by mmgrab/mmdrop |
| performed by context switch. This causes false-sharing for surrounding |
| mm_struct fields which are read-mostly. |
| |
| This has been observed on a 2sockets/112core/224cpu Intel Sapphire Rapids |
| server running hackbench, and by the kernel test robot will-it-scale |
| testcase. |
| |
| Move the mm_count field into its own cache line to prevent false-sharing |
| with other mm_struct fields. |
| |
| Move mm_count to the first field of mm_struct to minimize the amount of |
| padding required: rather than adding padding before and after the mm_count |
| field, padding is only added after mm_count. |
| |
| Note that I noticed this odd comment in mm_struct: |
| |
| commit 2e3025434a6b ("mm: relocate 'write_protect_seq' in struct mm_struct") |
| |
| /* |
| * With some kernel config, the current mmap_lock's offset |
| * inside 'mm_struct' is at 0x120, which is very optimal, as |
| * its two hot fields 'count' and 'owner' sit in 2 different |
| * cachelines, and when mmap_lock is highly contended, both |
| * of the 2 fields will be accessed frequently, current layout |
| * will help to reduce cache bouncing. |
| * |
| * So please be careful with adding new fields before |
| * mmap_lock, which can easily push the 2 fields into one |
| * cacheline. |
| */ |
| struct rw_semaphore mmap_lock; |
| |
| This comment is rather odd for a few reasons: |
| |
| - It requires addition/removal of mm_struct fields to carefully consider |
| field alignment of _other_ fields, |
| - It expresses the wish to keep an "optimal" alignment for a specific |
| kernel config. |
| |
| I suspect that the author of this comment may want to revisit this topic |
| and perhaps introduce a split-struct approach for struct rw_semaphore, |
| if the need is to place various fields of this structure in different |
| cache lines. |
| |
| Link: https://lkml.kernel.org/r/20230515143536.114960-1-mathieu.desnoyers@efficios.com |
| Fixes: 223baf9d17f2 ("sched: Fix performance regression introduced by mm_cid") |
| Fixes: af7f588d8f73 ("sched: Introduce per-memory-map concurrency ID") |
| Link: https://lore.kernel.org/lkml/7a0c1db1-103d-d518-ed96-1584a28fbf32@efficios.com |
| Reported-by: kernel test robot <yujie.liu@intel.com> |
| Link: https://lore.kernel.org/oe-lkp/202305151017.27581d75-yujie.liu@intel.com |
| Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> |
| Reviewed-by: Aaron Lu <aaron.lu@intel.com> |
| Reviewed-by: John Hubbard <jhubbard@nvidia.com> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Olivier Dion <odion@efficios.com> |
| Cc: <michael.christie@oracle.com> |
| Cc: Feng Tang <feng.tang@intel.com> |
| Cc: Jason Gunthorpe <jgg@nvidia.com> |
| Cc: Peter Xu <peterx@redhat.com> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| --- |
| |
| include/linux/mm_types.h | 23 +++++++++++++++-------- |
| 1 file changed, 15 insertions(+), 8 deletions(-) |
| |
| --- a/include/linux/mm_types.h~mm-move-mm_count-into-its-own-cache-line |
| +++ a/include/linux/mm_types.h |
| @@ -583,6 +583,21 @@ struct mm_cid { |
| struct kioctx_table; |
| struct mm_struct { |
| struct { |
| + /* |
| + * Fields which are often written to are placed in a separate |
| + * cache line. |
| + */ |
| + struct { |
| + /** |
| + * @mm_count: The number of references to &struct |
| + * mm_struct (@mm_users count as 1). |
| + * |
| + * Use mmgrab()/mmdrop() to modify. When this drops to |
| + * 0, the &struct mm_struct is freed. |
| + */ |
| + atomic_t mm_count; |
| + } ____cacheline_aligned_in_smp; |
| + |
| struct maple_tree mm_mt; |
| #ifdef CONFIG_MMU |
| unsigned long (*get_unmapped_area) (struct file *filp, |
| @@ -620,14 +635,6 @@ struct mm_struct { |
| */ |
| atomic_t mm_users; |
| |
| - /** |
| - * @mm_count: The number of references to &struct mm_struct |
| - * (@mm_users count as 1). |
| - * |
| - * Use mmgrab()/mmdrop() to modify. When this drops to 0, the |
| - * &struct mm_struct is freed. |
| - */ |
| - atomic_t mm_count; |
| #ifdef CONFIG_SCHED_MM_CID |
| /** |
| * @pcpu_cid: Per-cpu current cid. |
| _ |