diff mbox series

[7/9] mm/compaction: Get rid of RT ifdeffery

Message ID 20220817162703.728679-8-bigeasy@linutronix.de (mailing list archive)
State New
Headers show
Series [1/9] slub: Make PREEMPT_RT support less convoluted | expand

Commit Message

Sebastian Andrzej Siewior Aug. 17, 2022, 4:27 p.m. UTC 
From: Thomas Gleixner <tglx@linutronix.de>

Move the RT dependency for the initial value of
sysctl_compact_unevictable_allowed into Kconfig.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Nick Terrell <terrelln@fb.com>
Cc: linux-mm@kvack.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 mm/Kconfig      | 5 +++++
 mm/compaction.c | 6 +-----
 2 files changed, 6 insertions(+), 5 deletions(-)

Comments

Rasmus Villemoes Aug. 18, 2022, 8:55 a.m. UTC | #1 
On 17/08/2022 18.27, Sebastian Andrzej Siewior wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Move the RT dependency for the initial value of
> sysctl_compact_unevictable_allowed into Kconfig.
> 
>  
> +config COMPACT_UNEVICTABLE_DEFAULT
> +	int
> +	default 0 if PREEMPT_RT
> +	default 1
> +
>  #
>  # support for free page reporting
>  config PAGE_REPORTING
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 640fa76228dd9..10561cb1aaad9 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1727,11 +1727,7 @@ typedef enum {
>   * Allow userspace to control policy on scanning the unevictable LRU for
>   * compactable pages.
>   */
> -#ifdef CONFIG_PREEMPT_RT
> -int sysctl_compact_unevictable_allowed __read_mostly = 0;
> -#else
> -int sysctl_compact_unevictable_allowed __read_mostly = 1;
> -#endif
> +int sysctl_compact_unevictable_allowed __read_mostly = CONFIG_COMPACT_UNEVICTABLE_DEFAULT;

Why introduce a Kconfig symbol for this, and not just spell the
initializer "IS_ENABLED(CONFIG_PREEMPT_RT) ? 0 : 1" or simply
"!IS_ENABLED(CONFIG_PREEMPT_RT)"?

And if you do keep it in Kconfig, shouldn't the symbol be "depends on
COMPACTION" so it doesn't needlessly appear in .config when
!CONFIG_COMPACTION.

Rasmus
Sebastian Andrzej Siewior Aug. 18, 2022, 3:51 p.m. UTC | #2 
On 2022-08-18 10:55:28 [+0200], Rasmus Villemoes wrote:
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -1727,11 +1727,7 @@ typedef enum {
> >   * Allow userspace to control policy on scanning the unevictable LRU for
> >   * compactable pages.
> >   */
> > -#ifdef CONFIG_PREEMPT_RT
> > -int sysctl_compact_unevictable_allowed __read_mostly = 0;
> > -#else
> > -int sysctl_compact_unevictable_allowed __read_mostly = 1;
> > -#endif
> > +int sysctl_compact_unevictable_allowed __read_mostly = CONFIG_COMPACT_UNEVICTABLE_DEFAULT;
> 
> Why introduce a Kconfig symbol for this, and not just spell the
> initializer "IS_ENABLED(CONFIG_PREEMPT_RT) ? 0 : 1" or simply
> "!IS_ENABLED(CONFIG_PREEMPT_RT)"?

The idea was to remove the CONFIG_PREEMPT_RT. However if this IS_ENABLED
is preferred, we can certainly do this.

> And if you do keep it in Kconfig, shouldn't the symbol be "depends on
> COMPACTION" so it doesn't needlessly appear in .config when
> !CONFIG_COMPACTION.

Sure, if we keep the Kconfig.

> Rasmus

Sebastian
Thomas Gleixner Aug. 24, 2022, 1:50 p.m. UTC | #3 
On Thu, Aug 18 2022 at 10:55, Rasmus Villemoes wrote:
> On 17/08/2022 18.27, Sebastian Andrzej Siewior wrote:
>> -#ifdef CONFIG_PREEMPT_RT
>> -int sysctl_compact_unevictable_allowed __read_mostly = 0;
>> -#else
>> -int sysctl_compact_unevictable_allowed __read_mostly = 1;
>> -#endif
>> +int sysctl_compact_unevictable_allowed __read_mostly = CONFIG_COMPACT_UNEVICTABLE_DEFAULT;
>
> Why introduce a Kconfig symbol for this, and not just spell the
> initializer "IS_ENABLED(CONFIG_PREEMPT_RT) ? 0 : 1" or simply
> "!IS_ENABLED(CONFIG_PREEMPT_RT)"?

The reason for the config symbol is that Linus requested to have
semantically obvious constructs which can be utilized even without RT
and clearly spell out what the construct does. When RT selects this then
it's a documented requirement/dependency.

> And if you do keep it in Kconfig, shouldn't the symbol be "depends on
> COMPACTION" so it doesn't needlessly appear in .config when
> !CONFIG_COMPACTION.

Sure.

Thanks,

        tglx
diff mbox series

Patch

diff --git a/mm/Kconfig b/mm/Kconfig
index 0331f1461f81c..a0506a54a4f3f 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -579,6 +579,11 @@  config COMPACTION
 	  it and then we would be really interested to hear about that at
 	  linux-mm@kvack.org.
 
+config COMPACT_UNEVICTABLE_DEFAULT
+	int
+	default 0 if PREEMPT_RT
+	default 1
+
 #
 # support for free page reporting
 config PAGE_REPORTING
diff --git a/mm/compaction.c b/mm/compaction.c
index 640fa76228dd9..10561cb1aaad9 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1727,11 +1727,7 @@  typedef enum {
  * Allow userspace to control policy on scanning the unevictable LRU for
  * compactable pages.
  */
-#ifdef CONFIG_PREEMPT_RT
-int sysctl_compact_unevictable_allowed __read_mostly = 0;
-#else
-int sysctl_compact_unevictable_allowed __read_mostly = 1;
-#endif
+int sysctl_compact_unevictable_allowed __read_mostly = CONFIG_COMPACT_UNEVICTABLE_DEFAULT;
 
 static inline void
 update_fast_start_pfn(struct compact_control *cc, unsigned long pfn)