michaelh/zephyr
1
0
Fork 0
Code Releases Activity

samples/userspace/shared_mem: Add volatile to interthread data

Browse source 
This test uses bare variables to synchronize state between threads,
but had forgotten volatile qualifiers on all the data.  So the
compiler was free to reorder and make assumptions that aren't valid
when the values are being written from other CPUs.

Single-cpu operation was fine because the code would always hit an
external function call like k_sleep() that would force it to re-read
from memory every time there was a context switch (timeslicing isn't
enabled on this test and the threads are cooperative), but on SMP the
volatiles can change at any time and we could see spurious state
mixups and hangs.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This commit is contained in:
Andy Ross 2020-01-23 12:56:47 -08:00 • committed by Anas Nashif
commit 9e37e80a1d
2 changed files with 40 additions and 39 deletions
Download patch file Download diff file Expand all files Collapse all files

20
samples/userspace/shared_mem/src/enc.h
View file

@ -23,15 +23,15 @@ short char_to_index(char c);
int calc_rev_wheel(BYTE *wheel, BYTE *backpath); int calc_rev_wheel(BYTE *wheel, BYTE *backpath);
char enig_enc(char pt); char enig_enc(char pt);
extern BYTE W1[26]; extern volatile BYTE W1[26];
extern BYTE W2[26]; extern volatile BYTE W2[26];
extern BYTE W3[26]; extern volatile BYTE W3[26];
extern BYTE R[26]; extern volatile BYTE R[26];
extern BYTE W1R[26]; extern volatile BYTE W1R[26];
extern BYTE W2R[26]; extern volatile BYTE W2R[26];
extern BYTE W3R[26]; extern volatile BYTE W3R[26];
extern int IW1; extern volatile int IW1;
extern int IW2; extern volatile int IW2;
extern int IW3; extern volatile int IW3;
#endif #endif

59
samples/userspace/shared_mem/src/main.c
View file

