when a ifdef is used in the below manner, second one could be considered as
duplicate.
ifdef DEFINE_A
..code block...
ifdef DEFINE_A <-- This is a duplicate.
..code block...
endif
else
ifndef DEFINE_A <-- This is also duplicate.
..code block...
endif
endif
More details about the script and methods used to find these code
patterns are in cover letter of [1]
In the scheduler code, there are two places where above pattern can be
observed.
Hunk1: Code is under check of CONFIG_UCLAMP_TASK in kernel/sched/core.c
from line 1353. Hence the same check at line 1795 is duplicate.
Hunk2: Minor update of comment.
Hunk3: Code is already under the check of CONFIG_SMP in
kernel/sched/fair.c from line 8591. Hence the same check in line 10185
is a duplicate.
No functional change is intended here. It only aims to improve code
readability.
[1]:https://lore.kernel.org/all/[email protected]/
Signed-off-by: Shrikanth Hegde <[email protected]>
---
Changes since v2:
- Dropped RFC tag.
- Split the patches into individual from the series.
- Added more context to each Hunk for review.
kernel/sched/core.c | 4 +---
kernel/sched/fair.c | 2 --
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9116bcc90346..a76c7095f736 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1792,7 +1792,6 @@ static void cpu_util_update_eff(struct cgroup_subsys_state *css);
#endif
#ifdef CONFIG_SYSCTL
-#ifdef CONFIG_UCLAMP_TASK
#ifdef CONFIG_UCLAMP_TASK_GROUP
static void uclamp_update_root_tg(void)
{
@@ -1898,7 +1897,6 @@ static int sysctl_sched_uclamp_handler(struct ctl_table *table, int write,
return result;
}
#endif
-#endif
static int uclamp_validate(struct task_struct *p,
const struct sched_attr *attr)
@@ -2065,7 +2063,7 @@ static void __init init_uclamp(void)
}
}
-#else /* CONFIG_UCLAMP_TASK */
+#else /* !CONFIG_UCLAMP_TASK */
static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p) { }
static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p) { }
static inline int uclamp_validate(struct task_struct *p,
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 533547e3c90a..8e30e2bb77a0 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10182,10 +10182,8 @@ static int idle_cpu_without(int cpu, struct task_struct *p)
* be computed and tested before calling idle_cpu_without().
*/
-#ifdef CONFIG_SMP
if (rq->ttwu_pending)
return 0;
-#endif
return 1;
}
--
2.39.3
On Fri, 16 Feb 2024 at 07:14, Shrikanth Hegde <[email protected]> wrote:
>
> when a ifdef is used in the below manner, second one could be considered as
> duplicate.
>
> ifdef DEFINE_A
> ...code block...
> ifdef DEFINE_A <-- This is a duplicate.
> ...code block...
> endif
> else
> ifndef DEFINE_A <-- This is also duplicate.
> ...code block...
> endif
> endif
> More details about the script and methods used to find these code
> patterns are in cover letter of [1]
>
> In the scheduler code, there are two places where above pattern can be
> observed.
> Hunk1: Code is under check of CONFIG_UCLAMP_TASK in kernel/sched/core.c
> from line 1353. Hence the same check at line 1795 is duplicate.
> Hunk2: Minor update of comment.
> Hunk3: Code is already under the check of CONFIG_SMP in
> kernel/sched/fair.c from line 8591. Hence the same check in line 10185
> is a duplicate.
>
> No functional change is intended here. It only aims to improve code
> readability.
>
> [1]:https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Shrikanth Hegde <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
> ---
> Changes since v2:
> - Dropped RFC tag.
> - Split the patches into individual from the series.
> - Added more context to each Hunk for review.
>
> kernel/sched/core.c | 4 +---
> kernel/sched/fair.c | 2 --
> 2 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 9116bcc90346..a76c7095f736 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1792,7 +1792,6 @@ static void cpu_util_update_eff(struct cgroup_subsys_state *css);
> #endif
>
> #ifdef CONFIG_SYSCTL
> -#ifdef CONFIG_UCLAMP_TASK
> #ifdef CONFIG_UCLAMP_TASK_GROUP
> static void uclamp_update_root_tg(void)
> {
> @@ -1898,7 +1897,6 @@ static int sysctl_sched_uclamp_handler(struct ctl_table *table, int write,
> return result;
> }
> #endif
> -#endif
>
> static int uclamp_validate(struct task_struct *p,
> const struct sched_attr *attr)
> @@ -2065,7 +2063,7 @@ static void __init init_uclamp(void)
> }
> }
>
> -#else /* CONFIG_UCLAMP_TASK */
> +#else /* !CONFIG_UCLAMP_TASK */
> static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p) { }
> static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p) { }
> static inline int uclamp_validate(struct task_struct *p,
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 533547e3c90a..8e30e2bb77a0 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10182,10 +10182,8 @@ static int idle_cpu_without(int cpu, struct task_struct *p)
> * be computed and tested before calling idle_cpu_without().
> */
>
> -#ifdef CONFIG_SMP
> if (rq->ttwu_pending)
> return 0;
> -#endif
>
> return 1;
> }
> --
> 2.39.3
>
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 8cec3dd9e5930c82c6bd0af3fdb3a36bcd428310
Gitweb: https://git.kernel.org/tip/8cec3dd9e5930c82c6bd0af3fdb3a36bcd428310
Author: Shrikanth Hegde <[email protected]>
AuthorDate: Fri, 16 Feb 2024 11:44:33 +05:30
Committer: Ingo Molnar <[email protected]>
CommitterDate: Fri, 16 Feb 2024 09:37:15 +01:00
sched/core: Simplify code by removing duplicate #ifdefs
There's a few cases of nested #ifdefs in the scheduler code
that can be simplified:
#ifdef DEFINE_A
...code block...
#ifdef DEFINE_A <-- This is a duplicate.
...code block...
#endif
#else
#ifndef DEFINE_A <-- This is also duplicate.
...code block...
#endif
#endif
More details about the script and methods used to find these code
patterns can be found at:
https://lore.kernel.org/all/[email protected]/
No change in functionality intended.
[ mingo: Clarified the changelog. ]
Signed-off-by: Shrikanth Hegde <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/core.c | 4 +---
kernel/sched/fair.c | 2 --
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9116bcc..a76c709 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1792,7 +1792,6 @@ static void cpu_util_update_eff(struct cgroup_subsys_state *css);
#endif
#ifdef CONFIG_SYSCTL
-#ifdef CONFIG_UCLAMP_TASK
#ifdef CONFIG_UCLAMP_TASK_GROUP
static void uclamp_update_root_tg(void)
{
@@ -1898,7 +1897,6 @@ undo:
return result;
}
#endif
-#endif
static int uclamp_validate(struct task_struct *p,
const struct sched_attr *attr)
@@ -2065,7 +2063,7 @@ static void __init init_uclamp(void)
}
}
-#else /* CONFIG_UCLAMP_TASK */
+#else /* !CONFIG_UCLAMP_TASK */
static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p) { }
static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p) { }
static inline int uclamp_validate(struct task_struct *p,
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 533547e..8e30e2b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10182,10 +10182,8 @@ static int idle_cpu_without(int cpu, struct task_struct *p)
* be computed and tested before calling idle_cpu_without().
*/
-#ifdef CONFIG_SMP
if (rq->ttwu_pending)
return 0;
-#endif
return 1;
}