Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: Tejun Heo <htejun@gmail.com>
Date:   Wed, 27 Jan 2021 16:03:05 -0500
From:   Tejun Heo <tj@kernel.org>
To:     Saravanan D <saravanand@fb.com>
Cc:     x86@kernel.org, dave.hansen@linux.intel.com, luto@kernel.org,
        peterz@infradead.org, linux-kernel@vger.kernel.org,
        kernel-team@fb.com
Subject: Re: [PATCH V2] x86/mm: Tracking linear mapping split events
Message-ID: <YBHVCZscdVsxbFUk@slm.duckdns.org>
References: <bd157a11-8e6b-5f44-4d91-d99adb9f8686@intel.com>
 <20210127175124.3289879-1-saravanand@fb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210127175124.3289879-1-saravanand@fb.com>
Precedence: bulk

Hello,

On Wed, Jan 27, 2021 at 09:51:24AM -0800, Saravanan D wrote:
> Numerous hugepage splits in the linear mapping would give
> admins the signal to narrow down the sluggishness caused by TLB
> miss/reload.
> 
> To help with debugging, we introduce monotonic lifetime  hugepage
> split event counts since SYSTEM_RUNNING to be displayed as part of
> /proc/vmstat in x86 servers
> 
> The lifetime split event information will be displayed at the bottom of
> /proc/vmstat
> ....
> swap_ra 0
> swap_ra_hit 0
> direct_map_2M_splits 139
> direct_map_4M_splits 0
> direct_map_1G_splits 7
> nr_unstable 0
> ....

This looks great to me.

> 
> Ancillary debugfs split event counts exported to userspace via read-write
> endpoints : /sys/kernel/debug/x86/direct_map_[2M|4M|1G]_split
> 
> dmesg log when user resets the debugfs split event count for
> debugging
> ....
> [  232.470531] debugfs 2M Pages split event count(128) reset to 0
> ....

I'm not convinced this part is necessary or even beneficial.

> One of the many lasting (as we don't coalesce back) sources for huge page
> splits is tracing as the granular page attribute/permission changes would
> force the kernel to split code segments mapped to huge pages to smaller
> ones thereby increasing the probability of TLB miss/reload even after
> tracing has been stopped.
> 
> Signed-off-by: Saravanan D <saravanand@fb.com>
> ---
>  arch/x86/mm/pat/set_memory.c  | 117 ++++++++++++++++++++++++++++++++++
>  include/linux/vm_event_item.h |   8 +++
>  mm/vmstat.c                   |   8 +++
>  3 files changed, 133 insertions(+)

So, now the majority of the added code is to add debugfs knobs which don't
provide anything that userland can't already do by simply reading the
monotonic counters.

Dave, are you still set on the resettable counters?

Thanks.

-- 
tejun