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;
MIME-Version: 1.0
References: <20230329073322.323177-1-feng.tang@intel.com> <ZCUPxMQPJ8ETvUbM@debian.me>
In-Reply-To: <ZCUPxMQPJ8ETvUbM@debian.me>
From:   Shakeel Butt <shakeelb@google.com>
Date:   Wed, 29 Mar 2023 21:51:56 -0700
Message-ID: <CALvZod48Fwua_VJvnzHatF-J4YRWqfMFnYjYN6W0_ioLtPZEfA@mail.gmail.com>
Subject: Re: [PATCH v2] Documentation: Add document for false sharing
To:     Bagas Sanjaya <bagasdotme@gmail.com>
Cc:     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>,
        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>, dave.hansen@intel.com,
        ying.huang@intel.com, tim.c.chen@intel.com, andi.kleen@intel.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Mar 29, 2023 at 9:27=E2=80=AFPM Bagas Sanjaya <bagasdotme@gmail.com=
> wrote:
>
> 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_s=
ockets_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 red=
uce 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 t=
o vm overcommit policy")
> > +
>
> Here's what I mean by bridging conjunctions to example commits as I revie=
wed
> in v1 [1]:
>

This is too much and unneeded nitpicking. The patch looks good as is.