@ -34,34 +34,34 @@ struct k_mem_domain dom0, dom1, dom2;
* the names are symbolic for the memory partitions * the names are symbolic for the memory partitions
* purpose. * purpose.
*/ */
_app_red_b BYTE fBUFIN; volatile _app_red_b BYTE fBUFIN;
_app_red_b BYTE BUFIN[63]; volatile _app_red_b BYTE BUFIN[63];
_app_blk_b BYTE fBUFOUT; volatile _app_blk_b BYTE fBUFOUT;
_app_blk_b BYTE BUFOUT[63]; volatile _app_blk_b BYTE BUFOUT[63];
/* declare and set wheel and reflector */ /* declare and set wheel and reflector */
/* To use add definition ALTMSG */ /* To use add definition ALTMSG */
#ifdef ALTMSG #ifdef ALTMSG
_app_enc_d BYTE W1[26] = START_WHEEL; volatile _app_enc_d BYTE W1[26] = START_WHEEL;
#else #else
_app_enc_d BYTE W1[26] = START_WHEEL2; volatile _app_enc_d BYTE W1[26] = START_WHEEL2;
#endif #endif
_app_enc_d BYTE W2[26] = START_WHEEL; volatile _app_enc_d BYTE W2[26] = START_WHEEL;
_app_enc_d BYTE W3[26] = START_WHEEL; volatile _app_enc_d BYTE W3[26] = START_WHEEL;
_app_enc_d BYTE R[26] = REFLECT; volatile _app_enc_d BYTE R[26] = REFLECT;
_app_enc_b int IW1; volatile _app_enc_b int IW1;
_app_enc_b int IW2; volatile _app_enc_b int IW2;
_app_enc_b int IW3; volatile _app_enc_b int IW3;
/* /*
* calculated by the enc thread at init and when the wheels * calculated by the enc thread at init and when the wheels
* change. * change.
*/ */
_app_enc_b BYTE W1R[26]; volatile _app_enc_b BYTE W1R[26];
_app_enc_b BYTE W2R[26]; volatile _app_enc_b BYTE W2R[26];
_app_enc_b BYTE W3R[26]; volatile _app_enc_b BYTE W3R[26];
/* /*
* sync threads * sync threads
@ -78,8 +78,8 @@ struct k_thread ct_thread;
K_THREAD_STACK_DEFINE(ct_stack, STACKSIZE); K_THREAD_STACK_DEFINE(ct_stack, STACKSIZE);
_app_enc_d char encMSG[] = "ENC!\n"; _app_enc_d char encMSG[] = "ENC!\n";
_app_enc_b char enc_pt[50]; /* Copy form shared pt */ volatile _app_enc_b char enc_pt[50]; /* Copy form shared pt */
_app_enc_b char enc_ct[50]; /* Copy to shared ct */ volatile _app_enc_b char enc_ct[50]; /* Copy to shared ct */
_app_user_d char ptMSG[] = "PT: message to encrypt\n"; _app_user_d char ptMSG[] = "PT: message to encrypt\n";
@ -181,7 +181,7 @@ void enc(void)
if (fBUFIN == 1) { /* 1 is process text */ if (fBUFIN == 1) { /* 1 is process text */
printk("ENC Thread Received Data\n"); printk("ENC Thread Received Data\n");
/* copy message form shared mem and clear flag */ /* copy message form shared mem and clear flag */
memcpy(&enc_pt, BUFIN, SAMP_BLOCKSIZE); memcpy((void *)&enc_pt, (void *)BUFIN, SAMP_BLOCKSIZE);
printk("ENC PT MSG: %s\n", (char *)&enc_pt); printk("ENC PT MSG: %s\n", (char *)&enc_pt);
fBUFIN = 0; fBUFIN = 0;
/* reset wheel: probably better as a flag option */ /* reset wheel: probably better as a flag option */
@ -189,7 +189,7 @@ void enc(void)
IW2 = 2; IW2 = 2;
IW3 = 3; IW3 = 3;
/* encode */ /* encode */
memset(&enc_ct, 0, SAMP_BLOCKSIZE); /* clear memory */ memset((void *)&enc_ct, 0, SAMP_BLOCKSIZE);
for (index = 0, index_out = 0; index < SAMP_BLOCKSIZE; index++) { for (index = 0, index_out = 0; index < SAMP_BLOCKSIZE; index++) {
if (enc_pt[index] == '\0') { if (enc_pt[index] == '\0') {
enc_ct[index_out] = '\0'; enc_ct[index_out] = '\0';
@ -202,10 +202,11 @@ void enc(void)
} }
/* test for CT flag */ /* test for CT flag */
while (fBUFOUT != 0) { while (fBUFOUT != 0) {
k_sleep(K_MSEC(100)); k_sleep(K_MSEC(1));
} }
/* ct thread has cleared the buffer */ /* ct thread has cleared the buffer */
memcpy(&BUFOUT, &enc_ct, SAMP_BLOCKSIZE); memcpy((void *)&BUFOUT, (void *)&enc_ct,
SAMP_BLOCKSIZE);
fBUFOUT = 1; fBUFOUT = 1;
} }
@ -221,13 +222,13 @@ void enc(void)
void pt(void) void pt(void)
{ {
k_sleep(K_MSEC(2000)); k_sleep(K_MSEC(20));
while (1) { while (1) {
k_sem_take(&allforone, K_FOREVER); k_sem_take(&allforone, K_FOREVER);
if (fBUFIN == 0) { /* send message to encode */ if (fBUFIN == 0) { /* send message to encode */
printk("\nPT Sending Message 1\n"); printk("\nPT Sending Message 1\n");
memset(&BUFIN, 0, SAMP_BLOCKSIZE); memset((void *)&BUFIN, 0, SAMP_BLOCKSIZE);
memcpy(&BUFIN, &ptMSG, sizeof(ptMSG)); memcpy((void *)&BUFIN, (void *)&ptMSG, sizeof(ptMSG));
/* strlen should not be used if user provided data, needs a max length set */ /* strlen should not be used if user provided data, needs a max length set */
fBUFIN = 1; fBUFIN = 1;
} }
@ -235,12 +236,12 @@ void pt(void)
k_sem_take(&allforone, K_FOREVER); k_sem_take(&allforone, K_FOREVER);
if (fBUFIN == 0) { /* send message to decode */ if (fBUFIN == 0) { /* send message to decode */
printk("\nPT Sending Message 1'\n"); printk("\nPT Sending Message 1'\n");
memset(&BUFIN, 0, SAMP_BLOCKSIZE); memset((void *)&BUFIN, 0, SAMP_BLOCKSIZE);
memcpy(&BUFIN, &ptMSG2, sizeof(ptMSG2)); memcpy((void *)&BUFIN, (void *)&ptMSG2, sizeof(ptMSG2));
fBUFIN = 1; fBUFIN = 1;
} }
k_sem_give(&allforone); k_sem_give(&allforone);
k_sleep(K_MSEC(5000)); k_sleep(K_MSEC(50));
} }
} }
@ -257,8 +258,8 @@ void ct(void)
k_sem_take(&allforone, K_FOREVER); k_sem_take(&allforone, K_FOREVER);
if (fBUFOUT == 1) { if (fBUFOUT == 1) {
printk("CT Thread Receivedd Message\n"); printk("CT Thread Receivedd Message\n");
memset(&tbuf, 0, sizeof(tbuf)); memset((void *)&tbuf, 0, sizeof(tbuf));
memcpy(&tbuf, BUFOUT, SAMP_BLOCKSIZE); memcpy((void *)&tbuf, (void *)BUFOUT, SAMP_BLOCKSIZE);
fBUFOUT = 0; fBUFOUT = 0;
printk("CT MSG: %s\n", (char *)&tbuf); printk("CT MSG: %s\n", (char *)&tbuf);
} }