From 9d1c845dccb8334d8d73a0b438dc3fbc23580a62 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Thu, 20 Dec 2018 10:11:36 -0600 Subject: [PATCH] Revert "This patch prevent heap corruption as in below case." This solution to the problem noted by EunBong Song results in major memory fragmentation and and out-of-memory conditions on the PX4 platform. On that platform the lower priority work queue is very low priority and essentially never runs when the system is busy. As a result, the systems gets slowly starved of memory until failures and bad behaviors begin to occur. This is an addition patch coming later to result the original problem in a different way that does not have cause memory starvation. This reverts commit 91aa26774b291fa553f701ce5222e56a6156c323. --- sched/sched/sched_free.c | 66 ++++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/sched/sched/sched_free.c b/sched/sched/sched_free.c index 4d8ca557ce9..22934a2355d 100644 --- a/sched/sched/sched_free.c +++ b/sched/sched/sched_free.c @@ -85,27 +85,37 @@ void sched_ufree(FAR void *address) * must have exclusive access to the memory manager to do this. */ - irqstate_t flags; + if (up_interrupt_context() || kumm_trysemaphore() != 0) + { + irqstate_t flags; - /* Yes.. Make sure that this is not a attempt to free kernel memory - * using the user deallocator. - */ + /* Yes.. Make sure that this is not a attempt to free kernel memory + * using the user deallocator. + */ - flags = enter_critical_section(); + flags = enter_critical_section(); #if (defined(CONFIG_BUILD_PROTECTED) || defined(CONFIG_BUILD_KERNEL)) && \ defined(CONFIG_MM_KERNEL_HEAP) - DEBUGASSERT(!kmm_heapmember(address)); + DEBUGASSERT(!kmm_heapmember(address)); #endif - /* Delay the deallocation until a more appropriate time. */ + /* Delay the deallocation until a more appropriate time. */ - sq_addlast((FAR sq_entry_t *)address, - (FAR sq_queue_t *)&g_delayed_kufree); + sq_addlast((FAR sq_entry_t *)address, + (FAR sq_queue_t *)&g_delayed_kufree); - /* Signal the worker thread that is has some clean up to do */ + /* Signal the worker thread that is has some clean up to do */ - sched_signal_free(); - leave_critical_section(flags); + sched_signal_free(); + leave_critical_section(flags); + } + else + { + /* No.. just deallocate the memory now. */ + + kumm_free(address); + kumm_givesemaphore(); + } #endif } @@ -119,22 +129,32 @@ void sched_kfree(FAR void *address) * must have exclusive access to the memory manager to do this. */ - /* Yes.. Make sure that this is not a attempt to free user memory - * using the kernel deallocator. - */ + if (up_interrupt_context() || kmm_trysemaphore() != 0) + { + /* Yes.. Make sure that this is not a attempt to free user memory + * using the kernel deallocator. + */ - flags = enter_critical_section(); - DEBUGASSERT(kmm_heapmember(address)); + flags = enter_critical_section(); + DEBUGASSERT(kmm_heapmember(address)); - /* Delay the deallocation until a more appropriate time. */ + /* Delay the deallocation until a more appropriate time. */ - sq_addlast((FAR sq_entry_t *)address, - (FAR sq_queue_t *)&g_delayed_kfree); + sq_addlast((FAR sq_entry_t *)address, + (FAR sq_queue_t *)&g_delayed_kfree); - /* Signal the worker thread that is has some clean up to do */ + /* Signal the worker thread that is has some clean up to do */ - sched_signal_free(); - leave_critical_section(flags); + sched_signal_free(); + leave_critical_section(flags); + } + else + { + /* No.. just deallocate the memory now. */ + + kmm_free(address); + kmm_givesemaphore(); + } } #endif