Assignment operation not assigning?!

[foliagecanine]

I've been trying to implement an inter-process communication method in which a program can listen on a port (not related to networking at all, it's sort of like a mailbox identifier) and another program can send data to that port.
Additionally, I've implemented a function that waits for IPC data on either a specified port or any port called waitipc:

Code: Select all

void waitipc(uint32_t port) {
	if (!verify_ipc_port_ownership(port))
		yield();

	dprintf("Port is %u\n", port);

	current_task->state = TASK_STATE_WAITIPC;
	current_task->waitval = port;
	dprintf("Port is %u\n", current_task->waitval);
	yield();
}

The code executes fine until it reaches current_task->waitval = port;
At this point it acts like it assigns the value of port to the current task's waitval, but it actually does NOTHING.
The first dprintf (debug printf, goes to serial) says "Port is 24576" (which is correct as it is supposed to wait for port 0x6000).
The second dprintf says "Port is 0" which is wrong, even though it is set in the line before it!
I've verified this with GDB too.

What could be the cause of this? Is it being optimized out somehow?

(Note: I will try to upload the full code to GitHub soon)

[nullplan]

Is it possible that another process or some interrupt handler is writing to whatever memory you use for "current_task"? Is "current_task" in normal RAM? These are just two pitfalls I can imagine from the top of my head.

You could try to enlist the help of the Debug Registers to find the culprit. Just register "waitport" as a write break point for some task and see where the interrupts are coming from.

[bzt]

There are three things that could go wrong here:

- another task is modifying the current_task struct. As @nullplan said, try to put a write break on it with gdb and see if this is the case
- another task changes the current_task pointer, so it is pointing to a different (potentially empty) struct
- you have caching issues. The paging changed, but cache not flushed. Flushing happens occasionally between the two dprint calls.

Your locking is insufficient. You just check if the lock is acquired, but since your entire waitipc isn't atomic, it can lost ownership after the check and before the function ends. Try something like

Code: Select all

{
  if (!can_acquire_ownership(port))
     yield();
...
  release_lock(port);
  yield();
}

You'll need no caching and special assembly instructions (or LOCK prefix) for those functions if you're using SMP, otherwise CLI / STI might suffice. I recommend to declare the lock variable as volatile too so that the compiler won't optimize it and generates code that always reads its value from memory. Best to use one of the atomic test and set built-ins.

Cheers,
bzt

[foliagecanine]

I figured out what it was and am rather surprised the compiler didn't warn me...
The problem:
waitval was an 8-bit value, I was writing a 16 bit value where the low 8 bits = 0.

I don't know why waitval was 8 bit, since my pid type (which also uses the waitval variable for waitpid) should be 32 bit, but at least I found the error.

Am I missing a compiler warning flag that would have helped me here?

As for locking, this isn't necessary if you don't have multiprocessing (assuming I make sure the interrupts don't interfere), correct?

[thewrongchristian]

As for locking, this isn't necessary if you don't have multiprocessing (assuming I make sure the interrupts don't interfere), correct?

You could design your critical section round disabling interrupts, but you'd be better off hiding that detail, so you can change the details to support SMP without changing client code. Put your critical section lock/unlock in a subroutine, and rely on compiler optimisation to obviate the subroutine overhead.

[Octocontrabass]

Am I missing a compiler warning flag that would have helped me here?

-Wconversion