Kernel doesn't boot with atomic_flags

Havadebo · 03-24-2018, 06:06 AM

Setup:
Cross compiling linux kernel 3.2.x for MIPS(little endian) with GCC 7.3.0 and C++11 as default.

Since kernel 3.2.94 the kernel settopbox(dm8000) doesn't boot.

I figured out, commit:
https://git.kernel.org/pub/scm/linux...72f33def17b7ef

Was the cause of the problem.
So just reverting the commit (and the next one, due to that) , and the box boot fine.

But the question remains, why the kernel doesn't boot due to adding

Code:

unsigned long atomic_flags; /* Flags needing atomic access. */

to 'struct task_struct'?

Can somone explain what's going wrong here?
I tried a lot, but no luck.

Mara · 03-27-2018, 03:12 PM

Is it the generic kernel you're using or there are vendor patches too? It seems to me that you may have something that depends on the size of task_struct and/or positions of the fields inside.

Havadebo · 03-29-2018, 05:35 AM

Thanks for reply.
Yes there are a lot of vendor patches to support hardware, fix compilation, support gcc 7 etc. But none of these patches touches 'task_struct', they even don't touch the linux/include/sched.h file.
The patches do also nothing with atomic_flags.

As test i added bit fields, thus 'unsigned long atomic_flags: 32;'.
That doesn't compile of course with the macro's, but the kernel boot.
Even creating a new member e.g. 'unsigned long test;' is no problem.

In the unpatched source code of the kernel(without vendor patches), there are other structures with 'unsigned long atomic_flags', so i assume that a conflict occurs?

I tried to make a dmesg with Putty via RS232, but i only get rubbish.

Anyway..It's not a big deal(i revert the 2 commits), but it's not how it should be.

Mara · 03-30-2018, 11:51 AM

Having multiple structures with atomic_flags shouldn't be an issue. However, you might have a good intuition about it creating the difference. Are you able to see any macros in the vendor patches that do redefine atomic_flags for example?

As I do not have access to the same sources as you, I can only guess what you have in the vendor patches.

Havadebo · 04-02-2018, 06:59 AM

OK i found it, i need to move atomic_flags field to the bottom of the structure.

Code:

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 2d4ab76..074b33b 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1313,8 +1313,6 @@ struct task_struct {
 	unsigned sched_reset_on_fork:1;
 	unsigned sched_contributes_to_load:1;
 
-	unsigned long atomic_flags; /* Flags needing atomic access. */
-
 	pid_t pid;
 	pid_t tgid;
 
@@ -1585,6 +1583,9 @@ struct task_struct {
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
 	atomic_t ptrace_bp_refcnt;
 #endif
+
+	unsigned long atomic_flags; /* Flags needing atomic access. */
+
 };
 
 /* Future-safe accessor for struct task_struct's cpus_allowed. */
--

Correction previous post. I said 'unsigned long test' and 'unsigned log atomic_flags:32' did work, but it wasn't. Sorry about that, my mistake.

With:

Code:

unsigned long atomic_flags:16;

or:

Code:

unsigned short atomic_flags;

the box boot fine. So it's indeed the size of the field.

As base kernel 3.2.0
https://mirrors.edge.kernel.org/pub/...ux-3.2.tar.bz2

Followed by patch level 101.
https://mirrors.edge.kernel.org/pub/...tch-3.2.101.xz

Followed by the vendor patches.
https://ufile.io/xzngm

General bug? Or indeed caused by the vendor patch(in that case probably 001-linux-dreambox-kernel.patch).
I tested my patch a few days, and no problems. So far so good.

Mara · 04-04-2018, 02:32 PM

This is a very good path. If you have a unsigned int atomic_flags:16 next to the other bitfields it doesn't change anything in the structure - the compiler will pack all the fields together.

I suspect that it might be linked to struct thread_struct thread; You may try to put it before and after. The big patch 001-dreambox includes a change in resume funcion assembly that is accessing certain offsets of this structure. If they move, the function may work incorrectly. You may test this theory out.

Havadebo · 04-08-2018, 04:53 AM

I did the test, and i figured out the 'struct task_cputime cputime_expires' field at line 1367 is the, let's say border.

So with this patch, the box doesn't boot(prior).

Code:

@@ -1364,6 +1362,8 @@ struct task_struct {
 /* mm fault and swap info: this can arguably be seen as either mm-specific or thread-specific */
 	unsigned long min_flt, maj_flt;
 
+	unsigned long atomic_flags; /* Flags needing atomic access. */
 	struct task_cputime cputime_expires;
 	struct list_head cpu_timers[3];

With this patch, the box boot flawlessly(after).

Code:

@@ -1365,6 +1363,7 @@ struct task_struct {
 	unsigned long min_flt, maj_flt;
 
 	struct task_cputime cputime_expires;
+	unsigned long atomic_flags; /* Flags needing atomic access. */
 	struct list_head cpu_timers[3];
 
 /* process credentials */

Eventually i apply this patch.

Code:

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 2d4ab76..074b33b 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1313,8 +1313,6 @@ struct task_struct {
 	unsigned sched_reset_on_fork:1;
 	unsigned sched_contributes_to_load:1;
 
-	unsigned long atomic_flags; /* Flags needing atomic access. */
-
 	pid_t pid;
 	pid_t tgid;
 
@@ -1367,6 +1365,8 @@ struct task_struct {
 	struct task_cputime cputime_expires;
 	struct list_head cpu_timers[3];
 
+	unsigned long atomic_flags; /* Flags needing atomic access. */
+
 /* process credentials */
 	const struct cred __rcu *real_cred; /* objective and real subjective task
 					 * credentials (COW) */

I searched for 'cpu time' in all custom patches, but found nothing.
This patch should do the job.Also no problems so far.