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$(echo .)org \
--cc=21cnbao@gmail$(echo .)com \
--cc=anshuman.khandual@arm$(echo .)com \
--cc=arnd@arndb$(echo .)de \
--cc=baohua@kernel$(echo .)org \
--cc=catalin.marinas@arm$(echo .)com \
--cc=corbet@lwn$(echo .)net \
--cc=darren@os$(echo .)amperecomputing.com \
--cc=guojian@oppo$(echo .)com \
--cc=huzhanyuan@oppo$(echo .)com \
--cc=khandual@linux$(echo .)vnet.ibm.com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-doc@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-mips@vger$(echo .)kernel.org \
--cc=linux-mm@kvack$(echo .)org \
--cc=linux-riscv@lists$(echo .)infradead.org \
--cc=linux-s390@vger$(echo .)kernel.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=lipeifeng@oppo$(echo .)com \
--cc=openrisc@lists$(echo .)librecores.org \
--cc=peterz@infradead$(echo .)org \
--cc=prime.zeng@hisilicon$(echo .)com \
--cc=punit.agrawal@bytedance$(echo .)com \
--cc=realmz6@gmail$(echo .)com \
--cc=wangkefeng.wang@huawei$(echo .)com \
--cc=will@kernel$(echo .)org \
--cc=x86@kernel$(echo .)org \
--cc=xhao@linux$(echo .)alibaba.com \
--cc=yangyicong@hisilicon$(echo .)com \
--cc=yangyicong@huawei$(echo .)com \
--cc=zhangshiming@oppo$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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