Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 30 Mar 2023 11:27:48 +0700
From:   Bagas Sanjaya <bagasdotme@gmail.com>
To:     Feng Tang <feng.tang@intel.com>, Jonathan Corbet <corbet@lwn.net>,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        Randy Dunlap <rdunlap@infradead.org>
Cc:     Arnaldo Carvalho de Melo <acme@redhat.com>,
        Joe Mario <jmario@redhat.com>, Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Eric Dumazet <edumazet@google.com>,
        Shakeel Butt <shakeelb@google.com>, dave.hansen@intel.com,
        ying.huang@intel.com, tim.c.chen@intel.com, andi.kleen@intel.com
Subject: Re: [PATCH v2] Documentation: Add document for false sharing
Message-ID: <ZCUPxMQPJ8ETvUbM@debian.me>
References: <20230329073322.323177-1-feng.tang@intel.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="+bc6jzHwCokGdRls"
Content-Disposition: inline
In-Reply-To: <20230329073322.323177-1-feng.tang@intel.com>
Precedence: bulk


--+bc6jzHwCokGdRls
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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_soc=
kets_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 reduc=
e 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() m=
ultithreaded 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 ca=
che 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:
=20
 * 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_socke=
ts_allocated")
+  cache line and TLB entries. The example implentation is in commit
+  91b6d3256356 ("net: cache align tcp_memory_allocated, tcp_sockets_alloca=
ted").
=20
 * 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").
=20
 * 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 c=
ommon mitigations
 		set_bit(XXX);
=20
   instead of directly "set_bit(XXX);", similarly for atomic_t data.
+  Example commits are:
=20
-  - Commit 7b1002f7cfe5 ("bcache: fixup bcache_dev_sectors_dirty_add() mul=
tithreaded CPU false sharing")
-  - Commit 292648ac5cf1 ("mm: gup: allow FOLL_PIN to scale in SMP")
+  - 7b1002f7cfe5 ("bcache: fixup bcache_dev_sectors_dirty_add() multithrea=
ded
+    CPU false sharing")
+  - 292648ac5cf1 ("mm: gup: allow FOLL_PIN to scale in SMP")
=20
 * 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:
=20
-  - Commit 520f897a3554 ("ext4: use percpu_counters for extent_status cach=
e 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 overco=
mmit
+    policy")
=20
 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/

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

--+bc6jzHwCokGdRls
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQSSYQ6Cy7oyFNCHrUH2uYlJVVFOowUCZCUPvwAKCRD2uYlJVVFO
o6ngAQCe45mznyyZlyy4uLSGqihng7Df4U/iJd9SjCZBs5/2+gD9Ei5+SL30Pf4H
NOpggsvX5sBAmHridg0qrx79tLN/pQE=
=WQq6
-----END PGP SIGNATURE-----

--+bc6jzHwCokGdRls--