[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] exec: fetch the alignment of Linux devdax pmem character dev
From: |
Paolo Bonzini |
Subject: |
Re: [PATCH] exec: fetch the alignment of Linux devdax pmem character device nodes |
Date: |
Tue, 7 Apr 2020 10:08:29 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 07/04/20 09:29, Liu, Jingqi wrote:
> Ping.
>
> Any comments are appreciated.
>
> Hi Paolo, Richard,
>
> Any comments about this ?
I was hoping to get a review from someone else because I have no way to
test it. But I've now queued the patch, thanks.
Paolo
>
> Thanks,
>
> Jingqi
>
> On 4/1/2020 11:13 AM, Liu, Jingqi wrote:
>> If the backend file is devdax pmem character device, the alignment
>> specified by the option 'align=NUM' in the '-object memory-backend-file'
>> needs to match the alignment requirement of the devdax pmem character
>> device.
>>
>> This patch fetches the devdax pmem file 'align', so that we can compare
>> it with the NUM of 'align=NUM'.
>> The NUM needs to be larger than or equal to the devdax pmem file 'align'.
>>
>> It also fixes the problem that mmap() returns failure in qemu_ram_mmap()
>> when the NUM of 'align=NUM' is less than the devdax pmem file 'align'.
>>
>> Cc: Dan Williams <address@hidden>
>> Signed-off-by: Jingqi Liu <address@hidden>
>> ---
>> exec.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/exec.c b/exec.c
>> index de9d949902..8221abffec 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1736,6 +1736,42 @@ static int64_t get_file_size(int fd)
>> return size;
>> }
>> +static int64_t get_file_align(int fd)
>> +{
>> + int64_t align = -1;
>> +#if defined(__linux__)
>> + struct stat st;
>> +
>> + if (fstat(fd, &st) < 0) {
>> + return -errno;
>> + }
>> +
>> + /* Special handling for devdax character devices */
>> + if (S_ISCHR(st.st_mode)) {
>> + g_autofree char *subsystem_path = NULL;
>> + g_autofree char *subsystem = NULL;
>> +
>> + subsystem_path =
>> g_strdup_printf("/sys/dev/char/%d:%d/subsystem",
>> + major(st.st_rdev),
>> minor(st.st_rdev));
>> + subsystem = g_file_read_link(subsystem_path, NULL);
>> +
>> + if (subsystem && g_str_has_suffix(subsystem, "/dax")) {
>> + g_autofree char *align_path = NULL;
>> + g_autofree char *align_str = NULL;
>> +
>> + align_path =
>> g_strdup_printf("/sys/dev/char/%d:%d/device/align",
>> + major(st.st_rdev),
>> minor(st.st_rdev));
>> +
>> + if (g_file_get_contents(align_path, &align_str, NULL,
>> NULL)) {
>> + return g_ascii_strtoll(align_str, NULL, 0);
>> + }
>> + }
>> + }
>> +#endif /* defined(__linux__) */
>> +
>> + return align;
>> +}
>> +
>> static int file_ram_open(const char *path,
>> const char *region_name,
>> bool *created,
>> @@ -2275,7 +2311,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t
>> size, MemoryRegion *mr,
>> {
>> RAMBlock *new_block;
>> Error *local_err = NULL;
>> - int64_t file_size;
>> + int64_t file_size, file_align;
>> /* Just support these ram flags by now. */
>> assert((ram_flags & ~(RAM_SHARED | RAM_PMEM)) == 0);
>> @@ -2311,6 +2347,14 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t
>> size, MemoryRegion *mr,
>> return NULL;
>> }
>> + file_align = get_file_align(fd);
>> + if (file_align > 0 && mr && file_align > mr->align) {
>> + error_setg(errp, "backing store align 0x%" PRIx64
>> + " is larger than 'align' option 0x" RAM_ADDR_FMT,
>> + file_align, mr->align);
>> + return NULL;
>> + }
>> +
>> new_block = g_malloc0(sizeof(*new_block));
>> new_block->mr = mr;
>> new_block->used_length = size;
>