[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 04/14] softmmu/memory: Pass ram_flags to qemu_ram_alloc_fr
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v5 04/14] softmmu/memory: Pass ram_flags to qemu_ram_alloc_from_fd() |
Date: |
Tue, 20 Apr 2021 12:45:49 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 4/20/21 12:36 PM, David Hildenbrand wrote:
> On 20.04.21 12:18, Philippe Mathieu-Daudé wrote:
>> Hi David,
>>
>> On 4/20/21 11:52 AM, Philippe Mathieu-Daudé wrote:
>>> On 4/13/21 11:14 AM, David Hildenbrand wrote:
>>>> Let's pass in ram flags just like we do with
>>>> qemu_ram_alloc_from_file(),
>>>> to clean up and prepare for more flags.
>>>>
>>>> Simplify the documentation of passed ram flags: Looking at our
>>>> documentation of RAM_SHARED and RAM_PMEM is sufficient, no need to be
>>>> repetitive.
>>>>
>>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>> backends/hostmem-memfd.c | 7 ++++---
>>>> hw/misc/ivshmem.c | 5 ++---
>>>> include/exec/memory.h | 9 +++------
>>>> include/exec/ram_addr.h | 6 +-----
>>>> softmmu/memory.c | 7 +++----
>>>> 5 files changed, 13 insertions(+), 21 deletions(-)
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>
>>
>> Actually it would be clearer to define the 0 value, maybe:
>>
>> #define RAM_NOFLAG (0 << 0)
>>
>
> This will also turn some code into
>
> ram_flags = backend->share ? RAM_SHARED : RAM_NOFLAG;
> ram_flags |= backend->reserve ? RAM_NOFLAG : RAM_NORESERVE;
This is the callee view, withing the API, where you have all
the context.
> Looking at other flag users, I barely see any such usage.
> XKB_CONTEXT_NO_FLAGS, ALLOC_NO_FLAGS, and MEM_AFFINITY_NOFLAGS seem to
> be the only ones. That's why I tend to not do it unless there are strong
> opinions.
I'm more concerned about the caller perspective. What means this
magic '0' in the arguments? Then I have to check the prototype.
If the caller uses RAM_NO_FLAGS, I directly understand what is passed.
Anyway my comment fits the usual "can be cleaned later" case.
Thanks,
Phil.
[PATCH v5 05/14] softmmu/memory: Pass ram_flags to memory_region_init_ram_shared_nomigrate(), David Hildenbrand, 2021/04/13
[PATCH v5 07/14] memory: Introduce RAM_NORESERVE and wire it up in qemu_ram_mmap(), David Hildenbrand, 2021/04/13
[PATCH v5 06/14] util/mmap-alloc: Pass flags instead of separate bools to qemu_ram_mmap(), David Hildenbrand, 2021/04/13