RT-Thread / rt-thread

RT-Thread is an open source IoT Real-Time Operating System (RTOS).
https://www.rt-thread.io
Apache License 2.0
10.33k stars 4.98k forks source link

关于赋值语句是否具有原子性的讨论 #8449

Open FragrantRye opened 8 months ago

FragrantRye commented 8 months ago

出发点是看到了在这个PR中的讨论,其中提到的 int32 数据赋值在对齐时本来就是原子的 与我印象中不符,于是查阅了一些资料与各位开发者分享,也希望听听大家的意见。

简单总结一下就是,C/C++中的“赋值”操作并不是原子的,不论从编译器实现上还是语言标准规定上皆是如此,而且变量的地址对齐与否并不影响该结论。 具体可以参考这篇文章 其中详细解释了引起非原子性的原因。

针对rt-thread中大量存在(maybe)的“直接赋值当成原子操作”的行为,是否有必要集中做一次修改?

mysterywolf commented 8 months ago

我认为这个观点是正确的 只要是赋值 绝对不能当做是原子操作,这个行为与编译器、CPU架构很多因素都有关,很有可能大部分情况运气好,没有什么问题。但是作为操作系统,不能有这种假设。

mysterywolf commented 8 months ago

你不说我都没注意,他评论那个位置的临界区原来是我加的,我都不知道什么时候被删了😂

BernardXiong commented 8 months ago

赋值不是原子性的,需要rt_atomic_store,例如

void rt_tick_set(rt_tick_t tick)
{
    rt_atomic_store(&(rt_tick), tick);
}
mysterywolf commented 8 months ago

@xqyjlj

xqyjlj commented 8 months ago

嗯,int32对齐时的原子性在arm,x86平台上是显而易见的。你提到的那篇文章阐述的也是在32为平台对64为变量赋值的原子性。至于32位平台对一个对齐的32位数据进行赋值,在我目前接触的arch来看是必然原子的。如果这条不成立,那linux也得乱套了

FragrantRye commented 8 months ago

感谢回复!

int32对齐时的原子性在arm,x86平台上是显而易见的

我认同这一点,但我觉得应该仅限于手写汇编代码使用单条指令完成赋值时。因为较真一点的话这一点只在arch的文档中有规定,例如aarch64上在这里,但是其并没有规定C/C++代码的原子性。 就目前的C/C++标准来看也没有要求编译器在遇到aligned memory access时必须生成单条汇编指令,比如对int32的赋值完全可以生成4条int8 store来完成。 虽然不见得哪个编译器真会这么犯病,但是anyway我们写的代码是C而不是汇编嘛,我觉得还是应该尊重一下标准的。

至于您提到的linux,我印象中在必要时是会采用WRITE_ONCE/READ_ONCE两个宏(其实是对volatile的封装)来实现,不会直接赋值。

xqyjlj commented 8 months ago

至于您提到的linux,我印象中在必要时是会采用WRITE_ONCE/READ_ONCE两个宏(其实是对volatile的封装)来实现,不会直接赋值。


#define __READ_ONCE_SIZE                        \
({                                  \
switch (size) {                         \
case 1: *(__u8 *)res = *(volatile __u8 *)p; break;      \
case 2: *(__u16 *)res = *(volatile __u16 *)p; break;        \
case 4: *(__u32 *)res = *(volatile __u32 *)p; break;        \
case 8: *(__u64 *)res = *(volatile __u64 *)p; break;        \
default:                            \
barrier();                      \
__builtin_memcpy((void *)res, (const void *)p, size);   \
barrier();                      \
}                               \
})

static __always_inline void write_once_size(volatile void p, void res, int size) { switch (size) { case 1: *(volatile u8 )p = (u8 )res; break; case 2: (volatile u16 )p = (u16 )res; break; case 4: (volatile u32 )p = (u32 )res; break; case 8: (volatile u64 )p = (u64 *)res; break; default: barrier(); builtin_memcpy((void )p, (const void )res, size); barrier(); } }


我比较认同这种做法,使用volatile修饰,让编译器来生成赋值代码,而不是使用atomic系列api,目前rtt的atomic类型会强制上升到cpu位数(32/64)并且可能会存在部分架构(比如m0核)的性能问题(软件模拟原子)
polarvid commented 8 months ago

就目前的C/C++标准来看也没有要求编译器在遇到aligned memory access时必须生成单条汇编指令,比如对int32的赋值完全可以生成4条int8 store来完成。

所以标准 C 给出了 c11 atomic 的补丁。

void rt_tz_set(int32_t offset_sec)
{
    rt_base_t level;
    level = rt_hw_interrupt_disable();
    _current_tz_offset_sec = offset_sec;
    rt_hw_interrupt_enable(level);
}

