bit fields && data tearing

[Oleg Nesterov <oleg@redhat.com>]

Hello,

I am not sure I should ask here, but since Documentation/memory-barriers.txt
mentions load/store tearing perhaps my question is not completely off-topic...

I am fighting with mysterious RHEL bug, it can be reproduced on ppc and s390
but not on x86. Finally I seem to understand the problem, and I even wrote the
stupid kernel module to ensure, see it below at the end.

It triggers the problem immediately, kt_2() sees the wrong value in freeze_stop.
(If I turn ->freeze_stop int "long", the problem goes away).

So the question is: is this gcc bug or the code below is buggy?

If it is buggy, then probably memory-barriers.txt could mention that you should
be carefull with bit fields, even ACCESS_ONCE() obviously can't help.

Or this just discloses my ignorance and you need at least aligned(long) after a
bit field to be thread-safe ? I thought that compiler should take care and add
the necessary alignment if (say) CPU can't update a single byte/uint.

gcc version 4.4.7 20120313 (Red Hat 4.4.7-9) (GCC). Asm:

	0000000000000000 <.kt_2>:
	   0:	7c 08 02 a6 	mflr    r0
	   4:	fb 81 ff e0 	std     r28,-32(r1)
	   8:	fb a1 ff e8 	std     r29,-24(r1)
	   c:	fb c1 ff f0 	std     r30,-16(r1)
	  10:	fb e1 ff f8 	std     r31,-8(r1)
	  14:	eb c2 00 00 	ld      r30,0(r2)
	  18:	f8 01 00 10 	std     r0,16(r1)
	  1c:	f8 21 ff 71 	stdu    r1,-144(r1)
	  20:	7c 7d 1b 78 	mr      r29,r3
	  24:	3b e0 00 00 	li      r31,0
	  28:	78 3c 04 64 	rldicr  r28,r1,0,49
	  2c:	3b 9c 00 80 	addi    r28,r28,128
	  30:	48 00 00 2c 	b       5c <.kt_2+0x5c>
	  34:	60 00 00 00 	nop
	  38:	60 00 00 00 	nop
	  3c:	60 00 00 00 	nop
	  40:	93 fd 00 04 	stw     r31,4(r29)
	  44:	e8 9d 00 06 	lwa     r4,4(r29)
	  48:	7f 84 f8 00 	cmpw    cr7,r4,r31
	  4c:	40 de 00 4c 	bne-    cr7,98 <.kt_2+0x98>
	  50:	e8 1c 00 00 	ld      r0,0(r28)
	  54:	78 09 f7 e3 	rldicl. r9,r0,62,63
	  58:	40 c2 00 54 	bne-    ac <.kt_2+0xac>
	  5c:	48 00 00 01 	bl      5c <.kt_2+0x5c>
	  60:	60 00 00 00 	nop
	  64:	3b ff 00 01 	addi    r31,r31,1
	  68:	2f a3 00 00 	cmpdi   cr7,r3,0
	  6c:	7f ff 07 b4 	extsw   r31,r31
	  70:	41 9e ff d0 	beq+    cr7,40 <.kt_2+0x40>
	  74:	38 21 00 90 	addi    r1,r1,144
	  78:	38 60 00 00 	li      r3,0
	  7c:	e8 01 00 10 	ld      r0,16(r1)
	  80:	eb 81 ff e0 	ld      r28,-32(r1)
	  84:	eb a1 ff e8 	ld      r29,-24(r1)
	  88:	eb c1 ff f0 	ld      r30,-16(r1)
	  8c:	eb e1 ff f8 	ld      r31,-8(r1)
	  90:	7c 08 03 a6 	mtlr    r0
	  94:	4e 80 00 20 	blr
	  98:	e8 7e 80 28 	ld      r3,-32728(r30)
	  9c:	7f e5 fb 78 	mr      r5,r31
	  a0:	48 00 00 01 	bl      a0 <.kt_2+0xa0>
	  a4:	60 00 00 00 	nop
	  a8:	4b ff ff a8 	b       50 <.kt_2+0x50>
	  ac:	48 00 00 01 	bl      ac <.kt_2+0xac>
	  b0:	60 00 00 00 	nop
	  b4:	4b ff ff a8 	b       5c <.kt_2+0x5c>
	  b8:	60 00 00 00 	nop
	  bc:	60 00 00 00 	nop

	00000000000000c0 <.kt_1>:
	  c0:	7c 08 02 a6 	mflr    r0
	  c4:	fb 81 ff e0 	std     r28,-32(r1)
	  c8:	fb a1 ff e8 	std     r29,-24(r1)
	  cc:	fb c1 ff f0 	std     r30,-16(r1)
	  d0:	fb e1 ff f8 	std     r31,-8(r1)
	  d4:	eb c2 00 00 	ld      r30,0(r2)
	  d8:	f8 01 00 10 	std     r0,16(r1)
	  dc:	f8 21 ff 71 	stdu    r1,-144(r1)
	  e0:	7c 7d 1b 78 	mr      r29,r3
	  e4:	3b e0 00 00 	li      r31,0
	  e8:	78 3c 04 64 	rldicr  r28,r1,0,49
	  ec:	3b 9c 00 80 	addi    r28,r28,128
	  f0:	48 00 00 38 	b       128 <.kt_1+0x68>
	  f4:	60 00 00 00 	nop
	  f8:	60 00 00 00 	nop
	  fc:	60 00 00 00 	nop
	 100:	e8 1d 00 00 	ld      r0,0(r29)
	 104:	79 20 e8 0e 	rldimi  r0,r9,61,0
	 108:	f8 1d 00 00 	std     r0,0(r29)
	 10c:	80 1d 00 00 	lwz     r0,0(r29)
	 110:	54 00 1f 7e 	rlwinm  r0,r0,3,29,31
	 114:	7f 80 f8 00 	cmpw    cr7,r0,r31
	 118:	40 de 00 6c 	bne-    cr7,184 <.kt_1+0xc4>
	 11c:	e8 1c 00 00 	ld      r0,0(r28)
	 120:	78 09 f7 e3 	rldicl. r9,r0,62,63
	 124:	40 c2 00 70 	bne-    194 <.kt_1+0xd4>
	 128:	48 00 00 01 	bl      128 <.kt_1+0x68>
	 12c:	60 00 00 00 	nop
	 130:	3b ff 00 01 	addi    r31,r31,1
	 134:	2f a3 00 00 	cmpdi   cr7,r3,0
	 138:	7f ff 07 b4 	extsw   r31,r31
	 13c:	2f 1f 00 07 	cmpwi   cr6,r31,7
	 140:	7b e9 07 60 	clrldi  r9,r31,61
	 144:	40 9e 00 1c 	bne-    cr7,160 <.kt_1+0xa0>
	 148:	40 9a ff b8 	bne+    cr6,100 <.kt_1+0x40>
	 14c:	39 20 00 00 	li      r9,0
	 150:	3b e0 00 00 	li      r31,0
	 154:	4b ff ff ac 	b       100 <.kt_1+0x40>
	 158:	60 00 00 00 	nop
	 15c:	60 00 00 00 	nop
	 160:	38 21 00 90 	addi    r1,r1,144
	 164:	38 60 00 00 	li      r3,0
	 168:	e8 01 00 10 	ld      r0,16(r1)
	 16c:	eb 81 ff e0 	ld      r28,-32(r1)
	 170:	eb a1 ff e8 	ld      r29,-24(r1)
	 174:	eb c1 ff f0 	ld      r30,-16(r1)
	 178:	eb e1 ff f8 	ld      r31,-8(r1)
	 17c:	7c 08 03 a6 	mtlr    r0
	 180:	4e 80 00 20 	blr
	 184:	e8 7e 80 30 	ld      r3,-32720(r30)
	 188:	48 00 00 01 	bl      188 <.kt_1+0xc8>
	 18c:	60 00 00 00 	nop
	 190:	4b ff ff 8c 	b       11c <.kt_1+0x5c>
	 194:	48 00 00 01 	bl      194 <.kt_1+0xd4>
	 198:	60 00 00 00 	nop
	 19c:	4b ff ff 8c 	b       128 <.kt_1+0x68>

