Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Yang Shi <yang.shi@linux.alibaba.com>
To: Yu Xu <xuyu@linux.alibaba.com>,
	Catalin Marinas <catalin.marinas@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Hillf Danton <hdanton@sina.com>, Hugh Dickins <hughd@google.com>,
	Josef Bacik <josef@toxicpanda.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Linux-MM <linux-mm@kvack.org>,
	mm-commits@vger.kernel.org, Will Deacon <will.deacon@arm.com>,
	Matthew Wilcox <willy@infradead.org>
Subject: Re: [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault
Date: Mon, 27 Jul 2020 11:04:01 -0700
Message-ID: <eb1f5cb4-7c3d-df42-f4aa-804e12df45e2@linux.alibaba.com> (raw)
In-Reply-To: <39560818-463f-da3a-fc9e-3a4a0a082f61@linux.alibaba.com>



On 7/27/20 10:12 AM, Yu Xu wrote:
> On 7/27/20 7:05 PM, Catalin Marinas wrote:
>> On Mon, Jul 27, 2020 at 03:31:16PM +0800, Yu Xu wrote:
>>> On 7/25/20 4:22 AM, Linus Torvalds wrote:
>>>> On Fri, Jul 24, 2020 at 12:27 PM Linus Torvalds
>>>> <torvalds@linux-foundation.org> wrote:
>>>>>
>>>>> It *may* make sense to say "ok, don't bother flushing the TLB if this
>>>>> is a retry, because we already did that originally". MAYBE.
>> [...]
>>>> We could say that we never need it at all for FAULT_FLAG_RETRY. That
>>>> makes a lot of sense to me.
>>>>
>>>> So a patch that does something like the appended (intentionally
>>>> whitespace-damaged) seems sensible.
>>>
>>> I tested your patch on our aarch64 box, with 128 online CPUs.
>> [...]
>>> There are two points to sum up.
>>>
>>> 1) the performance of page_fault3_process is restored, while the 
>>> performance
>>> of page_fault3_thread is about ~80% of the vanilla, except the case 
>>> of 128
>>> threads.
>>>
>>> 2) in the case of 128 threads, test worker threads seem to get 
>>> stuck, making
>>> no progress in the iterations of mmap-write-munmap until a period of 
>>> time
>>> later.  the test result is 0 because only first 16 samples are 
>>> counted, and
>>> they are all 0.  This situation is easy to re-produce with large 
>>> number of
>>> threads (not necessarily 128), and the stack of one stuck thread is 
>>> shown
>>> below.
>>>
>>> [<0>] __switch_to+0xdc/0x150
>>> [<0>] wb_wait_for_completion+0x84/0xb0
>>> [<0>] __writeback_inodes_sb_nr+0x9c/0xe8
>>> [<0>] try_to_writeback_inodes_sb+0x6c/0x88
>>> [<0>] ext4_nonda_switch+0x90/0x98 [ext4]
>>> [<0>] ext4_page_mkwrite+0x248/0x4c0 [ext4]
>>> [<0>] do_page_mkwrite+0x4c/0x100
>>> [<0>] do_fault+0x2ac/0x3e0
>>> [<0>] handle_pte_fault+0xb4/0x258
>>> [<0>] __handle_mm_fault+0x1d8/0x3a8
>>> [<0>] handle_mm_fault+0x104/0x1d0
>>> [<0>] do_page_fault+0x16c/0x490
>>> [<0>] do_translation_fault+0x60/0x68
>>> [<0>] do_mem_abort+0x58/0x100
>>> [<0>] el0_da+0x24/0x28
>>> [<0>] 0xffffffffffffffff
>>>
>>> It seems quite normal, right? and I've run out of ideas.
>>
>> If threads get stuck here, it could be a stale TLB entry that's not
>> flushed with Linus' patch. Since that's a write fault, I think it hits
>> the FAULT_FLAG_TRIED case.
>
> There must be some changes in my test box, because I find that even the
> vanilla kernel (89b15332af7c^) get result of 0 in 128t testcase. And I
> just directly used the history test data as the baseline.  I will dig
> into this then.

Thanks for doing the test.

>
> And do we still need to concern the ~20% performance drop in thread mode?

I guess there might be more resource contention for thread mode, i.e. 
page table lock, etc so the result might be not very stable. And retried 
page fault may exacerbate such contention. Anyway we got the process 
mode back to normal and improved the thread mode a lot.

