On 3/27/23 07:39, Feng Tang wrote:
> Hi Bagas Sanjaya,
>
> Many thanks for the reviews!
>
> On Fri, Mar 24, 2023 at 07:45:28PM +0700, Bagas Sanjaya wrote:
>> On Fri, Mar 24, 2023 at 03:13:16PM +0800, Feng Tang wrote:
>>> +There are many real-world cases of performance regressions caused by
>>> +false sharing, and one is a rw_semaphore 'mmap_lock' inside struct
>> "... . One of these is rw_semaphore 'mmap_lock' ..."
>
> OK, will use this.
>
>> But I think in English we commonly name things as "foobar struct"
>> instead of "struct foobar" (that is, common noun follow the proper noun
>> that names something).
>
> I can change that. And IIRC, I saw 'struct XXX' and 'XXX struct' both
> frequently used in kernel. I just run '# git log | grep -w struct'
> and the majority use 'struct XXX'
>
>>> +* A global datum accessed (shared) by many CPUs
>> Global data?
>
> In RFC version, I used 'data' and Randy suggested 'datum'. TBH, I
> looked it up in a dictionary :), and found:
> "Data" is the Latin plural form of "datum"
>
OK, I understand.
>>> + #perf c2c record -ag sleep 3
>>> + #perf c2c report --call-graph none -k vmlinux
>>
>> Are these commands really run as root?
>
> You are right, people can run it as 'root' or a normal user. And I
> guess this won't confuse kernel developers.
>
> My original version is kind of too long and full of explainations,
> and some kernel developer suggested that this doc is under
> 'kernel-hacking' and its audience is kernel developers, and I should
> make it clear and short, and not make it look like a wiki page or
> man page.
>
So something like below, right?
```
$ perf <command> <args>...
$ perf <command> <args>...
```
>>> +* 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:
>> "... For example, write::"
>
> The following is a coding pattern (for bit operation, atomic, etc.),
> and I think 'use' may also be good?
>
I tend to say "write" when the context is typing code.
>>> +
>>> + if (!test_bit(XXX))
>>> + set_bit(XXX);
>>> +
>>> + instead of directly "set_bit(XXX);", similarly for atomic_t data.
>>> +
>>> + 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")
>>
>> IMO it's odd to jump to specifying example commits without some sort of
>> conjuction (e.g. "for example, see commit <commit>").
>
> I agree, and I had the same concern, but I was also afraid of that
> too many repeating of this, so the previous
> "Following 'mitigation' section provides real-world examples."
> in last section (which you helped to improve) was added trying
> to address this.
>
OK.
And see you in v2!
--
An old man doll... just what I always wanted! - Clara
