[PATCH 0/8] Style and small fixes for core-scheduling

[PATCH 0/8] Style and small fixes for core-scheduling

[John B. Wyatt IV]

Patchset of style and small fixes for the 8th iteration of the
Core-Scheduling feature.

Style fixes include changing spaces to tabs, inserting new lines before
declarations, removing unused braces, and spelling.

Two small fixes involving changing a main() to main(void) and removing an
unused 'else'.

All issues were reported by checkpatch.

I am a new Linux kernel developer interning with the Outreachy project.

Please feel free to advise on any corrections or improvements that can be
made.

John B. Wyatt IV (8):
  sched: Correct misspellings in core-scheduling.rst
  sched: Fix bad function definition
  sched: Fix some style issues in test_coresched.c
  sched: Remove unused else
  sched: Add newline after declaration
  sched: Remove unneeded braces
  sched: Replace spaces with tabs
  sched: Add newlines after declarations

 Documentation/admin-guide/hw-vuln/core-scheduling.rst | 8 ++++----
 arch/x86/include/asm/thread_info.h                    | 4 ++--
 kernel/sched/core.c                                   | 6 ++++--
 kernel/sched/coretag.c                                | 3 ++-
 tools/testing/selftests/sched/test_coresched.c        | 8 ++++----
 5 files changed, 16 insertions(+), 13 deletions(-)

-- 
2.28.0

[PATCH 1/8] sched: Correct misspellings in core-scheduling.rst

[John B. Wyatt IV]

'priorty', 'guarenteed', 'guarentee' should be: priority, guaranteed,
guarantee.

Issue reported by checkpatch.

Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com>
---
 Documentation/admin-guide/hw-vuln/core-scheduling.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/hw-vuln/core-scheduling.rst b/Documentation/admin-guide/hw-vuln/core-scheduling.rst
index eacafbb8fa3f..638d0f3c1c09 100644
--- a/Documentation/admin-guide/hw-vuln/core-scheduling.rst
+++ b/Documentation/admin-guide/hw-vuln/core-scheduling.rst
@@ -161,7 +161,7 @@ priority task is not trusted with respect to the core wide highest priority
 task.  If a sibling does not have a trusted task to run, it will be forced idle
 by the scheduler(idle thread is scheduled to run).
 
-When the highest priorty task is selected to run, a reschedule-IPI is sent to
+When the highest priority task is selected to run, a reschedule-IPI is sent to
 the sibling to force it into idle. This results in 4 cases which need to be
 considered depending on whether a VM or a regular usermode process was running
 on either HT::
@@ -223,9 +223,9 @@ Also this does nothing about syscall entries.
 3. Kernel Address Space Isolation
 #################################
 System calls could run in a much restricted address space which is
-guarenteed not to leak any sensitive data. There are practical limitation in
+guaranteed not to leak any sensitive data. There are practical limitation in
 implementing this - the main concern being how to decide on an address space
-that is guarenteed to not have any sensitive data.
+that is guaranteed to not have any sensitive data.
 
 4. Limited cookie-based protection
 ##################################
@@ -251,7 +251,7 @@ outside. Tasks outside the group also don't trust tasks within.
 
 Limitations
 -----------
-Core scheduling tries to guarentee that only trusted tasks run concurrently on a
+Core scheduling tries to guarantee that only trusted tasks run concurrently on a
 core. But there could be small window of time during which untrusted tasks run
 concurrently or kernel could be running concurrently with a task not trusted by
 kernel.
-- 
2.28.0

[PATCH 2/8] sched: Fix bad function definition

[John B. Wyatt IV]

main() should have a void.

Issue reported by checkpatch.

Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com>
---
 tools/testing/selftests/sched/test_coresched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/sched/test_coresched.c b/tools/testing/selftests/sched/test_coresched.c
index 2fdefb843115..91cfb00f15b5 100644
--- a/tools/testing/selftests/sched/test_coresched.c
+++ b/tools/testing/selftests/sched/test_coresched.c
@@ -826,7 +826,7 @@ static void test_prctl_in_group(char *root)
     print_pass();
 }
 
