From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54353) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJzOf-00055f-6K for qemu-devel@nongnu.org; Tue, 28 Jul 2015 03:33:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZJzOb-0002Lf-VV for qemu-devel@nongnu.org; Tue, 28 Jul 2015 03:33:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32914) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJzOb-0002LX-O2 for qemu-devel@nongnu.org; Tue, 28 Jul 2015 03:33:53 -0400 Date: Tue, 28 Jul 2015 09:33:47 +0200 From: Andrew Jones Message-ID: <20150728073347.GA3629@hawk.localdomain> References: <1438043577-28636-1-git-send-email-marcandre.lureau@redhat.com> <1438043577-28636-34-git-send-email-marcandre.lureau@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1438043577-28636-34-git-send-email-marcandre.lureau@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 33/45] ivshmem-server: fix hugetlbfs support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Marc-Andr=E9?= Lureau Cc: cam@cs.ualberta.ca, =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , qemu-devel@nongnu.org, stefanha@redhat.com On Tue, Jul 28, 2015 at 02:32:45AM +0200, Marc-Andr=E9 Lureau wrote: > From: Marc-Andr=E9 Lureau >=20 > As pointed out on the ML by Andrew Jones, glibc no longer permits > creating POSIX shm on hugetlbfs directly. When given a hugetlbfs path, > create a shareable file there. >=20 > Signed-off-by: Marc-Andr=E9 Lureau > --- > contrib/ivshmem-server/ivshmem-server.c | 47 +++++++++++++++++++++++++= +++++++- > contrib/ivshmem-server/ivshmem-server.h | 3 +-- > contrib/ivshmem-server/main.c | 5 ++-- > 3 files changed, 49 insertions(+), 6 deletions(-) >=20 > diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-= server/ivshmem-server.c > index 972fda2..4bf774b 100644 > --- a/contrib/ivshmem-server/ivshmem-server.c > +++ b/contrib/ivshmem-server/ivshmem-server.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > =20 > #include "qemu-common.h" > #include "qemu/queue.h" > @@ -271,15 +272,59 @@ ivshmem_server_init(IvshmemServer *server, const = char *unix_sock_path, > return 0; > } > =20 > +#define HUGETLBFS_MAGIC 0x958458f6 > + > +static long gethugepagesize(const char *path) > +{ > + struct statfs fs; > + int ret; > + > + do { > + ret =3D statfs(path, &fs); > + } while (ret !=3D 0 && errno =3D=3D EINTR); > + > + if (ret !=3D 0) { > + if (errno !=3D ENOENT) { > + fprintf(stderr, "cannot stat shm file %s: %s\n", path, > + strerror(errno)); > + } > + return -1; > + } > + > + if (fs.f_type !=3D HUGETLBFS_MAGIC) { > + return -1; > + } > + > + return fs.f_bsize; > +} > + > + > + few extra lines here > /* open shm, create and bind to the unix socket */ > int > ivshmem_server_start(IvshmemServer *server) > { > struct sockaddr_un sun; > int shm_fd, sock_fd, ret; > + long hpagesize; > + gchar *filename; > =20 > /* open shm file */ > - shm_fd =3D shm_open(server->shm_path, O_CREAT|O_RDWR, S_IRWXU); > + hpagesize =3D gethugepagesize(server->shm_path); > + if (hpagesize > 0) { > + if (server->shm_size < hpagesize) { should be >, but isn't this forcing the shared memory to be less than equal to the size of a single hugepage? I think we should allow up to nr-pages * page-size. Also, I'm not sure we want the dependency, but what about libhugetlbfs? It has hugetlbfs_test_path > + fprintf(stderr, "hugepage must be at least of size: %ld\n"= , > + hpagesize); > + return -1; > + } > + filename =3D g_strdup_printf("%s/ivshmem.XXXXXX", server->shm_= path); > + shm_fd =3D mkstemp(filename); Shouldn't we change the perms for shm_fd to match the non-hugetlbfs case? Or change the non-hugetlbfs case to match mkstemp? Actually, probably the later, because I don't think we want the region to have execute perms, or do we? Also, I guess the plan is to pass the hugetlbfs file descriptor around if other host processes need to know where the memory is, as we never allow a full path. Should we do the same for shm? i.e. keep them anonymous too and always pass file descriptors? > + unlink(filename); > + g_free(filename); > + } else { > + shm_fd =3D shm_open(server->shm_path, O_CREAT|O_RDWR, S_IRWXU)= ; > + } > + > if (shm_fd < 0) { > fprintf(stderr, "cannot open shm file %s: %s\n", server->shm_p= ath, > strerror(errno)); > diff --git a/contrib/ivshmem-server/ivshmem-server.h b/contrib/ivshmem-= server/ivshmem-server.h > index 2176d5e..e9b0e7a 100644 > --- a/contrib/ivshmem-server/ivshmem-server.h > +++ b/contrib/ivshmem-server/ivshmem-server.h > @@ -81,8 +81,7 @@ typedef struct IvshmemServer { > * @server: A pointer to an uninitialized IvshmemServer struct= ure > * @unix_sock_path: The pointer to the unix socket file name > * @shm_path: Path to the shared memory. The path corresponds to= a POSIX > - * shm name. To use a real file, for instance in a hu= getlbfs, > - * it is possible to use /../../abspath/to/file. > + * shm name or a hugetlbfs mount point. > * @shm_size: Size of shared memory > * @n_vectors: Number of interrupt vectors per client > * @verbose: True to enable verbose mode > diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/mai= n.c > index 84ffc4d..cd8d9ed 100644 > --- a/contrib/ivshmem-server/main.c > +++ b/contrib/ivshmem-server/main.c > @@ -47,9 +47,8 @@ ivshmem_server_usage(const char *name, int code) > " to listen to.\n" > " Default=3D%s\n", IVSHMEM_SERVER_DEFAULT_UNIX= _SOCK_PATH); > fprintf(stderr, " -m : path to the shared memory.\n" > - " The path corresponds to a POSIX shm name. To= use a\n" > - " real file, for instance in a hugetlbfs, use\= n" > - " /../../abspath/to/file.\n" > + " The path corresponds to a POSIX shm name or = a\n" > + " hugetlbfs mount point.\n" > " default=3D%s\n", IVSHMEM_SERVER_DEFAULT_SHM_= PATH); > fprintf(stderr, " -l : size of shared memory in bytes. The = suffix\n" > " K, M and G can be used (ex: 1K means 1024).\= n" > --=20 > 2.4.3 >=20 >=20 Thanks, drew