Re: [Qemu-devel] [PATCH 2/7 V10] memory, exec: switch file ram allocation functions to 'flags' parameters

2018-07-17 Thread He, Junyan
OK, I wil chang all flags to 32bits
It is a little strange that the compiler give no warning when losing precision.


-Original Message-
From: Richard Henderson [mailto:richard.hender...@linaro.org] 
Sent: Wednesday, July 18, 2018 12:03 AM
To: junyan...@gmx.com; qemu-devel@nongnu.org
Cc: ehabk...@redhat.com; imamm...@redhat.com; pbonz...@redhat.com; 
crosthwaite.pe...@gmail.com; xiaoguangrong.e...@gmail.com; m...@redhat.com; 
quint...@redhat.com; dgilb...@redhat.com; stefa...@redhat.com; Zhang, Yi Z 
; He, Junyan ; Haozhong Zhang 

Subject: Re: [PATCH 2/7 V10] memory, exec: switch file ram allocation functions 
to 'flags' parameters

On 07/16/2018 11:32 PM, junyan...@gmx.com wrote:
>  RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
> - bool share, int fd,
> + uint64_t ram_flags, int fd,
>   Error **errp)  {
>  RAMBlock *new_block;
> @@ -2280,14 +2280,14 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, 
> MemoryRegion *mr,
>  new_block->mr = mr;
>  new_block->used_length = size;
>  new_block->max_length = size;
> -new_block->flags = share ? RAM_SHARED : 0;
> +new_block->flags = ram_flags;

The type of "flags" within RAMBlock is uint32_t.

You should either change the member type in the struct, or change the argument 
type in all of these functions.

More likely the latter, since you seem to have just five used bits at the 
moment.


r~


Re: [Qemu-devel] [PATCH 2/7 V10] memory, exec: switch file ram allocation functions to 'flags' parameters

2018-07-17 Thread Richard Henderson
On 07/16/2018 11:32 PM, junyan...@gmx.com wrote:
>  RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
> - bool share, int fd,
> + uint64_t ram_flags, int fd,
>   Error **errp)
>  {
>  RAMBlock *new_block;
> @@ -2280,14 +2280,14 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, 
> MemoryRegion *mr,
>  new_block->mr = mr;
>  new_block->used_length = size;
>  new_block->max_length = size;
> -new_block->flags = share ? RAM_SHARED : 0;
> +new_block->flags = ram_flags;

The type of "flags" within RAMBlock is uint32_t.

You should either change the member type in the struct,
or change the argument type in all of these functions.

More likely the latter, since you seem to have just
five used bits at the moment.


r~



[Qemu-devel] [PATCH 2/7 V10] memory, exec: switch file ram allocation functions to 'flags' parameters

2018-07-17 Thread junyan . he
From: Junyan He 

As more flag parameters besides the existing 'share' are going to be
added to following functions
memory_region_init_ram_from_file
qemu_ram_alloc_from_fd
qemu_ram_alloc_from_file
let's switch them to use the 'flags' parameters so as to ease future
flag additions.

The existing 'share' flag is converted to the RAM_SHARED bit in ram_flags,
and other flag bits are ignored by above functions right now.

Signed-off-by: Junyan He 
Signed-off-by: Haozhong Zhang 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Igor Mammedov 
---
 backends/hostmem-file.c |  3 ++-
 exec.c  | 10 +-
 include/exec/memory.h   |  7 +--
 include/exec/ram_addr.h | 25 +++--
 memory.c|  8 +---
 numa.c  |  2 +-
 6 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 134b08d..34c68bb 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -58,7 +58,8 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 path = object_get_canonical_path(OBJECT(backend));
 memory_region_init_ram_from_file(>mr, OBJECT(backend),
  path,
- backend->size, fb->align, backend->share,
+ backend->size, fb->align,
+ backend->share ? RAM_SHARED : 0,
  fb->mem_path, errp);
 g_free(path);
 }
diff --git a/exec.c b/exec.c
index cc042dc..1ec539d 100644
--- a/exec.c
+++ b/exec.c
@@ -2238,7 +2238,7 @@ static void ram_block_add(RAMBlock *new_block, Error 
**errp, bool shared)
 
 #ifdef __linux__
 RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
- bool share, int fd,
+ uint64_t ram_flags, int fd,
  Error **errp)
 {
 RAMBlock *new_block;
@@ -2280,14 +2280,14 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, 
MemoryRegion *mr,
 new_block->mr = mr;
 new_block->used_length = size;
 new_block->max_length = size;
-new_block->flags = share ? RAM_SHARED : 0;
+new_block->flags = ram_flags;
 new_block->host = file_ram_alloc(new_block, size, fd, !file_size, errp);
 if (!new_block->host) {
 g_free(new_block);
 return NULL;
 }
 
-ram_block_add(new_block, _err, share);
+ram_block_add(new_block, _err, ram_flags & RAM_SHARED);
 if (local_err) {
 g_free(new_block);
 error_propagate(errp, local_err);
@@ -2299,7 +2299,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, 
MemoryRegion *mr,
 
 
 RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
-   bool share, const char *mem_path,
+   uint64_t ram_flags, const char *mem_path,
Error **errp)
 {
 int fd;
@@ -2311,7 +2311,7 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, 
MemoryRegion *mr,
 return NULL;
 }
 
-block = qemu_ram_alloc_from_fd(size, mr, share, fd, errp);
+block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, errp);
 if (!block) {
 if (created) {
 unlink(mem_path);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 6d0af29..513ec8d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -640,6 +640,7 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
void *host),
Error **errp);
 #ifdef __linux__
+
 /**
  * memory_region_init_ram_from_file:  Initialize RAM memory region with a
  *mmap-ed backend.
@@ -651,7 +652,9 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
  * @size: size of the region.
  * @align: alignment of the region base address; if 0, the default alignment
  * (getpagesize()) will be used.
- * @share: %true if memory must be mmaped with the MAP_SHARED flag
+ * @ram_flags: Memory region features:
+ * - RAM_SHARED: memory must be mmaped with the MAP_SHARED flag
+ * Other bits are ignored now.
  * @path: the path in which to allocate the RAM.
  * @errp: pointer to Error*, to store an error if it happens.
  *
@@ -663,7 +666,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
   const char *name,
   uint64_t size,
   uint64_t align,
-  bool share,
+  uint64_t ram_flags,
   const char *path,
   Error **errp);
 
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index cf4ce06..bb0a09b 100644
---