osconfig.h OS_QUEUE_MAX_DEPTH unused

[skliper]

OS_QUEUE_MAX_DEPTH is defined as 50 in osconfig.h for both pc-rtems and pc-linux, but the limit isn't applied/checked/or even used within OSAL.

Queue depth is accepted as input by OS_QueueCreate with no limiting within OSAL. Note cFE also has a max pipe depth (CFE_PLATFORM_SB_MAX_PIPE_DEPTH) that is applied by the cFE prior to calling OS_QueueCreate, but it's set to 256. This limit seems arbitrary at the cFE level.

Linux depth limit by default is 10 on at least CentOS, would be nice if it worked out of the box.

[skliper]

Imported from trac issue 212. Created by jhageman on 2019-04-29T16:44:52, last modified: 2019-08-14T14:11:46

[skliper]

Trac comment by jhageman on 2019-06-19 10:47:58:

Note apps all use SB's CFE_SB_CreatePipe (ES, EVS, TBL, TIME, SB):

• cfe_sb_priv.h: CFE_SB_CMD_PIPE_DEPTH 32
• cfe_es_task.c: CFE_ES_TaskData.PipeDepth = 12
• cfe_evs_task.h: CFE_EVS_PIPE_DEPTH 32
• cfe_tbl_task.h: CFE_TBL_TASK_PIPE_DEPTH 12
• cfe_time_utils.h: CFE_TIME_TASK_PIPE_DEPTH 12

[skliper]

Trac comment by jhageman on 2019-06-20 17:55:51:

Suggest something like:
{{{
/* Limit to platform depth */
if (Depth > OS_QUEUE_MAX_DEPTH)
{
Send a warning
Depth = OS_QUEUE_MAX_DEPTH;
}
}}}

Note preference is not to fail, since use case is for out of the box linux setting OS_QUEUE_MAX_DEPTH to 10 (in osal/src/bsp/pc-linux/config) should just work.

[skliper]

Trac comment by jhageman on 2019-06-26 12:40:19:

CCB 6/26/19 - Gated behind a permissive mode flag (already done, will give smaller depth vs failing).

Permissive mode does non real time threads and limits depth of queues.

[skliper]

Trac comment by jhageman on 2019-06-26 15:12:17:

See osapi.c, OSAL_DEBUG_PERMISSIVE_MODE, when defined truncates the queue depth and reports an OS_DEBUG message (in next gen for linux). Note currently the permissive mode is defaulted ON. and debug messages are defaulted as disabled.

Remaining work - remove OS_QUEUE_MAX_DEPTH from osconfig.h in src/bsp/pc-rtems/config/ and src/bsp/pc-linux/config/ since it isn't used anywhere.

[skliper]

Grabbed assignment as placeholder for training (if the issue doesn't go away with fix for #285)

[jphickey]

It may be worth keeping this around. Issue #73 comes up again and again as a nice to have, which would improve our ability to support OS's that do not have posix mqueue support, or even as a higher-performance alternative. To do this, we would need to have a configuration-limited maximum size.

Alternatively, one could just use malloc() to get memory for a local queue without this fixed limit. After all this is basically what the current implementation does, its just hidden inside the C library so we don't see it in the OSAL code, but its still there.... Since queues typically are created at boot time then this might not violate the "no malloc at runtime" rule.

[skliper]

I'd support closing this issue with better documentation within the new os configuration scheme (placeholder for now, not used but reserved for static sizing of local queues, or something similar)? Anything that indicates it's not applicable to the current implementation, and reference the OS limit.

[jphickey]

What if we just add a check for this in the shared layer? It is probably a good idea (architecture-wise) to impose some additional sanity check limit on the depths of queues independently of the OS/implementation limit.

A buggy software application could, after all, try to create a queue with thousands/millions of entries, and depending on the RTOS implementation it might even allow it, at the expense of using all the memory in the system. This can be an obscure problem that manifests in weird ways such as slow performance or weird issues in other parts of the system.

Anecdote from past experience -- at one point years ago on a different project, a software lookup table was based on a computed size and due to a bug it became something like 4 million entries instead of about 30-40 entries. But aside from the huge size it actually worked/ran OK, the issue wasn't noticed until some time later when developers on the team noticed the enormous VRAM sizes and investigated.

[skliper]

What if we just add a check for this in the shared layer?

Good approach. Recommend a comment something like - upper limit of this value may be restricted by underlying implementation, for mqueues see xyz, but I like it as a sanity check.

[skliper]

@jphickey what's the status on this issue?