>
>>
>> Could you give my patch here a try as an alternative:
>>
>> https://lore.kernel.org/linux-mm/20200725155841.GA14490@gaia/
>
> I ran the same test on the same aarch64 box, with your patch, the result
> is as follows.
>
> test          vanilla kernel      patched kernel
> parameter     (89b15332af7c^)     (Catalin's patch)
> 1p            829299              787676    (96.36 %)
> 1t            998007              789284    (78.36 %)
> 32p           18916718            17921100  (94.68 %)
> 32t           2020918             1644146   (67.64 %)
> 64p           18965168            18983580  (100.0 %)
> 64t           1415404             1093750   (48.03 %)
> 96p           18949438            18963921  (100.1 %)
> 96t           1622876             1262878   (63.72 %)
> 128p          18926813            1680146   (8.89  %)
> 128t          1643109             0 (0.00 % ) # ignore this temporarily

It looks Linus's patch has better data. It seems sane to me since 
Catalin's patch still needs flush TLB in the shared domain.

>
> Thanks
> Yu
>
>>
>> It leaves the spurious flush in place but only local (though note that
>> in a guest under KVM, all local TLBIs are upgraded to inner-shareable,
>> so you'd not get the performance benefit).
>>



  reply index

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-24  4:14 incoming Andrew Morton
2020-07-24  4:15 ` [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault Andrew Morton
2020-07-24  4:38   ` Yang Shi
2020-07-24  4:56     ` Andrew Morton
2020-07-24 19:27   ` Linus Torvalds
2020-07-24 20:22     ` Linus Torvalds
2020-07-25  0:36       ` Yang Shi
2020-07-25  1:29         ` Linus Torvalds
2020-07-25 15:58           ` Catalin Marinas
2020-07-28  9:22             ` Will Deacon
2020-07-28  9:39               ` Catalin Marinas
2020-07-28 10:07                 ` Yu Xu
2020-07-28 11:46                   ` Catalin Marinas
2020-07-28 10:21                 ` Will Deacon
2020-07-28 18:28                 ` Linus Torvalds
2020-07-27 17:52           ` Yang Shi
2020-07-27 18:04             ` Linus Torvalds
2020-07-27 18:42               ` Catalin Marinas
2020-07-27 20:56                 ` Linus Torvalds
2020-07-27 22:34               ` Yang Shi
2020-07-27  7:31       ` Yu Xu
2020-07-27 11:05         ` Catalin Marinas
2020-07-27 17:01           ` Linus Torvalds
2020-07-28 11:19             ` Catalin Marinas
2020-07-27 17:12           ` Yu Xu
2020-07-27 18:04             ` Yang Shi [this message]
2020-07-27 18:37               ` Linus Torvalds
2020-07-27 22:43                 ` Yang Shi
2020-07-28  0:38                   ` Linus Torvalds
2020-07-28  0:13                 ` Yu Xu
2020-07-28 10:53                 ` Nicholas Piggin
2020-07-28 19:02                   ` Linus Torvalds
2020-07-28 22:53                     ` Nicholas Piggin
2020-07-29 13:58                       ` Michael Ellerman
2020-07-28  6:41             ` Yu Xu
2020-07-24  4:15 ` [patch 02/15] mm/mmap.c: close race between munmap() and expand_upwards()/downwards() Andrew Morton
2020-07-24  4:15 ` [patch 03/15] vfs/xattr: mm/shmem: kernfs: release simple xattr entry in a right way Andrew Morton
2020-07-24  4:15 ` [patch 04/15] mm: initialize return of vm_insert_pages Andrew Morton
2020-07-24  4:15 ` [patch 05/15] mm/memcontrol: fix OOPS inside mem_cgroup_get_nr_swap_pages() Andrew Morton
2020-07-24  4:15 ` [patch 06/15] mm/memcg: fix refcount error while moving and swapping Andrew Morton
2020-07-24 13:41   ` Alex Shi
2020-07-24  4:15 ` [patch 07/15] mm: memcg/slab: fix memory leak at non-root kmem_cache destroy Andrew Morton
2020-07-24  4:15 ` [patch 08/15] mm/hugetlb: avoid hardcoding while checking if cma is enabled Andrew Morton
2020-07-24  4:15 ` [patch 09/15] khugepaged: fix null-pointer dereference due to race Andrew Morton
2020-07-24  4:15 ` [patch 10/15] mailmap: add entry for Mike Rapoport Andrew Morton
2020-07-24  4:15 ` [patch 11/15] squashfs: fix length field overlap check in metadata reading Andrew Morton
2020-07-24  4:15 ` [patch 12/15] scripts/decode_stacktrace: strip basepath from all paths Andrew Morton
2020-07-24  4:15 ` [patch 13/15] io-mapping: indicate mapping failure Andrew Morton
2020-07-24  4:15 ` [patch 14/15] MAINTAINERS: add KCOV section Andrew Morton
2020-07-24  4:15 ` [patch 15/15] scripts/gdb: fix lx-symbols 'gdb.error' while loading modules Andrew Morton
2020-07-28  1:19 ` mmotm 2020-07-27-18-18 uploaded Andrew Morton
2020-07-28  2:14   ` Stephen Rothwell
2020-07-28  3:22   ` mmotm 2020-07-27-18-18 uploaded (drivers/scsi/ufs/: SCSI_UFS_EXYNOS) Randy Dunlap
2020-07-28  8:23     ` Alim Akhtar
2020-07-28 12:33   ` mmotm 2020-07-27-18-18 uploaded (mm/page_alloc.c) Randy Dunlap
2020-07-28 21:55     ` Andrew Morton
2020-07-28 22:20       ` Stephen Rothwell
2020-07-28 22:31         ` Andrew Morton
2020-07-29 14:18           ` Michael S. Tsirkin
2020-07-29 14:38             ` David Hildenbrand
2020-07-29 16:14               ` David Hildenbrand
2020-07-29 17:29                 ` Randy Dunlap
2020-07-28 22:39       ` Randy Dunlap
2020-07-29  1:43         ` Nathan Chancellor
2020-07-29  1:44         ` Andrew Morton
2020-07-29  2:04           ` Randy Dunlap
2020-07-29 14:09           ` make oldconfig (Re: mmotm 2020-07-27-18-18 uploaded (mm/page_alloc.c)) Alexey Dobriyan
2020-07-31 23:46 ` mmotm 2020-07-31-16-45 uploaded Andrew Morton
2020-08-01  5:24   ` mmotm 2020-07-31-16-45 uploaded (drivers/staging/vc04_services/) Randy Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eb1f5cb4-7c3d-df42-f4aa-804e12df45e2@linux.alibaba.com \
    --to=yang.shi@linux.alibaba.com \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=hannes@cmpxchg.org \
    --cc=hdanton@sina.com \
    --cc=hughd@google.com \
    --cc=josef@toxicpanda.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-mm@kvack.org \
    --cc=mm-commits@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    --cc=willy@infradead.org \
    --cc=xuyu@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git