Re: [PATCH v2] Documentation: Add document for false sharing

[Bagas Sanjaya <bagasdotme@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5127 bytes --]

On Wed, Mar 29, 2023 at 03:33:22PM +0800, Feng Tang wrote:
> +False sharing hurting performance cases are seen more frequently with
> +core count increasing.  Because of these detrimental effects, many
> +patches have been proposed across variety of subsystems (like
> +networking and memory management) and merged.  Some common mitigations
> +(with examples) are:
> +
> +* Separate hot global data in its own dedicated cache line, even if it
> +  is just a 'short' type. The downside is more consumption of memory,
> +  cache line and TLB entries.
> +
> +  - Commit 91b6d3256356 ("net: cache align tcp_memory_allocated, tcp_sockets_allocated")
> +
> +* Reorganize the data structure, separate the interfering members to
> +  different cache lines.  One downside is it may introduce new false
> +  sharing of other members.
> +
> +  - Commit 802f1d522d5f ("mm: page_counter: re-layout structure to reduce false sharing")
> +
> +* Replace 'write' with 'read' when possible, especially in loops.
> +  Like for some global variable, use compare(read)-then-write instead
> +  of unconditional write. For example, use::
> +
> +	if (!test_bit(XXX))
> +		set_bit(XXX);
> +
> +  instead of directly "set_bit(XXX);", similarly for atomic_t data.
"... The similar technique is also applicable to atomic_t data".

But how?

> +
> +  - Commit 7b1002f7cfe5 ("bcache: fixup bcache_dev_sectors_dirty_add() multithreaded CPU false sharing")
> +  - Commit 292648ac5cf1 ("mm: gup: allow FOLL_PIN to scale in SMP")
> +
> +* Turn hot global data to 'per-cpu data + global data' when possible,
> +  or reasonably increase the threshold for syncing per-cpu data to
> +  global data, to reduce or postpone the 'write' to that global data.
> +
> +  - Commit 520f897a3554 ("ext4: use percpu_counters for extent_status cache hits/misses")
> +  - Commit 56f3547bfa4d ("mm: adjust vm_committed_as_batch according to vm overcommit policy")
> +

Here's what I mean by bridging conjunctions to example commits as I reviewed
in v1 [1]:

---- >8 ----
diff --git a/Documentation/kernel-hacking/false-sharing.rst b/Documentation/kernel-hacking/false-sharing.rst
index ceeaf20290eabd..3b08f6f16d442e 100644
--- a/Documentation/kernel-hacking/false-sharing.rst
+++ b/Documentation/kernel-hacking/false-sharing.rst
@@ -141,19 +141,18 @@ False sharing hurting performance cases are seen more frequently with
 core count increasing.  Because of these detrimental effects, many
 patches have been proposed across variety of subsystems (like
 networking and memory management) and merged.  Some common mitigations
-(with examples) are:
+are:
 
 * Separate hot global data in its own dedicated cache line, even if it
   is just a 'short' type. The downside is more consumption of memory,
-  cache line and TLB entries.
-
-  - Commit 91b6d3256356 ("net: cache align tcp_memory_allocated, tcp_sockets_allocated")
+  cache line and TLB entries. The example implentation is in commit
+  91b6d3256356 ("net: cache align tcp_memory_allocated, tcp_sockets_allocated").
 
 * Reorganize the data structure, separate the interfering members to
   different cache lines.  One downside is it may introduce new false
-  sharing of other members.
-
-  - Commit 802f1d522d5f ("mm: page_counter: re-layout structure to reduce false sharing")
+  sharing of other members. How it is done is illustrated by commit
+  802f1d522d5f ("mm: page_counter: re-layout structure to reduce false
+  sharing").
 
 * Replace 'write' with 'read' when possible, especially in loops.
   Like for some global variable, use compare(read)-then-write instead
@@ -163,16 +162,21 @@ networking and memory management) and merged.  Some common mitigations
 		set_bit(XXX);
 
   instead of directly "set_bit(XXX);", similarly for atomic_t data.
+  Example commits are:
 
-  - Commit 7b1002f7cfe5 ("bcache: fixup bcache_dev_sectors_dirty_add() multithreaded CPU false sharing")
-  - Commit 292648ac5cf1 ("mm: gup: allow FOLL_PIN to scale in SMP")
+  - 7b1002f7cfe5 ("bcache: fixup bcache_dev_sectors_dirty_add() multithreaded
+    CPU false sharing")
+  - 292648ac5cf1 ("mm: gup: allow FOLL_PIN to scale in SMP")
 
 * Turn hot global data to 'per-cpu data + global data' when possible,
   or reasonably increase the threshold for syncing per-cpu data to
   global data, to reduce or postpone the 'write' to that global data.
+  Examples are in commits:
 
-  - Commit 520f897a3554 ("ext4: use percpu_counters for extent_status cache hits/misses")
-  - Commit 56f3547bfa4d ("mm: adjust vm_committed_as_batch according to vm overcommit policy")
+  - 520f897a3554 ("ext4: use percpu_counters for extent_status cache
+    hits/misses")
+  - 56f3547bfa4d ("mm: adjust vm_committed_as_batch according to vm overcommit
+    policy")
 
 Surely, all mitigations should be carefully verified to not cause side
 effects.  And to avoid false sharing in advance during coding, it's

Thanks.

[1]: https://lore.kernel.org/linux-doc/ZB2baIDIPhxj5Vdl@debian.me/

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]