關於 SHM 沒有加上 volatile 這件事 about pttbbs

[wens]

我想應該是要加才對? XD

[IepIweidieng]

pttbbs/common/bbs/cache.c

Lines 430 to 455 in 86f1574

	void
	sort_bcache(void)
	{
	int i;
	/* critical section 盡量不要呼叫 */
	/* 只有新增 或移除看板 需要呼叫到 */
	if(SHM->Bbusystate) {
	sleep(1);
	return;
	}
	SHM->Bbusystate = 1;
	for (i = 0; i < SHM->Bnumber; i++) {
	SHM->bsorted[BRD_GROUP_LL_TYPE_NAME][i] = i;
	SHM->bsorted[BRD_GROUP_LL_TYPE_CLASS][i] = i;
	}
	qsort(SHM->bsorted[BRD_GROUP_LL_TYPE_NAME],
	SHM->Bnumber, sizeof(int), cmpboardname);
	qsort(SHM->bsorted[BRD_GROUP_LL_TYPE_CLASS],
	SHM->Bnumber, sizeof(int), cmpboardclass);
	for (i = 0; i < SHM->Bnumber; i++) {
	bcache[i].firstchild[BRD_GROUP_LL_TYPE_NAME] = 0;
	bcache[i].firstchild[BRD_GROUP_LL_TYPE_CLASS] = 0;
	}
	SHM->Bbusystate = 0;
	}

Like this.

[PichuChen]

恩... 感覺直接改一行的話效能會降低

[IepIweidieng]

若要更謹慎的話，應可加上 ISO C11 的 _Atomic 或是 ISO C++11 的 std::atomic，使用 lock-free atomic 型別。
To take it more carefully, either _Atomic from ISO C11 or std::atomic from ISO C++11 could be added for using the lock-free atomic type.

因為 lock-free atomic 像 volatile 一般，也可防止編譯器最佳化掉變數的重要存取指令，僅使用 _Atomic 即可。
Since lock-free atomics, like volatile, can also prevent the compiler from optimizing out the important reading and writing instructions of the variables, it should be fine to use _Atomic along.

References:

• About C11 _Atomic: https://en.cppreference.com/w/c/language/atomic
• About C++11 std::atomic: https://en.cppreference.com/w/cpp/atomic/atomic

[wens]

SHM 是一整個 data structure ，沒辦法用 _Atomic 吧。

[IepIweidieng]

用在當作 synchronization lock 的變數上面。
It is meant for the lock variable which is taken as the synchronization lock.

[wens]

那是完全另一回事了。SHM 現在就是沒有 lock

[IepIweidieng]

不過在 SHM_t 中，有用途類似於 lock 的 member，例如：

UTMPbusystate:

pttbbs/include/pttstruct.h

Lines 533 to 537 in 86f1574

	int UTMPnumber;
	char UTMPneedsort;
	char UTMPbusystate;
	/* brdshm */

Bbusystate:

pttbbs/include/pttstruct.h

Lines 562 to 566 in 86f1574

	time4_t Btouchtime;
	int Bnumber;
	int Bbusystate;
	time4_t close_vote_time;

以及其它名字帶 busystate 的 members。

這些 members 就適合使用 _Atomic。

[hungte]

Not jut SHM, I think we need to add volatile to all cache.c variables:

/* cache.c */
SHM_t          *SHM;
boardheader_t  *bcache;
userinfo_t     *currutmp;

although, since we have never see issues so far, I'd assume the compiler is smart enough to figure hat shm, bcache, and currutmp are non-cacheable pointers.

[IepIweidieng]

Figuring out which global pointers point to non-cacheable contents is theoretically impossible without using link-time optimization (LTO). Enabling LTO requires the flag -flto to be explicitly passed to the compiler and the linker of GCC or Clang. However, this flag has never been used for PttBBS according to pttbbs.mk.

Not every code which accesses the data pointed by these pointers is compiled with the code which makes these pointers point into the shared memory. These codes are linked together instead of being compiled together, so the compiler is unable to figure out such usages at compile-time.

The reason why issues have not been observed should be because there have been efforts to reduce the frequency of writing into shared memory in the PttBBS codebase.

Writes into shared memory are more restricted and secured in PttBBS compared to the most of other MapleBBS-family BBSes, although the race condition issues due to the time-slicing scheduling of Linux and especially multi-processor environments (which cannot be solved by using volatile) seem to have not been addressed.

Note: The shared-memory–"caching" issue is about caching into the register of the processor (which volatile prevents) but not about caching into the memory cache. Because the memory cache of shared memory is also shared among processes and threads, the contents of shared memory are in fact "cacheable" in the memory-cache sense.