linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: David Hildenbrand <david@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>,
	dri-devel@lists.freedesktop.org, linux-mm@kvack.org,
	Gerd Hoffmann <kraxel@redhat.com>,
	Dongwon Kim <dongwon.kim@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	James Houghton <jthoughton@google.com>,
	Jerome Marchand <jmarchan@redhat.com>,
	Junxiao Chang <junxiao.chang@intel.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Michal Hocko <mhocko@suse.com>,
	Muchun Song <muchun.song@linux.dev>,
	Jason Gunthorpe <jgg@nvidia.com>,
	John Hubbard <jhubbard@nvidia.com>
Subject: Re: [PATCH v1 0/2] udmabuf: Add back support for mapping hugetlb pages
Date: Thu, 22 Jun 2023 14:33:44 -0700	[thread overview]
Message-ID: <20230622213344.GB4985@monkey> (raw)
In-Reply-To: <6e429fbc-e0e6-53c0-c545-2e2cbbe757de@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On 06/22/23 10:25, David Hildenbrand wrote:
> On 22.06.23 09:27, Vivek Kasireddy wrote:
> 
> There are *probably* more issues on the QEMU side when udmabuf is paired
> with things like MADV_DONTNEED/FALLOC_FL_PUNCH_HOLE used for virtio-balloon,
> virtio-mem, postcopy live migration, ... for example, in the vfio/vdpa case
> we make sure that we disallow most of these, because otherwise there can be
> an accidental "disconnect" between the pages mapped into the VM (guest view)
> and the pages mapped into the IOMMU (device view), for example, after a
> reboot.
> 

Yes, this "disconnect" is still possible.  Attached is a test program I
hacked up based on the udmabuf selftest.  You can see different content
in the memfd pages and udma pages.

FYI- I can verify this new udmabuf code is not accessing struct pages of
hugetlb tail pages, as this test program BUG'ed if hugetlb vmemmap
optimization was enabled in the old udmabuf.
-- 
Mike Kravetz

