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 <pet...@redhat.com> >>>> Signed-off-by: David Hildenbrand <da...@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é <phi...@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.