Unfortunately it tells me nothing, I do not know ppc.

Oleg.

--------------------------------------------------------------------------------
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/kthread.h>

struct utrace {
	unsigned int resume:3;
	int freeze_stop;
};

static int kt_1(void *arg)
{
	struct utrace *u = arg;
	int r = 0;

	while (!kthread_should_stop()) {
		if (++r == 7)
			r = 0;

		u->resume = r;
		barrier();
		if (u->resume != r)
			printk(KERN_CRIT "BUG! bitfield\n");

		if (need_resched())
			schedule();
	}

	return 0;
}

static int kt_2(void *arg)
{
	struct utrace *u = arg;
	int f = 0;

	while (!kthread_should_stop()) {
		u->freeze_stop = ++f;
		barrier();
		if (u->freeze_stop != f)
			printk(KERN_CRIT "BUG! freeze_stop %d != %d\n", u->freeze_stop, f);
		
		if (need_resched())
			schedule();
	}

	return 0;
}

static struct task_struct *t_1, *t_2;

static struct utrace utrace;

static int __init mod_init(void)
{
	WARN_ON(IS_ERR(t_1 = kthread_run(kt_1, &utrace, "kt_1")));
	WARN_ON(IS_ERR(t_2 = kthread_run(kt_2, &utrace, "kt_2")));

	return 0;
}

static void __exit mod_exit(void)
{
	kthread_stop(t_1);
	kthread_stop(t_2);
}

MODULE_LICENSE("GPL");
module_init(mod_init);
module_exit(mod_exit);