[-- Attachment #2: udmabuf.c --]
[-- Type: text/plain, Size: 3726 bytes --]

// SPDX-License-Identifier: GPL-2.0
#define _GNU_SOURCE
#define __EXPORTED_HEADERS__

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <fcntl.h>
#include <malloc.h>
#include <sys/mman.h>

#include <sys/ioctl.h>
#include <sys/syscall.h>
#include <linux/memfd.h>
#include <linux/udmabuf.h>

#define TEST_PREFIX	"drivers/dma-buf/udmabuf"
#define NUM_PAGES       2

static int my_getpagesize(void)
{
	/* huge page size */
	return getpagesize() * 512;
}

#if 0
static int memfd_create(const char *name, unsigned int flags)
{
	return syscall(__NR_memfd_create, name, flags);
}
#endif

int main(int argc, char *argv[])
{
	struct udmabuf_create create;
	int devfd, memfd, buf, ret;
	off_t size;
	void *mem;
	int i;
	char foo;
	int mem_fd, udma_fd;
	void *addr1, *addr2;

	devfd = open("/dev/udmabuf", O_RDWR);
	if (devfd < 0) {
		printf("%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n",
		       TEST_PREFIX);
		exit(77);
	}

	mem_fd = memfd_create("udmabuf-test", MFD_HUGETLB | MFD_ALLOW_SEALING);
	if (mem_fd < 0) {
		printf("%s: [skip,no-memfd]\n", TEST_PREFIX);
		exit(77);
	}

	ret = fcntl(mem_fd, F_ADD_SEALS, F_SEAL_SHRINK);
	if (ret < 0) {
		printf("%s: [skip,fcntl-add-seals]\n", TEST_PREFIX);
		exit(77);
	}


	size = my_getpagesize() * NUM_PAGES;
	ret = ftruncate(mem_fd, size);

	if (ret == -1) {
		printf("%s: [FAIL,memfd-truncate]\n", TEST_PREFIX);
		exit(1);
	}

	/* touch all pages */
	addr1 = mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, mem_fd, 0);
	if (addr1 == MAP_FAILED) {
		printf("%s: [FAIL,mmap]\n", TEST_PREFIX);
		exit(1);
	}

	for (i = 0; i < size / getpagesize(); i++) {
		*((char *)addr1 + (i * getpagesize())) = 'a';
	}

	memset(&create, 0, sizeof(create));

#if 0
	/* should fail (offset not page aligned) */
	create.memfd  = mem_fd;
	create.offset = getpagesize()/2;
	create.size   = getpagesize();
	buf = ioctl(devfd, UDMABUF_CREATE, &create);
	if (buf >= 0) {
		printf("%s: [FAIL,test-1]\n", TEST_PREFIX);
		exit(1);
	}

	/* should fail (size not multiple of page) */
	create.memfd  = mem_fd;
	create.offset = 0;
	create.size   = getpagesize()/2;
	buf = ioctl(devfd, UDMABUF_CREATE, &create);
	if (buf >= 0) {
		printf("%s: [FAIL,test-2]\n", TEST_PREFIX);
		exit(1);
	}

	/* should fail (not memfd) */
	create.memfd  = 0; /* stdin */
	create.offset = 0;
	create.size   = size;
	buf = ioctl(devfd, UDMABUF_CREATE, &create);
	if (buf >= 0) {
		printf("%s: [FAIL,test-3]\n", TEST_PREFIX);
		exit(1);
	}
#endif

	/* should work */
	create.memfd  = mem_fd;
	create.offset = getpagesize() * 256;
	create.size   = getpagesize() * 4;
	udma_fd = ioctl(devfd, UDMABUF_CREATE, &create);
	if (udma_fd < 0) {
		perror("UDMABUF_CREATE");
		printf("%s: [FAIL,test-4]\n", TEST_PREFIX);
		exit(1);
	}

	printf("before hole punch\n");
	(void)getchar();
	ret = fallocate(mem_fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
		0, my_getpagesize());
	if (ret)
		perror("fallocate punch hole");
	printf("after hole punch\n");
	(void)getchar();

	for (i = 0; i < size / getpagesize(); i++) {
		*((char *)addr1 + (i * getpagesize())) = 'b';
	}

	printf("after touch again\n");
	(void)getchar();

	/* touch all pages */
	addr2 = mmap(NULL, getpagesize() * 4, PROT_READ|PROT_WRITE, MAP_SHARED, udma_fd, 0);
	if (addr2 == MAP_FAILED) {
		perror("mmap");
		printf("%s: udma_fd mmap fail\n", TEST_PREFIX);
		exit(1);
	}

	for (i = 0; i < 4; i++) {
		foo = *((char *)addr2 + (i * getpagesize()));
		printf("udmabuf %c\n", foo);
	}

	for (i = 256; i < 260; i++) {
		foo = *((char *)addr1 + (i * getpagesize()));
		printf("memfd %c\n", foo);
	}

	fprintf(stderr, "%s: ok\n", TEST_PREFIX);
	close(udma_fd);
	close(mem_fd);
	close(devfd);
	return 0;
}

  reply	other threads:[~2023-06-22 21:34 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-22  7:27 [PATCH v1 0/2] udmabuf: Add back support for mapping hugetlb pages Vivek Kasireddy
2023-06-22  7:27 ` [PATCH v1 1/2] udmabuf: Use vmf_insert_pfn and VM_PFNMAP for handling mmap Vivek Kasireddy
2023-06-22  7:27 ` [PATCH v1 2/2] udmabuf: Add back support for mapping hugetlb pages Vivek Kasireddy
2023-06-22 22:10   ` kernel test robot
2023-06-22  8:25 ` [PATCH v1 0/2] " David Hildenbrand
2023-06-22 21:33   ` Mike Kravetz [this message]
2023-06-23  6:13   ` Kasireddy, Vivek
2023-06-23 16:35     ` Peter Xu
2023-06-23 16:37       ` Jason Gunthorpe
2023-06-23 17:28         ` Peter Xu
2023-06-26 12:57           ` Jason Gunthorpe
2023-06-26  7:45       ` Kasireddy, Vivek
2023-06-26 17:52         ` Peter Xu
2023-06-26 18:14           ` David Hildenbrand
2023-06-26 18:18             ` Jason Gunthorpe
2023-06-26 19:04               ` Peter Xu
2023-06-27 15:52                 ` Jason Gunthorpe
2023-06-27 16:00                   ` Peter Xu
2023-06-27 16:04                     ` Jason Gunthorpe
2023-06-27  6:37             ` Kasireddy, Vivek
2023-06-27  7:10               ` David Hildenbrand
2023-06-28  8:04                 ` Kasireddy, Vivek
2023-08-08 16:17   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230622213344.GB4985@monkey \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=dongwon.kim@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jgg@nvidia.com \
    --cc=jhubbard@nvidia.com \
    --cc=jmarchan@redhat.com \
    --cc=jthoughton@google.com \
    --cc=junxiao.chang@intel.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kraxel@redhat.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=muchun.song@linux.dev \
    --cc=vivek.kasireddy@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).