From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:33532)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1acuy5-0004Pr-JF
	for qemu-devel@nongnu.org; Mon, 07 Mar 2016 08:13:02 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1acuy0-0006Vo-GX
	for qemu-devel@nongnu.org; Mon, 07 Mar 2016 08:13:01 -0500
Received: from mx1.redhat.com ([209.132.183.28]:54162)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1acuy0-0006Vk-AW
	for qemu-devel@nongnu.org; Mon, 07 Mar 2016 08:12:56 -0500
References: <1456771254-17511-1-git-send-email-armbru@redhat.com>
	<1456771254-17511-2-git-send-email-armbru@redhat.com>
	<56D57E76.1080402@redhat.com> <87k2li5b77.fsf@blackfin.pond.sub.org>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <56DD7E53.8070309@redhat.com>
Date: Mon, 7 Mar 2016 14:12:51 +0100
MIME-Version: 1.0
In-Reply-To: <87k2li5b77.fsf@blackfin.pond.sub.org>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 01/38] exec: Fix memory allocation when
 memory path names new file
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>
Cc: claudio.fontana@huawei.com, cam@cs.ualberta.ca, mlureau@redhat.com, qemu-devel@nongnu.org, david.marchand@6wind.com



On 04/03/2016 19:50, Markus Armbruster wrote:
> There's another one in ivshmem_server.c, functionally identical and
> wrapped in CONFIG_LINUX.

Not quite identical, since it returns -1 for non-hugetlbfs.  It should
return getpagesize().

> Here's exec.c's:
> 
>     #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;
>         }
> 
>         return fs.f_bsize;
>     }
> 
> Before commit bfc2a1a, it additionally had
> 
>     if (fs.f_type != HUGETLBFS_MAGIC)
>         fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path);
> 
> Note the lack of "if not hugetlbfs, use getpagesize()" logic.
> 
> Here's util/mmap-alloc.c's:
> 
>     #define HUGETLBFS_MAGIC       0x958458f6
> 
>     #ifdef CONFIG_LINUX
>     #include <sys/vfs.h>
>     #endif
> 
>     size_t qemu_fd_getpagesize(int fd)
>     {
>     #ifdef CONFIG_LINUX
>         struct statfs fs;
>         int ret;
> 
>         if (fd != -1) {
>             do {
>                 ret = fstatfs(fd, &fs);
>             } while (ret != 0 && errno == EINTR);
> 
>             if (ret == 0 && fs.f_type == HUGETLBFS_MAGIC) {
>                 return fs.f_bsize;
>             }
>         }
>     #endif
> 
>         return getpagesize();
>     }
> 
> Would you like me to convert the others users to this one and drop the
> dupes?

That would be great, since all of them really should use fstatfs instead
of statfs.

Paolo