You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/04/09 10:11:04 UTC

[GitHub] [incubator-nuttx] GUIDINGLI opened a new pull request #761: Merge garbage feature to mm/mm_heap

GUIDINGLI opened a new pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761
 
 
   Merge garbage feature to mm/mm_heap
   
   sched_xfree() => kxmm_free()
   Remove garbage related APIs
   remove ARCH_HAVE_GARBAGE
   
   add getpid() return check in mm_trysemaphore() 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406338370
 
 

 ##########
 File path: include/nuttx/mm/mm.h
 ##########
 @@ -263,6 +270,12 @@ struct mm_heap_s
    */
 
   struct mm_freenode_s mm_nodelist[MM_NNODES];
+
+#ifdef __KERNEL__
+  /* Free delay list, for some situation can't do free immdiately */
+
+  struct mm_delaynode_s mm_delaylist;
 
 Review comment:
   Ok, let wait guiding provide a minor fix tomorrow.
   
   > A better, more consistent name would probably be mm_delayhead to indicate that it is a head of a list. sq_queue_t could be used, but that is not really necessary.
   
   Guiding avoid to use sq_queue_t  just because the whole mm don't use sq_queue_t/dq_queue_t manage the list.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406325463
 
 

 ##########
 File path: mm/mm_heap/mm_free.c
 ##########
 @@ -42,8 +42,31 @@
 #include <assert.h>
 #include <debug.h>
 
+#include <nuttx/arch.h>
+#include <nuttx/irq.h>
 
 Review comment:
   Is it enough to just include nuttx/arch.h?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406336424
 
 

 ##########
 File path: include/nuttx/mm/mm.h
 ##########
 @@ -263,6 +270,12 @@ struct mm_heap_s
    */
 
   struct mm_freenode_s mm_nodelist[MM_NNODES];
+
+#ifdef __KERNEL__
+  /* Free delay list, for some situation can't do free immdiately */
+
+  struct mm_delaynode_s mm_delaylist;
 
 Review comment:
   I don't think this matters since struct mm_delaynode_s is defined to be only:
   
       #ifdef __KERNEL__
       struct mm_delaynode_s
       {
         struct mm_delaynode_s *flink;
       };
       #endif

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo merged pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406338370
 
 

 ##########
 File path: include/nuttx/mm/mm.h
 ##########
 @@ -263,6 +270,12 @@ struct mm_heap_s
    */
 
   struct mm_freenode_s mm_nodelist[MM_NNODES];
+
+#ifdef __KERNEL__
+  /* Free delay list, for some situation can't do free immdiately */
+
+  struct mm_delaynode_s mm_delaylist;
 
 Review comment:
   Ok, let wait Guiding provide a minor fix tomorrow.
   
   > A better, more consistent name would probably be mm_delayhead to indicate that it is a head of a list. sq_queue_t could be used, but that is not really necessary.
   
   Guiding avoid to use sq_queue_t  just because the whole mm don't use sq_queue_t/dq_queue_t manage the list.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406324545
 
 

 ##########
 File path: include/nuttx/mm/mm.h
 ##########
 @@ -263,6 +270,12 @@ struct mm_heap_s
    */
 
   struct mm_freenode_s mm_nodelist[MM_NNODES];
+
+#ifdef __KERNEL__
+  /* Free delay list, for some situation can't do free immdiately */
+
+  struct mm_delaynode_s mm_delaylist;
 
 Review comment:
   change to:
   struct mm_delaynode_s *mm_delaylist;
   could simplify the code a littel bit
    

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406323926
 
 

 ##########
 File path: mm/mm_heap/mm_free.c
 ##########
 @@ -42,8 +42,31 @@
 #include <assert.h>
 #include <debug.h>
 
+#include <nuttx/arch.h>
+#include <nuttx/irq.h>
 #include <nuttx/mm/mm.h>
 
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#ifdef __KERNEL__
+static void mm_add_delaylist(FAR struct mm_heap_s *heap, FAR void *mem)
+{
+  FAR struct mm_delaynode_s *new = mem;
 
 Review comment:
   Change new to tmp like mm_free_delaylist

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406338370
 
 

 ##########
 File path: include/nuttx/mm/mm.h
 ##########
 @@ -263,6 +270,12 @@ struct mm_heap_s
    */
 
   struct mm_freenode_s mm_nodelist[MM_NNODES];
+
+#ifdef __KERNEL__
+  /* Free delay list, for some situation can't do free immdiately */
+
+  struct mm_delaynode_s mm_delaylist;
 
 Review comment:
   Ok, let wait guiding provide a minor fix tomorrow.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406335322
 
 

 ##########
 File path: mm/mm_heap/mm_free.c
 ##########
 @@ -42,8 +42,31 @@
 #include <assert.h>
 #include <debug.h>
 
+#include <nuttx/arch.h>
+#include <nuttx/irq.h>
 #include <nuttx/mm/mm.h>
 
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#ifdef __KERNEL__
+static void mm_add_delaylist(FAR struct mm_heap_s *heap, FAR void *mem)
+{
+  FAR struct mm_delaynode_s *new = mem;
 
 Review comment:
   Good catch.  You have very good eyes, Xiang.  I have already merged the change, but new will cause problems for some tools.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406338370
 
 

 ##########
 File path: include/nuttx/mm/mm.h
 ##########
 @@ -263,6 +270,12 @@ struct mm_heap_s
    */
 
   struct mm_freenode_s mm_nodelist[MM_NNODES];
+
+#ifdef __KERNEL__
+  /* Free delay list, for some situation can't do free immdiately */
+
+  struct mm_delaynode_s mm_delaylist;
 
 Review comment:
   Ok, let wait Guiding provide a minor fix tomorrow.
   
   > A better, more consistent name would probably be mm_delayhead to indicate that it is a head of a list. sq_queue_t could be used, but that is not really necessary.
   
   Guiding intentionally avoid to use sq_queue_t  just because the whole mm code base don't use sq_queue_t/dq_queue_t manage the list, he don't want to introduce the additional dependence.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #761: Merge garbage feature to mm/mm_heap

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #761: Merge garbage feature to mm/mm_heap
URL: https://github.com/apache/incubator-nuttx/pull/761#discussion_r406337276
 
 

 ##########
 File path: include/nuttx/mm/mm.h
 ##########
 @@ -263,6 +270,12 @@ struct mm_heap_s
    */
 
   struct mm_freenode_s mm_nodelist[MM_NNODES];
+
+#ifdef __KERNEL__
+  /* Free delay list, for some situation can't do free immdiately */
+
+  struct mm_delaynode_s mm_delaylist;
 
 Review comment:
   A better, more consistent name would probably be mm_delayhead to indicate that it is a head of a list.  sq_queue_t could be used, but that is not really necessary.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services