| Message ID | 1499438577-7674-3-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | a5c0234bb2754f5248e67929a34c843dbe039da5 |
| Headers | show |
| Series | Make memory_region_init_ram() and friends handle migration | expand |
On 07/07/2017 16:42, Peter Maydell wrote: > @@ -522,6 +537,9 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, > * @name: the name of the region. > * @size: size of the region. > * @ptr: memory to be mapped; must contain at least @size bytes. > + * > + * Note that this function does not do anything to cause the data in the > + * RAM memory region to be migrated; that is the responsibility of the caller. Perhaps add a note that it rarely makes sense for this function? Paolo > */ > void memory_region_init_ram_device_ptr(MemoryRegion *mr, > struct Object *owner, > --
On 10 July 2017 at 11:01, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 07/07/2017 16:42, Peter Maydell wrote: >> @@ -522,6 +537,9 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, >> * @name: the name of the region. >> * @size: size of the region. >> * @ptr: memory to be mapped; must contain at least @size bytes. >> + * >> + * Note that this function does not do anything to cause the data in the >> + * RAM memory region to be migrated; that is the responsibility of the caller. > > Perhaps add a note that it rarely makes sense for this function? Well, we have 4 callers of this function, and 3 of those register the RAM for migration, so the rare case seems to be the "don't need to handle migration"... thanks -- PMM
On 10/07/2017 12:04, Peter Maydell wrote: > On 10 July 2017 at 11:01, Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> >> On 07/07/2017 16:42, Peter Maydell wrote: >>> @@ -522,6 +537,9 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, >>> * @name: the name of the region. >>> * @size: size of the region. >>> * @ptr: memory to be mapped; must contain at least @size bytes. >>> + * >>> + * Note that this function does not do anything to cause the data in the >>> + * RAM memory region to be migrated; that is the responsibility of the caller. >> >> Perhaps add a note that it rarely makes sense for this function? > > Well, we have 4 callers of this function, and 3 of those > register the RAM for migration, so the rare case seems to be > the "don't need to handle migration"... Oops, the diff header is confusing and I didn't notice that when trimming. My comment refers to RAM device regions. Paolo
On 10 July 2017 at 11:05, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 10/07/2017 12:04, Peter Maydell wrote: >> On 10 July 2017 at 11:01, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> >>> >>> On 07/07/2017 16:42, Peter Maydell wrote: >>>> @@ -522,6 +537,9 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, >>>> * @name: the name of the region. >>>> * @size: size of the region. >>>> * @ptr: memory to be mapped; must contain at least @size bytes. >>>> + * >>>> + * Note that this function does not do anything to cause the data in the >>>> + * RAM memory region to be migrated; that is the responsibility of the caller. >>> >>> Perhaps add a note that it rarely makes sense for this function? >> >> Well, we have 4 callers of this function, and 3 of those >> register the RAM for migration, so the rare case seems to be >> the "don't need to handle migration"... > > Oops, the diff header is confusing and I didn't notice that when > trimming. My comment refers to RAM device regions. Yeah, no objection to adding a note for that one. thanks -- PMM
diff --git a/include/exec/memory.h b/include/exec/memory.h index 8503685..55cb5e7 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -412,6 +412,9 @@ void memory_region_init_io(MemoryRegion *mr, * must be unique within any device * @size: size of the region. * @errp: pointer to Error*, to store an error if it happens. + * + * Note that this function does not do anything to cause the data in the + * RAM memory region to be migrated; that is the responsibility of the caller. */ void memory_region_init_ram(MemoryRegion *mr, struct Object *owner, @@ -434,6 +437,9 @@ void memory_region_init_ram(MemoryRegion *mr, * @max_size: max size of the region. * @resized: callback to notify owner about used size change. * @errp: pointer to Error*, to store an error if it happens. + * + * Note that this function does not do anything to cause the data in the + * RAM memory region to be migrated; that is the responsibility of the caller. */ void memory_region_init_resizeable_ram(MemoryRegion *mr, struct Object *owner, @@ -457,6 +463,9 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr, * @share: %true if memory must be mmaped with the MAP_SHARED flag * @path: the path in which to allocate the RAM. * @errp: pointer to Error*, to store an error if it happens. + * + * Note that this function does not do anything to cause the data in the + * RAM memory region to be migrated; that is the responsibility of the caller. */ void memory_region_init_ram_from_file(MemoryRegion *mr, struct Object *owner, @@ -477,6 +486,9 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, * @share: %true if memory must be mmaped with the MAP_SHARED flag * @fd: the fd to mmap. * @errp: pointer to Error*, to store an error if it happens. + * + * Note that this function does not do anything to cause the data in the + * RAM memory region to be migrated; that is the responsibility of the caller. */ void memory_region_init_ram_from_fd(MemoryRegion *mr, struct Object *owner, @@ -498,6 +510,9 @@ void memory_region_init_ram_from_fd(MemoryRegion *mr, * must be unique within any device * @size: size of the region. * @ptr: memory to be mapped; must contain at least @size bytes. + * + * Note that this function does not do anything to cause the data in the + * RAM memory region to be migrated; that is the responsibility of the caller. */ void memory_region_init_ram_ptr(MemoryRegion *mr, struct Object *owner, @@ -522,6 +537,9 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, * @name: the name of the region. * @size: size of the region. * @ptr: memory to be mapped; must contain at least @size bytes. + * + * Note that this function does not do anything to cause the data in the + * RAM memory region to be migrated; that is the responsibility of the caller. */ void memory_region_init_ram_device_ptr(MemoryRegion *mr, struct Object *owner,
The various functions for initializing RAM MemoryRegions do not do anything to cause the data in the MemoryRegion to be migrated. Note in their documentation comments that this is the responsibility of the caller. (We will shortly add a new function that *does* do this for you.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/memory.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) -- 2.7.4