GCC and SMP problems

[FlashBurn]

I just finished my interrupt for service things in a smp system (like invaliding pages and so on). So I got a few problems with it and with gcc (4.4.2 on cygwin).

You may have a look at my spinlock functions, because I think that maybe from there could come some of my problems:

Code: Select all

struct spinlock_t {
	uint32t volatile lock;
	uint32t flags;
};

static inline void spinAcquire(struct spinlock_t *spinlock) {
	asm volatile("movl $0x1,%%edx\n\t"
				"1:\t testl $0x1,(%%eax)\n\t"
				"jne 1b\n\t"
				"xchg (%%eax),%%edx\n\t"
				"testl $0x1,%%edx\n\t"
				"jne 1b\n\t"
				"pushfl\n\t"
				"popl %[output]\n\t"
				"cli"
				:[output] "=q"(spinlock->flags)
				:"a"(spinlock)
				:"%edx");
}

static inline void spinRelease(struct spinlock_t *spinlock) {
	//look if ints were enabled or disabled
	if(likely((spinlock->flags & 0x200) != 0)) {
		spinlock->lock= 0;
		
		asm volatile("sti");
	} else {
		spinlock->lock= 0;
	}
}

A problem is what gcc makes out of this code:

Code: Select all

static struct smpMsg_t *freeList;
static struct spinlock_t freeLock;

struct smpMsg_t *smpAllocMsg() {
	struct smpMsg_t *msg;
	
	do {		
		spinAcquire(&freeLock);
		
		if(likely(freeList)) {
			msg= freeList;
			freeList= msg->next;
			
			spinRelease(&freeLock);
			
			return msg;
		}
		
		spinRelease(&freeLock);
		
		while(likely(freeList == 0))
			asm volatile("pause");
	} while(1);
}

The 2 lines where I wait for "freeList" becoming non null, gcc makes an endless loop out of it But why?

The next problem seems to be a little funny:

Code: Select all

void smpSendMsgBroadcast(uint32t code, uint32t data1, uint32t data2, uint32t data3) {
	if(likely(msgsAllowed)) {
		struct smpMsg_t *msg= smpAllocMsg();
		struct APIC_Regs_t *apic= (struct APIC_Regs_t *)APIC_ADDR_VIRT;
		
		msg->refCount= numCpus - 1;
		msg->code= code;
		msg->data1= data1;
		msg->data2= data2;
		msg->data3= data3;
		
		spinAcquire(&msgLock);
		
		serviceMsg= msg;
		
		while(unlikely(apic->icr1[0] & APIC_DELIVERY_STS))
			asm volatile("pause");

		apic->icr2[0]= 0;
		apic->icr1[0]= APIC_SERVICE_INT | APIC_DEL_MODE_FIXED | APIC_DEST_SHORT_ALL_EXC_SELF;
		
//		printf("msg send...");
		
		while(unlikely(apic->icr1[0] & APIC_DELIVERY_STS))
			asm volatile("pause");
			
//		printf("waiting...");
		
		while(likely(msg->refCount > 0))
			asm volatile("pause");
			
		printf("ok\n");
		
		serviceMsg= 0;
		
		spinRelease(&msgLock);
		
		smpFreeMsg(msg);
	}
}

If the code is like above it is working, but if I comment out the "printf("ok");", it isn´t working anymore??? I tested this on my real pc, bochs and qemu. I have no idea why should it working with the printf and why shouldn´t it working without it. I mean the same behavior on emulator and real system suggests a coding failure, but I can´t see one.

So if anyone has an idea what I could/should test to find this annoying bug, I would appreciate it !!!

[dosfan]

I see the use of volatile in places but have you tried turning the compiler optimization level down?

[FlashBurn]

Ok, this helped, but it is not a real solution, because I think the code has to work with and without optimizations!

So I think I maybe should compare the assembly outputs and see what gcc does and what isn´t working.

[dosfan]

Compiler over optimization is a common problem.

Basically the compiler tries to optimize the loop and believes it no longer needs to check the while condition because it doesn't "see" anything that modifies the variable!

[FlashBurn]

Yeah, but for what am I using then "volatile"??? I also found out why the code doesn´t work with printf commented out. It optimizes the line "serviceMsg= msg;" away (although I´m using there volatile, too). I don´t know how you see this, but I would say this is definitely a gcc bug.

[dosfan]

I agree in principle. I've suffered the same problems with volatiles to the point where I doubt my understanding of C and gcc.

You could try declaring the actual struct variables as volatile?

This may be of interest: http://www.cs.utah.edu/~regehr/papers/e ... eprint.pdf

[FlashBurn]

I also tested declaring the structs volatile, but this didn´t help. I just filled in a bug report for gcc. I will see what they say about it. Nice paper!

[pcmattman]

You may also be interested to read the atomic builtins that GCC supports, which we use in our Spinlock implementation.

[FlashBurn]

So, problems solved. You have to write the following and then it works:

Code: Select all

static struct smpMsg_t * volatile freeList

So maybe someone could say where should the volatile keyword stay? I always though that "volatile uint32t foo" is alright, isn´t it. Or is this only not right for pointers?

[pcmattman]

You could have a look at this. Pointers have two different uses of the "volatile" keyword with different behaviour (there is a significant difference between "volatile T *" and "T * volatile").

[FlashBurn]

Ok, this article cleared the things up. Thanks! Maybe a Wiki article about this behavior would be good, because I think everyone who wants to make his/her code multithread safe needs this.

[pcmattman]

The Volatile (keyword) article does exist

[FlashBurn]

Maybe I should look more often into the wiki

Edit:: Offtopic:

I don´t know how to help with the wiki articles, but in the article for the local APIC there is a wrong assumption. The msr for the apic base exists from cpu family >= 6, but not on the pentium (family 5). So on a pentium you can´t change the apic base addr. I don´t know where it stands in the intel docs, but if you have a look at the msrs for the cpu families you will see it.

[AJ]

Hi,

See here for how to edit the wiki. Because I don't know enough background about the MSR's on families 5 and 6 off the top of my head, I'll leave the edit to you

Cheers,
Adam

[gravaera]

This was a useful topic: I had always assumed that keywords and storage class keywords applied to the entire declaration/definition, such that

volatile T * t would mean:
t is a volatile pointer to a volatile T.

Had no idea that the compiler would apply the 'volatile' only to the actual data in memory, and not the pointer. Learn something new everyday, I guess.

--Informative read
gravaera