Re: [Qemu-devel] [PATCH v3 08/32] exec: allow memory to be allocated from any kind of path

[Xiao Guangrong <guangrong.xiao@linux.intel.com>]

On 10/12/2015 06:08 PM, Michael S. Tsirkin wrote:
> On Sun, Oct 11, 2015 at 11:52:40AM +0800, Xiao Guangrong wrote:
>> Currently file_ram_alloc() is designed for hugetlbfs, however, the memory
>> of nvdimm can come from either raw pmem device eg, /dev/pmem, or the file
>> locates at DAX enabled filesystem
>>
>> So this patch let it work on any kind of path
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>
> This conflicts with map alloc rework.
> Please rebase this on top of my tree.
>

Okay, thanks for your reminder. I did it based on upstream QEMU tree,
will do it on pci branch on your tree instead.

>
>> ---
>>   exec.c | 55 ++++++++++++++-----------------------------------------
>>   1 file changed, 14 insertions(+), 41 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 7d90a52..70cb0ef 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1154,32 +1154,6 @@ void qemu_mutex_unlock_ramlist(void)
>>   }
>>
>>   #ifdef __linux__
>> -
>> -#include <sys/vfs.h>
>> -
>> -#define HUGETLBFS_MAGIC       0x958458f6
>> -
>> -static long gethugepagesize(const char *path, Error **errp)
>> -{
>> -    struct statfs fs;
>> -    int ret;
>> -
>> -    do {
>> -        ret = statfs(path, &fs);
>> -    } while (ret != 0 && errno == EINTR);
>> -
>> -    if (ret != 0) {
>> -        error_setg_errno(errp, errno, "failed to get page size of file %s",
>> -                         path);
>> -        return 0;
>> -    }
>> -
>> -    if (fs.f_type != HUGETLBFS_MAGIC)
>> -        fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path);
>> -
>> -    return fs.f_bsize;
>
> What this *actually* is trying to warn against is that
> mapping a regular file (as opposed to hugetlbfs)
> means transparent huge pages don't work.
>
> So I don't think we should drop this warning completely.
> Either let's add the nvdimm magic, or simply check the
> page size.

Check the page size sounds good, will check:
if (pagesize != getpagesize()) {
	...print something...
}

I agree with you that showing the info is needed, however,
'Warning' might scare some users, how about drop this word or
just show　“Memory is not allocated from HugeTlbfs”?