| Message ID | 20250310172318.653630-2-sj@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/madvise: batch tlb flushes for MADV_DONTNEED and MADV_FREE | expand |
On Mon, Mar 10, 2025 at 10:23:10AM -0700, SeongJae Park wrote: > To reduce redundant open-coded checks of CONFIG_MEMORY_FAILURE and > MADV_{HWPOISON,SOFT_OFFLINE} in madvise_[un]lock(), is_memory_failure() > has introduced. madvise_do_behavior() is still doing the same NIT: 'is' introduced. > open-coded check, though. Use is_memory_failure() instead. > > To avoid build failure on !CONFIG_MEMORY_FAILURE case, implement an > empty madvise_inject_error() under the config. Also move the definition > of is_memory_failure() inside #ifdef CONFIG_MEMORY_FAILURE clause for > madvise_inject_error() definition, to reduce duplicated ifdef clauses. > > Signed-off-by: SeongJae Park <sj@kernel.org> Good improvement, thanks! Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> > --- > mm/madvise.c | 49 +++++++++++++++++++++++++++---------------------- > 1 file changed, 27 insertions(+), 22 deletions(-) > > diff --git a/mm/madvise.c b/mm/madvise.c > index 388dc289b5d1..c3ab1f283b18 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -1392,7 +1392,32 @@ static int madvise_inject_error(int behavior, > > return 0; > } > -#endif > + > +static bool is_memory_failure(int behavior) > +{ > + switch (behavior) { > + case MADV_HWPOISON: > + case MADV_SOFT_OFFLINE: > + return true; > + default: > + return false; > + } > +} > + > +#else > + > +static int madvise_inject_error(int behavior, > + unsigned long start, unsigned long end) > +{ > + return 0; > +} > + > +static bool is_memory_failure(int behavior) > +{ > + return false; > +} > + > +#endif /* CONFIG_MEMORY_FAILURE */ > > static bool > madvise_behavior_valid(int behavior) > @@ -1569,24 +1594,6 @@ int madvise_set_anon_name(struct mm_struct *mm, unsigned long start, > } > #endif /* CONFIG_ANON_VMA_NAME */ > > -#ifdef CONFIG_MEMORY_FAILURE > -static bool is_memory_failure(int behavior) > -{ > - switch (behavior) { > - case MADV_HWPOISON: > - case MADV_SOFT_OFFLINE: > - return true; > - default: > - return false; > - } > -} > -#else > -static bool is_memory_failure(int behavior) > -{ > - return false; > -} > -#endif > - > static int madvise_lock(struct mm_struct *mm, int behavior) > { > if (is_memory_failure(behavior)) > @@ -1640,10 +1647,8 @@ static int madvise_do_behavior(struct mm_struct *mm, > unsigned long end; > int error; > > -#ifdef CONFIG_MEMORY_FAILURE > - if (behavior == MADV_HWPOISON || behavior == MADV_SOFT_OFFLINE) > + if (is_memory_failure(behavior)) > return madvise_inject_error(behavior, start, start + len_in); > -#endif > start = untagged_addr_remote(mm, start); > end = start + len; > > -- > 2.39.5
diff --git a/mm/madvise.c b/mm/madvise.c index 388dc289b5d1..c3ab1f283b18 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -1392,7 +1392,32 @@ static int madvise_inject_error(int behavior, return 0; } -#endif + +static bool is_memory_failure(int behavior) +{ + switch (behavior) { + case MADV_HWPOISON: + case MADV_SOFT_OFFLINE: + return true; + default: + return false; + } +} + +#else + +static int madvise_inject_error(int behavior, + unsigned long start, unsigned long end) +{ + return 0; +} + +static bool is_memory_failure(int behavior) +{ + return false; +} + +#endif /* CONFIG_MEMORY_FAILURE */ static bool madvise_behavior_valid(int behavior) @@ -1569,24 +1594,6 @@ int madvise_set_anon_name(struct mm_struct *mm, unsigned long start, } #endif /* CONFIG_ANON_VMA_NAME */ -#ifdef CONFIG_MEMORY_FAILURE -static bool is_memory_failure(int behavior) -{ - switch (behavior) { - case MADV_HWPOISON: - case MADV_SOFT_OFFLINE: - return true; - default: - return false; - } -} -#else -static bool is_memory_failure(int behavior) -{ - return false; -} -#endif - static int madvise_lock(struct mm_struct *mm, int behavior) { if (is_memory_failure(behavior)) @@ -1640,10 +1647,8 @@ static int madvise_do_behavior(struct mm_struct *mm, unsigned long end; int error; -#ifdef CONFIG_MEMORY_FAILURE - if (behavior == MADV_HWPOISON || behavior == MADV_SOFT_OFFLINE) + if (is_memory_failure(behavior)) return madvise_inject_error(behavior, start, start + len_in); -#endif start = untagged_addr_remote(mm, start); end = start + len;
To reduce redundant open-coded checks of CONFIG_MEMORY_FAILURE and MADV_{HWPOISON,SOFT_OFFLINE} in madvise_[un]lock(), is_memory_failure() has introduced. madvise_do_behavior() is still doing the same open-coded check, though. Use is_memory_failure() instead. To avoid build failure on !CONFIG_MEMORY_FAILURE case, implement an empty madvise_inject_error() under the config. Also move the definition of is_memory_failure() inside #ifdef CONFIG_MEMORY_FAILURE clause for madvise_inject_error() definition, to reduce duplicated ifdef clauses. Signed-off-by: SeongJae Park <sj@kernel.org> --- mm/madvise.c | 49 +++++++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 22 deletions(-)