Re: [PATCH v7 3/5] mm/hugetlb: add support for mempolicy MPOL_PREFERRED_MANY
From: Hugh Dickins <hughd@google.com>
Date: 2021-08-10 21:35:21
Also in:
linux-api, lkml
On Tue, 10 Aug 2021, Feng Tang wrote:
On Mon, Aug 09, 2021 at 03:19:32PM +0200, Michal Hocko wrote: [snip]quoted
quoted
quoted
Do you think you can provide same helpers for other policies as well? Maybe we can get rid of some other ifdefery as well.Sure. I can make separate patch(es) for that. And you mean helper like mpol_is_bind/default/local/preferred? I just run 'git-grep MPOL', and for places using "mode == MPOL_XXX", mostly they are in mempolicy.[ch], the only another place is in shmem.c, do we need to create all the helpers for it and the potential future users?I would just go with those instances which need to ifdef for NUMA. Thanks!Yes, following is a patch to remove one CONFIG_NUMA check, though an bolder idea to extend the patch by removing the CONFIG_TMPFS check in the same line. Thanks, Feng ---------8<--------------------------------- From 1a5858721ac8ce99c27c13d310bba2983dc73d97 Mon Sep 17 00:00:00 2001 From: Feng Tang <redacted> Date: Tue, 10 Aug 2021 17:00:59 +0800 Subject: [PATCH] mm: shmem: avoid open coded check for mempolicy's mode Add a mempolicy helper to do the check, which can also remove a CONFIG_NUMA option check. Suggested-by: Michal Hocko <mhocko@suse.com> Signed-off-by: Feng Tang <redacted>
No thanks: this is not an improvement. The "#if defined(CONFIG_NUMA) && defined(CONFIG_TMPFS)" is there to eliminate dead code that would not be automatically eliminated by the optimizer: it's not there just to avoid MPOL_DEFAULT, and it's there to cover shmem_get_sbmpol() along with shmem_show_mpol(). I know we tend to avoid #ifdefs in .c files, and that's good; and I know you could find other code in mm/shmem.c which might also be #ifdef'ed to eliminate other dead code in other configs; but unless there's a new drive to purge our .c source of all #ifdefs, please just leave this as is. Thanks, Hugh
quoted hunk ↗ jump to hunk
--- include/linux/mempolicy.h | 14 ++++++++++++++ mm/shmem.c | 8 ++++---- 2 files changed, 18 insertions(+), 4 deletions(-)diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h index 60d5e6c3340c..8fc518ad4f3c 100644 --- a/include/linux/mempolicy.h +++ b/include/linux/mempolicy.h@@ -192,6 +192,10 @@ static inline bool mpol_is_preferred_many(struct mempolicy *pol) return (pol->mode == MPOL_PREFERRED_MANY); } +static inline bool mpol_is_default(struct mempolicy *pol) +{ + return (pol->mode == MPOL_DEFAULT); +} #else@@ -287,6 +291,10 @@ static inline int mpol_parse_str(char *str, struct mempolicy **mpol) } #endif +static inline void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol) +{ +} + static inline int mpol_misplaced(struct page *page, struct vm_area_struct *vma, unsigned long address) {@@ -309,5 +317,11 @@ static inline bool mpol_is_preferred_many(struct mempolicy *pol) return false; } +static inline bool mpol_is_default(struct mempolicy *pol) +{ + return false; +} + + #endif /* CONFIG_NUMA */ #endifdiff --git a/mm/shmem.c b/mm/shmem.c index 96f05f6af8bb..26b195209ef7 100644 --- a/mm/shmem.c +++ b/mm/shmem.c@@ -1437,12 +1437,12 @@ static int shmem_writepage(struct page *page, struct writeback_control *wbc) return 0; } -#if defined(CONFIG_NUMA) && defined(CONFIG_TMPFS) +#ifdef CONFIG_TMPFS static void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol) { char buffer[64]; - if (!mpol || mpol->mode == MPOL_DEFAULT) + if (!mpol || mpol_is_default(mpol)) return; /* show nothing */ mpol_to_str(buffer, sizeof(buffer), mpol);@@ -1461,7 +1461,7 @@ static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo) } return mpol; } -#else /* !CONFIG_NUMA || !CONFIG_TMPFS */ +#else /* !CONFIG_TMPFS */ static inline void shmem_show_mpol(struct seq_file *seq, struct mempolicy *mpol) { }@@ -1469,7 +1469,7 @@ static inline struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo) { return NULL; } -#endif /* CONFIG_NUMA && CONFIG_TMPFS */ +#endif /* CONFIG_TMPFS */ #ifndef CONFIG_NUMA #define vm_policy vm_private_data #endif-- 2.14.1