类似这种代码,符合标准 C 的严谨写法应该是。

static _Atomic(int32_t) _current_tz_offset_sec = \
    RT_LIBC_TZ_DEFAULT_HOUR * 3600U + RT_LIBC_TZ_DEFAULT_MIN * 60U + RT_LIBC_TZ_DEFAULT_SEC;

/* return current timezone offset in seconds */
void rt_tz_set(int32_t offset_sec)
{
    atomic_store_explicit(&_current_tz_offset_sec, offset_sec, memory_order_relaxed);
}

int32_t rt_tz_get(void)
{
    return atomic_load_explicit(&_current_tz_offset_sec, memory_order_relaxed);
}

在 C11 标准下,这一对 LOAD/STORE 操作是强制原子性的。只要能编译,就一定不会差错。至于 Linux 使用的 WRITE ONCE 的同等 API,是在 C11 标准之前的,非标准化,旧的实现。对于 RTT 完全没有必要抛开标准化的 API,去定义一套新的实现。如果非要支持不能使用 C11 的平台,RTT 内核也没必要遵循 Linux 内核的那套标准,完全可以完善一下现在的 rtatomic.h,皈依标准化的路径。这对于大量的 C/C++ 程序员而言,都是更加熟悉和亲切的。

而当前 rtatomic.h 欠缺的功能太多了。限制了不同数据类型操作,也没有显式内存次序等非常重要的内容。对于高频操作,默认使用最强的内存次序代价是非常高的。在 RV64 平台 c908/c906 下,这类指令的周期是没有确定性的。在 aarch64 平台下,实际测试下来,其指令周期是运算指令的数百倍。换而言之对于实时系统伤害很大。

最后提一句,上述 C11 atomic 的写法最后生成的指令流在 aarch64 平台下和 _current_tz_offset_sec = xxx 不会有任何区别 。C11 atomic 只是显式保证了正确性。而 合理的 的使用(符合业务逻辑的显式内存次序)才能保证这种正确性不会带来额外的代价。因为如果你用的是 rt_atomic_store/load 就会得到截然不同的结果。这也是我为什么说 rt atomic 缺乏内存次序是非常严重的设计漏洞。

上面两个是 C11 atomic,下面两个是采用了 volatile。

rt_tz_set:
        adrp    x1, .LANCHOR0
        add     x1, x1, :lo12:.LANCHOR0
        str     w0, [x1]
        ret
rt_tz_get:
        adrp    x0, .LANCHOR0
        add     x0, x0, :lo12:.LANCHOR0
        ldr     w0, [x0]
        ret
rt_tz_setx:
        adrp    x1, .LANCHOR0
        add     x1, x1, :lo12:.LANCHOR0
        str     w0, [x1, 4]
        ret
rt_tz_getx:
        adrp    x0, .LANCHOR0
        add     x0, x0, :lo12:.LANCHOR0
        ldr     w0, [x0, 4]
        ret
zhzhqian commented 8 months ago

为什么rtt要将atomic和内存墙绑定在一起,而不是显示地提供内存墙的api呢?类似于linux的smp_mb_before_atomic。我理解,这样将是否在atomic操作加内存墙的权利交给开发人员可以提高整体的性能。

rt_atomic_t rt_hw_atomic_load(volatile rt_atomic_t *ptr)
{
    rt_atomic_t ret;

    __asm__ volatile (
        "   ldr     %0, %1\n"
        "   dmb     ish"
        : "=r" (ret)
        : "Q" (*ptr)
        : "memory");

    return ret;
}
xqyjlj commented 8 months ago

为什么rtt要将atomic和内存墙绑定在一起,而不是显示地提供内存墙的api呢?类似于linux的smp_mb_before_atomic。我理解,这样将是否在atomic操作加内存墙的权利交给开发人员可以提高整体的性能。

建议是再加一个memory_order的参数

polarvid commented 8 months ago

为什么rtt要将atomic和内存墙绑定在一起,而不是显示地提供内存墙的api呢?类似于linux的smp_mb_before_atomic。我理解,这样将是否在atomic操作加内存墙的权利交给开发人员可以提高整体的性能。

建议是再加一个memory_order的参数

+1。标准 C 更加通用。没必要自己又玩一套。反而让人觉得只会抄 Linux。

可以有一个 rt_atomic_load_explicit()

zhzhqian commented 8 months ago

为什么rtt要将atomic和内存墙绑定在一起,而不是显示地提供内存墙的api呢?类似于linux的smp_mb_before_atomic。我理解,这样将是否在atomic操作加内存墙的权利交给开发人员可以提高整体的性能。

建议是再加一个memory_order的参数

+1。标准 C 更加通用。没必要自己又玩一套。反而让人觉得只会抄 Linux。

可以有一个 rt_atomic_load_explicit()

赞同