linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Vinayak Menon <vinmenon@codeaurora.org>
Cc: linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org,
	kirill.shutemov@linux.intel.com, akpm@linux-foundation.org,
	jack@suse.cz, minchan@kernel.org, catalin.marinas@arm.com,
	will.deacon@arm.com, ying.huang@intel.com, riel@redhat.com,
	dave.hansen@linux.intel.com, mgorman@suse.de,
	torvalds@linux-foundation.org
Subject: Re: [PATCH 1/2] mm: make faultaround produce old ptes
Date: Tue, 28 Nov 2017 10:12:34 +0100	[thread overview]
Message-ID: <20171128091234.GH5977@quack2.suse.cz> (raw)
In-Reply-To: <1511845670-12133-1-git-send-email-vinmenon@codeaurora.org>

On Tue 28-11-17 10:37:49, Vinayak Menon wrote:
> Based on Kirill's patch [1].
> 
> Currently, faultaround code produces young pte.  This can screw up
> vmscan behaviour[2], as it makes vmscan think that these pages are hot
> and not push them out on first round.
> 
> During sparse file access faultaround gets more pages mapped and all of
> them are young.  Under memory pressure, this makes vmscan swap out anon
> pages instead, or to drop other page cache pages which otherwise stay
> resident.
> 
> Modify faultaround to produce old ptes, so they can easily be reclaimed
> under memory pressure.
> 
> This can to some extend defeat the purpose of faultaround on machines
> without hardware accessed bit as it will not help us with reducing the
> number of minor page faults.
> 
> Making the faultaround ptes old results in a unixbench regression for some
> architectures [3][4]. But on some architectures it is not found to cause
> any regression. So by default produce young ptes and provide an option for
> architectures to make the ptes old.
> 
> [1] http://lkml.kernel.org/r/1463488366-47723-1-git-send-email-kirill.shutemov@linux.intel.com
> [2] https://lkml.kernel.org/r/1460992636-711-1-git-send-email-vinmenon@codeaurora.org
> [3] https://marc.info/?l=linux-kernel&m=146582237922378&w=2
> [4] https://marc.info/?l=linux-mm&m=146589376909424&w=2
> 
> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> ---
>  include/linux/mm-arch-hooks.h | 7 +++++++
>  include/linux/mm.h            | 2 ++
>  mm/filemap.c                  | 4 ++++
>  mm/memory.c                   | 5 +++++
>  4 files changed, 18 insertions(+)
> 
> diff --git a/include/linux/mm-arch-hooks.h b/include/linux/mm-arch-hooks.h
> index 4efc3f56..0322b98 100644
> --- a/include/linux/mm-arch-hooks.h
> +++ b/include/linux/mm-arch-hooks.h
> @@ -22,4 +22,11 @@ static inline void arch_remap(struct mm_struct *mm,
>  #define arch_remap arch_remap
>  #endif
>  
> +#ifndef arch_faultaround_pte_mkold
> +static inline void arch_faultaround_pte_mkold(struct vm_fault *vmf)
> +{
> +}
> +#define arch_faultaround_pte_mkold arch_faultaround_pte_mkold
> +#endif
> +
>  #endif /* _LINUX_MM_ARCH_HOOKS_H */
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7661156..be689a0 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -302,6 +302,7 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *,
>  #define FAULT_FLAG_USER		0x40	/* The fault originated in userspace */
>  #define FAULT_FLAG_REMOTE	0x80	/* faulting for non current tsk/mm */
>  #define FAULT_FLAG_INSTRUCTION  0x100	/* The fault was during an instruction fetch */
> +#define FAULT_FLAG_MKOLD	0x200	/* Make faultaround ptes old */

Nit: Can we make this FAULT_FLAG_PREFAULT_OLD or something like that so
that it is clear from the flag name that this is about prefaulting of
pages?

>  #define FAULT_FLAG_TRACE \
>  	{ FAULT_FLAG_WRITE,		"WRITE" }, \
> @@ -330,6 +331,7 @@ struct vm_fault {
>  	gfp_t gfp_mask;			/* gfp mask to be used for allocations */
>  	pgoff_t pgoff;			/* Logical page offset based on vma */
>  	unsigned long address;		/* Faulting virtual address */
> +	unsigned long fault_address;    /* Saved faulting virtual address */

Ugh, so I dislike how you hide the decision about whether the *particular*
PTE should be old or young in the arch code. Sure the arch wants to decide
whether the prefaulted PTEs should be old or young and that it has to tell
us but the arch code has no business in checking whether this is prefault
or a normal fault - that decision belongs to filemap_map_pages(). So I'd do
in filemap_map_pages() something like:

	if (iter.index > start_pgoff && arch_wants_old_faultaround_pte())
		vmf->flags |= FAULT_FLAG_PREFAULT_OLD;

And then there's no need for new fault_address in vm_fault or messing with
addresses and fault flags in arch specific code.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2017-11-28  9:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-28  5:07 [PATCH 1/2] mm: make faultaround produce old ptes Vinayak Menon
2017-11-28  5:07 ` [PATCH 2/2] arm64: add faultaround mm hook Vinayak Menon
2017-11-28  9:12 ` Jan Kara [this message]
2017-11-29  5:03   ` [PATCH 1/2] mm: make faultaround produce old ptes Vinayak Menon
2017-11-28 19:45 ` Linus Torvalds
2017-11-29  6:05   ` Vinayak Menon
2017-11-29 10:51     ` Will Deacon
2017-11-29 11:24       ` Vinayak Menon
2017-11-29 17:37       ` Linus Torvalds
2017-12-05 12:16   ` Catalin Marinas
2017-12-05 16:39     ` Linus Torvalds
2017-12-11  7:01     ` Vinayak Menon

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=20171128091234.GH5977@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=minchan@kernel.org \
    --cc=riel@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vinmenon@codeaurora.org \
    --cc=will.deacon@arm.com \
    --cc=ying.huang@intel.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).