From: Andrew Morton <akpm@linux-foundation.org>
To: Yicong Yang <yangyicong@huawei.com>
Cc: wangkefeng.wang@huawei.com, prime.zeng@hisilicon.com,
realmz6@gmail.com, linux-doc@vger.kernel.org,
peterz@infradead.org, catalin.marinas@arm.com,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
punit.agrawal@bytedance.com, linux-riscv@lists.infradead.org,
will@kernel.org, Anshuman Khandual <khandual@linux.vnet.ibm.com>,
linux-s390@vger.kernel.org, zhangshiming@oppo.com,
lipeifeng@oppo.com, corbet@lwn.net, x86@kernel.org,
Barry Song <21cnbao@gmail.com>,
arnd@arndb.de, anshuman.khandual@arm.com,
openrisc@lists.librecores.org, darren@os.amperecomputing.com,
yangyicong@hisilicon.com, linux-arm-kernel@lists.infradead.org,
Barry Song <baohua@kernel.org>,
guojian@oppo.com, xhao@linux.alibaba.com,
linux-mips@vger.kernel.org, huzhanyuan@oppo.com,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v7 1/2] mm/tlbbatch: Introduce arch_tlbbatch_should_defer()
Date: Tue, 29 Nov 2022 15:23:06 -0800 [thread overview]
Message-ID: <20221129152306.54b6d439e2a0ca7ece1d1afa@linux-foundation.org> (raw)
In-Reply-To: <20221117082648.47526-2-yangyicong@huawei.com>
On Thu, 17 Nov 2022 16:26:47 +0800 Yicong Yang <yangyicong@huawei.com> wrote:
> From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>
> The entire scheme of deferred TLB flush in reclaim path rests on the
> fact that the cost to refill TLB entries is less than flushing out
> individual entries by sending IPI to remote CPUs. But architecture
> can have different ways to evaluate that. Hence apart from checking
> TTU_BATCH_FLUSH in the TTU flags, rest of the decision should be
> architecture specific.
>
> ...
>
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -240,6 +240,18 @@ static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long a)
> flush_tlb_mm_range(vma->vm_mm, a, a + PAGE_SIZE, PAGE_SHIFT, false);
> }
>
> +static inline bool arch_tlbbatch_should_defer(struct mm_struct *mm)
> +{
> + bool should_defer = false;
> +
> + /* If remote CPUs need to be flushed then defer batch the flush */
> + if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids)
> + should_defer = true;
> + put_cpu();
> +
> + return should_defer;
> +}
> +
> static inline u64 inc_mm_tlb_gen(struct mm_struct *mm)
> {
> /*
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 2ec925e5fa6a..a9ab10bc0144 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -685,17 +685,10 @@ static void set_tlb_ubc_flush_pending(struct mm_struct *mm, bool writable)
> */
> static bool should_defer_flush(struct mm_struct *mm, enum ttu_flags flags)
> {
> - bool should_defer = false;
> -
> if (!(flags & TTU_BATCH_FLUSH))
> return false;
>
> - /* If remote CPUs need to be flushed then defer batch the flush */
> - if (cpumask_any_but(mm_cpumask(mm), get_cpu()) < nr_cpu_ids)
> - should_defer = true;
> - put_cpu();
> -
> - return should_defer;
> + return arch_tlbbatch_should_defer(mm);
> }
I think this conversion could have been done better.
should_defer_flush() is compiled if
CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH. So the patch implicitly
assumes that only x86 implements
CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH. Presently true, but what
happens if sparc (for example) wants to set
CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH? Now sparc needs its private
version of arch_tlbbatch_should_defer(), even if that is identical to
x86's.
Wouldn't it be better to make arch_tlbbatch_should_defer() a __weak
function in rmap.c, or a static inline inside #ifndef
ARCH_HAS_ARCH_TLBBATCH_SHOULD_DEFER, or whatever technique best fits?
next prev parent reply other threads:[~2022-11-29 23:24 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-17 8:26 [PATCH v7 0/2] arm64: support batched/deferred tlb shootdown during page reclamation Yicong Yang
2022-11-17 8:26 ` [PATCH v7 1/2] mm/tlbbatch: Introduce arch_tlbbatch_should_defer() Yicong Yang
2022-11-29 23:23 ` Andrew Morton [this message]
2022-11-30 2:23 ` Yicong Yang
2022-11-30 2:57 ` Anshuman Khandual
2022-11-17 8:26 ` [PATCH v7 2/2] arm64: support batched/deferred tlb shootdown during page reclamation Yicong Yang
2022-11-23 14:07 ` Anshuman Khandual
2023-01-05 18:14 ` Catalin Marinas
2023-01-08 10:48 ` Barry Song
2023-01-09 17:19 ` Catalin Marinas
2023-01-09 21:28 ` Barry Song
2022-11-29 11:09 ` [PATCH v7 0/2] " Yicong Yang
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=20221129152306.54b6d439e2a0ca7ece1d1afa@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=21cnbao@gmail.com \
--cc=anshuman.khandual@arm.com \
--cc=arnd@arndb.de \
--cc=baohua@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=corbet@lwn.net \
--cc=darren@os.amperecomputing.com \
--cc=guojian@oppo.com \
--cc=huzhanyuan@oppo.com \
--cc=khandual@linux.vnet.ibm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-s390@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=lipeifeng@oppo.com \
--cc=openrisc@lists.librecores.org \
--cc=peterz@infradead.org \
--cc=prime.zeng@hisilicon.com \
--cc=punit.agrawal@bytedance.com \
--cc=realmz6@gmail.com \
--cc=wangkefeng.wang@huawei.com \
--cc=will@kernel.org \
--cc=x86@kernel.org \
--cc=xhao@linux.alibaba.com \
--cc=yangyicong@hisilicon.com \
--cc=yangyicong@huawei.com \
--cc=zhangshiming@oppo.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).