-int main() {
+int main(void) {
     char *root = make_group(NULL, NULL);
 
     test_cgroup_parent_tag_child_inherit(root);
-- 
2.28.0

[PATCH 3/8] sched: Fix some style issues in test_coresched.c

[John B. Wyatt IV]

Line 825: open brace '{' following function definitions go
on the next line.
Line 459: that open brace { should be on the previous line
Line 459: space required before the open parenthesis '('

Issues reported by checkpatch.

There are other issues including over a hundred instances of using spaces
instead of tabs in this file.
I am currently fixing these specific issues in this patch.

Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com>
---
 tools/testing/selftests/sched/test_coresched.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/sched/test_coresched.c b/tools/testing/selftests/sched/test_coresched.c
index 91cfb00f15b5..f11ed8370c07 100644
--- a/tools/testing/selftests/sched/test_coresched.c
+++ b/tools/testing/selftests/sched/test_coresched.c
@@ -459,9 +459,8 @@ char *get_task_core_cookie(char *pid)
     sprintf(proc_path, "/proc/%s/sched", pid);
 
     fp = fopen(proc_path, "r");
-    while ((fgets(line, 1024, fp)) != NULL)
-    {
-        if(!strstr(line, "core_cookie"))
+    while ((fgets(line, 1024, fp)) != NULL) {
+        if (!strstr(line, "core_cookie"))
             continue;
 
         for (j = 0, i = 0; i < 1024 && line[i] != '\0'; i++)
@@ -826,7 +825,8 @@ static void test_prctl_in_group(char *root)
     print_pass();
 }
 
-int main(void) {
+int main(void)
+{
     char *root = make_group(NULL, NULL);
 
     test_cgroup_parent_tag_child_inherit(root);
-- 
2.28.0

[PATCH 4/8] sched: Remove unused else

[John B. Wyatt IV]

Silences suspect code warning.

Issue reported by checkpatch.

Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com>
---
 kernel/sched/coretag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/coretag.c b/kernel/sched/coretag.c
index 3333c9b0afc5..11139dbed648 100644
--- a/kernel/sched/coretag.c
+++ b/kernel/sched/coretag.c
@@ -146,7 +146,7 @@ int sched_core_share_tasks(struct task_struct *t1, struct task_struct *t2)
 		}
 		wr.tasks[0] = t1;
 		wr.cookies[0] = cookie;
-	} else
+	}
 	/*
 	 * 		t1		joining		t2
 	 * CASE 1:
-- 
2.28.0

[PATCH 5/8] sched: Add newline after declaration

[John B. Wyatt IV]

Silences checkpatch warning.

Issue reported by checkpatch.

Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com>
---
 kernel/sched/coretag.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sched/coretag.c b/kernel/sched/coretag.c
index 11139dbed648..6d17d11cb46f 100644
--- a/kernel/sched/coretag.c
+++ b/kernel/sched/coretag.c
@@ -287,6 +287,7 @@ unsigned long cpu_core_get_group_cookie(struct task_group *tg)
 
 		if (tg->core_tagged) {
 			unsigned long cookie = ((unsigned long)tg << 8) | color;
+
 			cookie &= SCHED_CORE_GROUP_COOKIE_MASK;
 			return cookie;
 		}
-- 
2.28.0

[PATCH 6/8] sched: Remove unneeded braces

[John B. Wyatt IV]

Remove unneeded braces and the spaces for the bottom brace to silence
checkpatch warnings.

Issue reported by checkpatch.

Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com>
---
 kernel/sched/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 61758b5478d8..7bf20110fdf5 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3588,9 +3588,9 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p)
 	if (current->core_task_cookie) {
 
 		/* If it is not CLONE_THREAD fork, assign a unique per-task tag. */
-		if (!(clone_flags & CLONE_THREAD)) {
+		if (!(clone_flags & CLONE_THREAD))
 			return sched_core_share_tasks(p, p);
-               }
+
 		/* Otherwise share the parent's per-task tag. */
 		return sched_core_share_tasks(p, current);
 	}
-- 
2.28.0

[PATCH 7/8] sched: Replace spaces with tabs

[John B. Wyatt IV]

Matches the convention of the surrounding code.

Issue reported by checkpatch.

Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com>
---
 arch/x86/include/asm/thread_info.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 45b6dbdf116e..c29b62ff7701 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -99,7 +99,7 @@ struct thread_info {
 #define TIF_SPEC_FORCE_UPDATE	23	/* Force speculation MSR update in context switch */
 #define TIF_FORCED_TF		24	/* true if TF in eflags artificially */
 #define TIF_BLOCKSTEP		25	/* set when we want DEBUGCTLMSR_BTF */
-#define TIF_UNSAFE_RET   	26	/* On return to process/guest, perform safety checks. */
+#define TIF_UNSAFE_RET		26	/* On return to process/guest, perform safety checks. */
 #define TIF_LAZY_MMU_UPDATES	27	/* task is updating the mmu lazily */
 #define TIF_SYSCALL_TRACEPOINT	28	/* syscall tracepoint instrumentation */
 #define TIF_ADDR32		29	/* 32-bit address space on 64 bits */
@@ -130,7 +130,7 @@ struct thread_info {
 #define _TIF_SPEC_FORCE_UPDATE	(1 << TIF_SPEC_FORCE_UPDATE)
 #define _TIF_FORCED_TF		(1 << TIF_FORCED_TF)
 #define _TIF_BLOCKSTEP		(1 << TIF_BLOCKSTEP)
-#define _TIF_UNSAFE_RET 	(1 << TIF_UNSAFE_RET)
+#define _TIF_UNSAFE_RET		(1 << TIF_UNSAFE_RET)
 #define _TIF_LAZY_MMU_UPDATES	(1 << TIF_LAZY_MMU_UPDATES)
 #define _TIF_SYSCALL_TRACEPOINT	(1 << TIF_SYSCALL_TRACEPOINT)
 #define _TIF_ADDR32		(1 << TIF_ADDR32)
-- 
2.28.0

[PATCH 8/8] sched: Add newlines after declarations

[John B. Wyatt IV]

Silences checkpatch warnings.

Issue reported by checkpatch.

Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com>
---
 kernel/sched/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 7bf20110fdf5..b38cdb54ec81 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5034,6 +5034,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
 	if (!fi_before) {
 		for_each_cpu(i, smt_mask) {
 			struct rq *rq_i = cpu_rq(i);
+
 			rq_i->cfs.min_vruntime_fi = rq_i->cfs.min_vruntime;
 		}
 	}
@@ -5200,6 +5201,7 @@ next_class:;
 	if (!fi_before && rq->core->core_forceidle) {
 		for_each_cpu(i, smt_mask) {
 			struct rq *rq_i = cpu_rq(i);
+
 			rq_i->cfs.min_vruntime_fi = rq_i->cfs.min_vruntime;
 		}
 	}
-- 
2.28.0

Re: [PATCH 0/8] Style and small fixes for core-scheduling

[Phil Auld]

Hi John,

On Wed, Oct 28, 2020 at 05:19:09AM -0700 John B. Wyatt IV wrote:
> Patchset of style and small fixes for the 8th iteration of the
> Core-Scheduling feature.
> 
> Style fixes include changing spaces to tabs, inserting new lines before
> declarations, removing unused braces, and spelling.
> 
> Two small fixes involving changing a main() to main(void) and removing an
> unused 'else'.
> 
> All issues were reported by checkpatch.
> 
> I am a new Linux kernel developer interning with the Outreachy project.
>

Welcome!

> Please feel free to advise on any corrections or improvements that can be
> made.

Thanks for these. I wonder, though, if it would not make more sense
to post these changes as comments on the original as-yet-unmerged
patches that you are fixing up? 


Cheers,
Phil

> 
> John B. Wyatt IV (8):
>   sched: Correct misspellings in core-scheduling.rst
>   sched: Fix bad function definition
>   sched: Fix some style issues in test_coresched.c
>   sched: Remove unused else
>   sched: Add newline after declaration
>   sched: Remove unneeded braces
>   sched: Replace spaces with tabs
>   sched: Add newlines after declarations
> 
>  Documentation/admin-guide/hw-vuln/core-scheduling.rst | 8 ++++----
>  arch/x86/include/asm/thread_info.h                    | 4 ++--
>  kernel/sched/core.c                                   | 6 ++++--
>  kernel/sched/coretag.c                                | 3 ++-
>  tools/testing/selftests/sched/test_coresched.c        | 8 ++++----
>  5 files changed, 16 insertions(+), 13 deletions(-)
> 
> -- 
> 2.28.0
> 

-- 

Re: [PATCH 0/8] Style and small fixes for core-scheduling

[John Wyatt]

Hi Phil,

> Hi John,
> 
> Welcome!

Thank you!

> 
> > Please feel free to advise on any corrections or improvements that
> > can be
> > made.
> 
> Thanks for these. I wonder, though, if it would not make more sense
> to post these changes as comments on the original as-yet-unmerged
> patches that you are fixing up? 

Patch 23/26 has a lot of spaces to tabs issues that are isolated to one
file (and one patch if I read it correctly). It would definitely make
sense for those to be a comment. Especially with patch 23 being a later
patch in the patch series. Most of those issues are not fixed by this
patch series I submitted.

For the rest, I am willing to do resubmit as comments if that would be
easier for the submitter.

Please let me know if that is how I should resubmit them.

Being a patch set does makes them easier to verify. Some warnings
against the individual patches, like braces surrounding a one line in a
if block, disappear with later patches as the code is added. For
clarity for the mailing list, I used the jfern tree with the
'coresched-v8-posted-to-list'.

https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/tag/?h=coresched-v8-posted-to-list

Thank you for your feedback.

-- 
Sincerely,
John Wyatt