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 2023/01/11 12:12:39 UTC
[GitHub] [nuttx] jlaitine opened a new pull request, #8085: Add common virtual region allocator
jlaitine opened a new pull request, #8085:
URL: https://github.com/apache/nuttx/pull/8085
This is a suggestion to move virtual memory region allocation from mm/shm to a more common place. There are many use cases where one needs to map page pool pages into virtual memory, and for those cases it should not be necessary to configure the mm/shm (which is the system-v shared memory interface) into use.
The change has no functional impact, only the gran allocator which keeps track of the mapped virtual memory areas is moved from mm/shm into mm/map.
The gran allocator is initialized together with task_group initialization for user processes. Previously this was initialized in binfmt.
--
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] jlaitine commented on a diff in pull request #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1067747251
##########
mm/shm/shmat.c:
##########
@@ -80,7 +80,8 @@ static int munmap_shm(FAR struct task_group_s *group,
{
/* Free the virtual address space */
- shm_free(group, (FAR void *)shmaddr, region->sr_ds.shm_segsz);
+ vm_release_region(get_group_mm(group), (FAR void *)shmaddr,
Review Comment:
done, good catch
--
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] jlaitine commented on a diff in pull request #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1068309757
##########
arch/Kconfig:
##########
@@ -568,7 +568,7 @@ config ARCH_HEAP_NPAGES
This, along with knowledge of the page size, determines the size of
the heap virtual address space. Default is 1.
-if MM_SHM
+if ARCH_VMA_MAPPING
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 #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1068128488
##########
arch/Kconfig:
##########
@@ -568,7 +568,7 @@ config ARCH_HEAP_NPAGES
This, along with knowledge of the page size, determines the size of
the heap virtual address space. Default is 1.
-if MM_SHM
+if ARCH_VMA_MAPPING
Review Comment:
merge to the second patch?
--
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 #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1068129604
##########
include/nuttx/mm/map.h:
##########
@@ -73,6 +74,11 @@ struct mm_map_entry_s
struct mm_map_s
{
sq_queue_t mm_map_sq;
+
+#ifdef CONFIG_ARCH_VMA_MAPPING
+ GRAN_HANDLE mm_vpages;
Review Comment:
let's unify the prefix mm_map_ v.s. mm_
--
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 #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1067329512
##########
include/nuttx/mm/map.h:
##########
@@ -162,6 +169,46 @@ void mm_map_destroy(FAR struct mm_map_s *mm);
*
****************************************************************************/
+#ifdef CONFIG_ARCH_VMA_MAPPING
Review Comment:
should we emulate with kmm_malloc/umm_malloc in case of CONFIG_ARCH_VMA_MAPPING == n?
--
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] jlaitine commented on a diff in pull request #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1067751260
##########
include/nuttx/mm/map.h:
##########
@@ -162,6 +169,46 @@ void mm_map_destroy(FAR struct mm_map_s *mm);
*
****************************************************************************/
+#ifdef CONFIG_ARCH_VMA_MAPPING
Review Comment:
Not in this place I think.
This only keeps track of virtual memory reservations with page granularity, so that it is easy to find a free virtual memory and find an address for a process to map memory to. And in theory this same info could be extracted from the list of mappings as well, it would just be a bit more complicated.
This info is not needed at all when you don't have virtual memory/MMU, it is easier to just return the physical address in mmap, as what is done already in rammap and anonmap (and what we do also in shmfs for flat&protected modes).
So, IMHO, there is nothing to re-use in here for rammap and anonmap.
I do realize that in rammaps and anonmaps the physical memory is also reserved in mmap, but this is not the common case; if it was shared mapping, you wouldn't allocate anything there.
--
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] jlaitine commented on a diff in pull request #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1067751260
##########
include/nuttx/mm/map.h:
##########
@@ -162,6 +169,46 @@ void mm_map_destroy(FAR struct mm_map_s *mm);
*
****************************************************************************/
+#ifdef CONFIG_ARCH_VMA_MAPPING
Review Comment:
Not in this place I think.
This only keeps track of virtual memory reservations with page granularity, so that it is easy to find a free virtual memory and find an address for a process to map memory to. And in theory this same info could be extracted from the list of mappings as well, it would just be a bit more complicated.
This info is not needed at all when you don't have virtual memory/MMU, it is easier to just return the physical address in mmap, as what is done already in rammap and anonmap (and what we do also in shmfs for flat&protected modes).
So, IMHO, there is nothing to re-use in here for rammap and anonmap.
--
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 #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1067293203
##########
mm/shm/shmat.c:
##########
@@ -80,7 +80,8 @@ static int munmap_shm(FAR struct task_group_s *group,
{
/* Free the virtual address space */
- shm_free(group, (FAR void *)shmaddr, region->sr_ds.shm_segsz);
+ vm_release_region(get_group_mm(group), (FAR void *)shmaddr,
Review Comment:
remove the const from line 50 and the 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 #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1067329512
##########
include/nuttx/mm/map.h:
##########
@@ -162,6 +169,46 @@ void mm_map_destroy(FAR struct mm_map_s *mm);
*
****************************************************************************/
+#ifdef CONFIG_ARCH_VMA_MAPPING
Review Comment:
should we emulate with kmm_malloc/umm_malloc in case of CONFIG_ARCH_VMA_MAPPING == n? So, we can reuse the code for rammap and anonmap.
--
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] jlaitine commented on a diff in pull request #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1068310200
##########
include/nuttx/mm/map.h:
##########
@@ -73,6 +74,11 @@ struct mm_map_entry_s
struct mm_map_s
{
sq_queue_t mm_map_sq;
+
+#ifdef CONFIG_ARCH_VMA_MAPPING
+ GRAN_HANDLE mm_vpages;
Review Comment:
done, thanks
--
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] jlaitine commented on a diff in pull request #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#discussion_r1067751260
##########
include/nuttx/mm/map.h:
##########
@@ -162,6 +169,46 @@ void mm_map_destroy(FAR struct mm_map_s *mm);
*
****************************************************************************/
+#ifdef CONFIG_ARCH_VMA_MAPPING
Review Comment:
Not in this place I think.
This only keeps track of virtual memory reservations with page granularity, so that it is easy to find a free virtual memory and find an address for a process to map memory to. And in theory this same info could be extracted from the list of mappings as well, it would just be a bit more complicated.
This info is not needed at all when you don't have virtual memory/MMU, it is easier to just return the physical address in mmap, as what is done already in rammap and anonmap (and what we do also in shmfs for flat&protected modes).
So, IMHO, there is nothing to re-use in here for rammap and anonmap.
I do realize that in rammaps and anonmaps the physical memory is also reserved in mmap, but this is not the common case; if it was shared mapping, you would only allocate once there, not on every mmap call. But on every mmap call you'd need to find the virtual memory for the process to create a new mapping to the allocated physical memory.
--
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] jlaitine commented on pull request #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
jlaitine commented on PR #8085:
URL: https://github.com/apache/nuttx/pull/8085#issuecomment-1379908033
I just realized that for CONFIG_ARCH_VMA_MAPPING to be usable, there is a bit more that needs to be under that flag, instead of CONFIG_MM_SHM. I pushed one more patch on top of this PR
--
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 merged pull request #8085: Add common virtual region allocator
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #8085:
URL: https://github.com/apache/nuttx/pull/8085
--
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