You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "xiaoxiang781216 (via GitHub)" <gi...@apache.org> on 2023/06/18 14:10:31 UTC

[GitHub] [nuttx] xiaoxiang781216 opened a new pull request, #9564: Compute the heap info more accuracy(2)

xiaoxiang781216 opened a new pull request, #9564:
URL: https://github.com/apache/nuttx/pull/9564

   ## Summary
   
   Continue the work https://github.com/apache/nuttx/pull/9561
   
   - mm/mempool: Count the backtrace overhead in mempool_[dump|info[_task]] 
   - mm/mempool: Count the expend block overhead in mempool_info
   - mm/tlfs: Count the backtrace overhead in mm_mallinfo_task
   - mm: Simplify memdump_handler logic a little bit
   
   ## Impact
   
   Report more accuracy statistics info
   
   ## Testing
   
   sim:kasan and sim:adb


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9564: Compute the heap info more accuracy(2)

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9564:
URL: https://github.com/apache/nuttx/pull/9564#discussion_r1233425723


##########
mm/mempool/mempool.c:
##########
@@ -460,55 +462,55 @@ mempool_info_task(FAR struct mempool_s *pool,
 void mempool_memdump(FAR struct mempool_s *pool,
                      FAR const struct mm_memdump_s *dump)
 {
+  size_t blocksize = MEMPOOL_REALBLOCKSIZE(pool);
+
   if (dump->pid == PID_MM_FREE)
     {
       FAR sq_entry_t *entry;
 
       sq_for_every(&pool->queue, entry)
         {
           syslog(LOG_INFO, "%12zu%*p\n",
-                 pool->blocksize, MM_PTR_FMT_WIDTH,
-                 (FAR char *)entry);
+                 blocksize, MM_PTR_FMT_WIDTH, (FAR char *)entry);
         }
 
       sq_for_every(&pool->iqueue, entry)
         {
           syslog(LOG_INFO, "%12zu%*p\n",
-                 pool->blocksize, MM_PTR_FMT_WIDTH,
-                 (FAR char *)entry);
+                 blocksize, MM_PTR_FMT_WIDTH, (FAR char *)entry);
         }
     }
 #if CONFIG_MM_BACKTRACE >= 0
   else
     {
       FAR struct mempool_backtrace_s *buf;
 
-      list_for_every_entry(&pool->alist, buf, struct mempool_backtrace_s,
-                           node)
+      list_for_every_entry(&pool->alist, buf,
+                           struct mempool_backtrace_s, node)
         {
           if ((dump->pid == PID_MM_ALLOC || dump->pid == buf->pid) &&
               buf->seqno >= dump->seqmin && buf->seqno <= dump->seqmax)
             {
+              char tmp[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] =
+              {
+                '\0'
+              };

Review Comment:
   ```suggestion
                 char tmp[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] = "";
   ```
   Optional 



##########
mm/tlsf/mm_tlsf.c:
##########
@@ -404,33 +399,31 @@ static void memdump_handler(FAR void *ptr, size_t size, int used,
                             FAR void *user)
 {
   FAR const struct mm_memdump_s *dump = user;
-#if CONFIG_MM_BACKTRACE >= 0
-  FAR struct memdump_backtrace_s *buf;
-
-  size -= sizeof(struct memdump_backtrace_s);
-  buf = ptr + size;
-#endif
 
   if (used)
     {
 #if CONFIG_MM_BACKTRACE < 0
       if (dump->pid == PID_MM_ALLOC)
 #else
+      FAR struct memdump_backtrace_s *buf =
+        ptr + size - sizeof(struct memdump_backtrace_s);
+
       if ((dump->pid == PID_MM_ALLOC || dump->pid == buf->pid) &&
           buf->seqno >= dump->seqmin && buf->seqno <= dump->seqmax)
 #endif
         {
 #if CONFIG_MM_BACKTRACE < 0
           syslog(LOG_INFO, "%12zu%*p\n", size, MM_PTR_FMT_WIDTH, ptr);
 #else
+          char tmp[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] =

Review Comment:
   Ditto



##########
mm/mempool/mempool.c:
##########
@@ -460,55 +462,55 @@ mempool_info_task(FAR struct mempool_s *pool,
 void mempool_memdump(FAR struct mempool_s *pool,
                      FAR const struct mm_memdump_s *dump)
 {
+  size_t blocksize = MEMPOOL_REALBLOCKSIZE(pool);
+
   if (dump->pid == PID_MM_FREE)
     {
       FAR sq_entry_t *entry;
 
       sq_for_every(&pool->queue, entry)
         {
           syslog(LOG_INFO, "%12zu%*p\n",
-                 pool->blocksize, MM_PTR_FMT_WIDTH,
-                 (FAR char *)entry);
+                 blocksize, MM_PTR_FMT_WIDTH, (FAR char *)entry);
         }
 
       sq_for_every(&pool->iqueue, entry)
         {
           syslog(LOG_INFO, "%12zu%*p\n",
-                 pool->blocksize, MM_PTR_FMT_WIDTH,
-                 (FAR char *)entry);
+                 blocksize, MM_PTR_FMT_WIDTH, (FAR char *)entry);
         }
     }
 #if CONFIG_MM_BACKTRACE >= 0
   else
     {
       FAR struct mempool_backtrace_s *buf;
 
-      list_for_every_entry(&pool->alist, buf, struct mempool_backtrace_s,
-                           node)
+      list_for_every_entry(&pool->alist, buf,
+                           struct mempool_backtrace_s, node)
         {
           if ((dump->pid == PID_MM_ALLOC || dump->pid == buf->pid) &&
               buf->seqno >= dump->seqmin && buf->seqno <= dump->seqmax)
             {
+              char tmp[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] =
+              {
+                '\0'
+              };
+
 #  if CONFIG_MM_BACKTRACE > 0
-              int i;
               FAR const char *format = " %0*p";
-#  endif
-              char bt[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1];
+              int i;
 
-              bt[0] = '\0';
-#  if CONFIG_MM_BACKTRACE > 0
               for (i = 0; i < CONFIG_MM_BACKTRACE && buf->backtrace[i]; i++)
                 {
-                  snprintf(bt + i * MM_PTR_FMT_WIDTH,
-                           sizeof(bt) - i * MM_PTR_FMT_WIDTH,
+                  snprintf(tmp + i * MM_PTR_FMT_WIDTH,
+                           sizeof(tmp) - i * MM_PTR_FMT_WIDTH,
                            format, MM_PTR_FMT_WIDTH - 1, buf->backtrace[i]);
                 }
 #  endif
 
               syslog(LOG_INFO, "%6d%12zu%12lu%*p%s\n",
-                     (int)buf->pid, pool->blocksize, buf->seqno,
-                     MM_PTR_FMT_WIDTH,
-                     ((FAR char *)buf - pool->blocksize), bt);
+                     (int)buf->pid, blocksize, buf->seqno,

Review Comment:
   Do we need `int` cast here?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9564: Compute the heap info more accuracy(2)

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9564:
URL: https://github.com/apache/nuttx/pull/9564#discussion_r1233542436


##########
mm/mm_heap/mm_memdump.c:
##########
@@ -70,14 +70,15 @@ static void memdump_handler(FAR struct mm_allocnode_s *node, FAR void *arg)
                  nodesize, MM_PTR_FMT_WIDTH,
                  ((FAR char *)node + SIZEOF_MM_ALLOCNODE));
 #else
+          char buf[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] =
+          {
+            '\0'
+          };

Review Comment:
   Done.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9564: Compute the heap info more accuracy(2)

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9564:
URL: https://github.com/apache/nuttx/pull/9564#discussion_r1233455020


##########
mm/mempool/mempool.c:
##########
@@ -460,55 +462,55 @@ mempool_info_task(FAR struct mempool_s *pool,
 void mempool_memdump(FAR struct mempool_s *pool,
                      FAR const struct mm_memdump_s *dump)
 {
+  size_t blocksize = MEMPOOL_REALBLOCKSIZE(pool);
+
   if (dump->pid == PID_MM_FREE)
     {
       FAR sq_entry_t *entry;
 
       sq_for_every(&pool->queue, entry)
         {
           syslog(LOG_INFO, "%12zu%*p\n",
-                 pool->blocksize, MM_PTR_FMT_WIDTH,
-                 (FAR char *)entry);
+                 blocksize, MM_PTR_FMT_WIDTH, (FAR char *)entry);
         }
 
       sq_for_every(&pool->iqueue, entry)
         {
           syslog(LOG_INFO, "%12zu%*p\n",
-                 pool->blocksize, MM_PTR_FMT_WIDTH,
-                 (FAR char *)entry);
+                 blocksize, MM_PTR_FMT_WIDTH, (FAR char *)entry);
         }
     }
 #if CONFIG_MM_BACKTRACE >= 0
   else
     {
       FAR struct mempool_backtrace_s *buf;
 
-      list_for_every_entry(&pool->alist, buf, struct mempool_backtrace_s,
-                           node)
+      list_for_every_entry(&pool->alist, buf,
+                           struct mempool_backtrace_s, node)
         {
           if ((dump->pid == PID_MM_ALLOC || dump->pid == buf->pid) &&
               buf->seqno >= dump->seqmin && buf->seqno <= dump->seqmax)
             {
+              char tmp[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] =
+              {
+                '\0'
+              };

Review Comment:
   Done.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9564: Compute the heap info more accuracy(2)

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9564:
URL: https://github.com/apache/nuttx/pull/9564#discussion_r1233490546


##########
mm/mm_heap/mm_memdump.c:
##########
@@ -70,14 +70,15 @@ static void memdump_handler(FAR struct mm_allocnode_s *node, FAR void *arg)
                  nodesize, MM_PTR_FMT_WIDTH,
                  ((FAR char *)node + SIZEOF_MM_ALLOCNODE));
 #else
+          char buf[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] =
+          {
+            '\0'
+          };

Review Comment:
   ```suggestion
             char buf[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] = "";
   ```
   



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9564: Compute the heap info more accuracy(2)

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9564:
URL: https://github.com/apache/nuttx/pull/9564#discussion_r1233481003


##########
mm/mempool/mempool.c:
##########
@@ -460,55 +462,55 @@ mempool_info_task(FAR struct mempool_s *pool,
 void mempool_memdump(FAR struct mempool_s *pool,
                      FAR const struct mm_memdump_s *dump)
 {
+  size_t blocksize = MEMPOOL_REALBLOCKSIZE(pool);
+
   if (dump->pid == PID_MM_FREE)
     {
       FAR sq_entry_t *entry;
 
       sq_for_every(&pool->queue, entry)
         {
           syslog(LOG_INFO, "%12zu%*p\n",
-                 pool->blocksize, MM_PTR_FMT_WIDTH,
-                 (FAR char *)entry);
+                 blocksize, MM_PTR_FMT_WIDTH, (FAR char *)entry);
         }
 
       sq_for_every(&pool->iqueue, entry)
         {
           syslog(LOG_INFO, "%12zu%*p\n",
-                 pool->blocksize, MM_PTR_FMT_WIDTH,
-                 (FAR char *)entry);
+                 blocksize, MM_PTR_FMT_WIDTH, (FAR char *)entry);
         }
     }
 #if CONFIG_MM_BACKTRACE >= 0
   else
     {
       FAR struct mempool_backtrace_s *buf;
 
-      list_for_every_entry(&pool->alist, buf, struct mempool_backtrace_s,
-                           node)
+      list_for_every_entry(&pool->alist, buf,
+                           struct mempool_backtrace_s, node)
         {
           if ((dump->pid == PID_MM_ALLOC || dump->pid == buf->pid) &&
               buf->seqno >= dump->seqmin && buf->seqno <= dump->seqmax)
             {
+              char tmp[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] =
+              {
+                '\0'
+              };
+
 #  if CONFIG_MM_BACKTRACE > 0
-              int i;
               FAR const char *format = " %0*p";
-#  endif
-              char bt[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1];
+              int i;
 
-              bt[0] = '\0';
-#  if CONFIG_MM_BACKTRACE > 0
               for (i = 0; i < CONFIG_MM_BACKTRACE && buf->backtrace[i]; i++)
                 {
-                  snprintf(bt + i * MM_PTR_FMT_WIDTH,
-                           sizeof(bt) - i * MM_PTR_FMT_WIDTH,
+                  snprintf(tmp + i * MM_PTR_FMT_WIDTH,
+                           sizeof(tmp) - i * MM_PTR_FMT_WIDTH,
                            format, MM_PTR_FMT_WIDTH - 1, buf->backtrace[i]);
                 }
 #  endif
 
               syslog(LOG_INFO, "%6d%12zu%12lu%*p%s\n",
-                     (int)buf->pid, pool->blocksize, buf->seqno,
-                     MM_PTR_FMT_WIDTH,
-                     ((FAR char *)buf - pool->blocksize), bt);
+                     (int)buf->pid, blocksize, buf->seqno,

Review Comment:
   Done.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9564: Compute the heap info more accuracy(2)

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9564:
URL: https://github.com/apache/nuttx/pull/9564#discussion_r1233455083


##########
mm/tlsf/mm_tlsf.c:
##########
@@ -404,33 +399,31 @@ static void memdump_handler(FAR void *ptr, size_t size, int used,
                             FAR void *user)
 {
   FAR const struct mm_memdump_s *dump = user;
-#if CONFIG_MM_BACKTRACE >= 0
-  FAR struct memdump_backtrace_s *buf;
-
-  size -= sizeof(struct memdump_backtrace_s);
-  buf = ptr + size;
-#endif
 
   if (used)
     {
 #if CONFIG_MM_BACKTRACE < 0
       if (dump->pid == PID_MM_ALLOC)
 #else
+      FAR struct memdump_backtrace_s *buf =
+        ptr + size - sizeof(struct memdump_backtrace_s);
+
       if ((dump->pid == PID_MM_ALLOC || dump->pid == buf->pid) &&
           buf->seqno >= dump->seqmin && buf->seqno <= dump->seqmax)
 #endif
         {
 #if CONFIG_MM_BACKTRACE < 0
           syslog(LOG_INFO, "%12zu%*p\n", size, MM_PTR_FMT_WIDTH, ptr);
 #else
+          char tmp[CONFIG_MM_BACKTRACE * MM_PTR_FMT_WIDTH + 1] =

Review Comment:
   Done.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko merged pull request #9564: Compute the heap info more accuracy(2)

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko merged PR #9564:
URL: https://github.com/apache/nuttx/pull/9564


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org