contigmem: do not zero pages during each mmap
authorTiwei Bie <tiwei.bie@intel.com>
Sun, 4 Jun 2017 05:53:24 +0000 (13:53 +0800)
committerThomas Monjalon <thomas@monjalon.net>
Mon, 3 Jul 2017 23:32:57 +0000 (01:32 +0200)
Don't zero the pages during each mmap. Instead, only zero the pages
when they are not already mmapped. Otherwise, the multi-process
support will be broken, as the pages will be zeroed when secondary
processes map the memory. Besides, track the open and mmap operations
on the cdev, and prevent the module from being unloaded when it is
still in use.

Fixes: 82f931805506 ("contigmem: zero all pages during mmap")
Cc: stable@dpdk.org
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
lib/librte_eal/bsdapp/contigmem/contigmem.c

index 03e3e8d..e8fb908 100644 (file)
@@ -50,24 +50,37 @@ __FBSDID("$FreeBSD$");
 
 #include <vm/vm.h>
 #include <vm/pmap.h>
+#include <vm/vm_param.h>
 #include <vm/vm_object.h>
 #include <vm/vm_page.h>
 #include <vm/vm_pager.h>
+#include <vm/vm_phys.h>
+
+struct contigmem_buffer {
+       void           *addr;
+       int             refcnt;
+       struct mtx      mtx;
+};
+
+struct contigmem_vm_handle {
+       int             buffer_index;
+};
 
 static int              contigmem_load(void);
 static int              contigmem_unload(void);
 static int              contigmem_physaddr(SYSCTL_HANDLER_ARGS);
 
-static d_mmap_t         contigmem_mmap;
 static d_mmap_single_t  contigmem_mmap_single;
 static d_open_t         contigmem_open;
+static d_close_t        contigmem_close;
 
 static int              contigmem_num_buffers = RTE_CONTIGMEM_DEFAULT_NUM_BUFS;
 static int64_t          contigmem_buffer_size = RTE_CONTIGMEM_DEFAULT_BUF_SIZE;
 
 static eventhandler_tag contigmem_eh_tag;
-static void            *contigmem_buffers[RTE_CONTIGMEM_MAX_NUM_BUFS];
+static struct contigmem_buffer contigmem_buffers[RTE_CONTIGMEM_MAX_NUM_BUFS];
 static struct cdev     *contigmem_cdev = NULL;
+static int              contigmem_refcnt;
 
 TUNABLE_INT("hw.contigmem.num_buffers", &contigmem_num_buffers);
 TUNABLE_QUAD("hw.contigmem.buffer_size", &contigmem_buffer_size);
@@ -78,6 +91,8 @@ SYSCTL_INT(_hw_contigmem, OID_AUTO, num_buffers, CTLFLAG_RD,
        &contigmem_num_buffers, 0, "Number of contigmem buffers allocated");
 SYSCTL_QUAD(_hw_contigmem, OID_AUTO, buffer_size, CTLFLAG_RD,
        &contigmem_buffer_size, 0, "Size of each contiguous buffer");
+SYSCTL_INT(_hw_contigmem, OID_AUTO, num_references, CTLFLAG_RD,
+       &contigmem_refcnt, 0, "Number of references to contigmem");
 
 static SYSCTL_NODE(_hw_contigmem, OID_AUTO, physaddr, CTLFLAG_RD, 0,
        "physaddr");
@@ -114,9 +129,10 @@ MODULE_VERSION(contigmem, 1);
 static struct cdevsw contigmem_ops = {
        .d_name         = "contigmem",
        .d_version      = D_VERSION,
-       .d_mmap         = contigmem_mmap,
+       .d_flags        = D_TRACKCLOSE,
        .d_mmap_single  = contigmem_mmap_single,
        .d_open         = contigmem_open,
+       .d_close        = contigmem_close,
 };
 
 static int
@@ -124,6 +140,7 @@ contigmem_load()
 {
        char index_string[8], description[32];
        int  i, error = 0;
+       void *addr;
 
        if (contigmem_num_buffers > RTE_CONTIGMEM_MAX_NUM_BUFS) {
                printf("%d buffers requested is greater than %d allowed\n",
@@ -141,18 +158,20 @@ contigmem_load()
        }
 
        for (i = 0; i < contigmem_num_buffers; i++) {
-               contigmem_buffers[i] =
-                               contigmalloc(contigmem_buffer_size, M_CONTIGMEM, M_ZERO, 0,
-                       BUS_SPACE_MAXADDR, contigmem_buffer_size, 0);
-
-               if (contigmem_buffers[i] == NULL) {
+               addr = contigmalloc(contigmem_buffer_size, M_CONTIGMEM, M_ZERO,
+                       0, BUS_SPACE_MAXADDR, contigmem_buffer_size, 0);
+               if (addr == NULL) {
                        printf("contigmalloc failed for buffer %d\n", i);
                        error = ENOMEM;
                        goto error;
                }
 
-               printf("%2u: virt=%p phys=%p\n", i, contigmem_buffers[i],
-                               (void *)pmap_kextract((vm_offset_t)contigmem_buffers[i]));
+               printf("%2u: virt=%p phys=%p\n", i, addr,
+                       (void *)pmap_kextract((vm_offset_t)addr));
+
+               mtx_init(&contigmem_buffers[i].mtx, "contigmem", NULL, MTX_DEF);
+               contigmem_buffers[i].addr = addr;
+               contigmem_buffers[i].refcnt = 0;
 
                snprintf(index_string, sizeof(index_string), "%d", i);
                snprintf(description, sizeof(description),
@@ -170,10 +189,13 @@ contigmem_load()
        return 0;
 
 error:
-       for (i = 0; i < contigmem_num_buffers; i++)
-               if (contigmem_buffers[i] != NULL)
-                       contigfree(contigmem_buffers[i], contigmem_buffer_size,
-                                       M_CONTIGMEM);
+       for (i = 0; i < contigmem_num_buffers; i++) {
+               if (contigmem_buffers[i].addr != NULL)
+                       contigfree(contigmem_buffers[i].addr,
+                               contigmem_buffer_size, M_CONTIGMEM);
+               if (mtx_initialized(&contigmem_buffers[i].mtx))
+                       mtx_destroy(&contigmem_buffers[i].mtx);
+       }
 
        return error;
 }
@@ -183,16 +205,22 @@ contigmem_unload()
 {
        int i;
 
+       if (contigmem_refcnt > 0)
+               return EBUSY;
+
        if (contigmem_cdev != NULL)
                destroy_dev(contigmem_cdev);
 
        if (contigmem_eh_tag != NULL)
                EVENTHANDLER_DEREGISTER(process_exit, contigmem_eh_tag);
 
-       for (i = 0; i < RTE_CONTIGMEM_MAX_NUM_BUFS; i++)
-               if (contigmem_buffers[i] != NULL)
-                       contigfree(contigmem_buffers[i], contigmem_buffer_size,
-                                       M_CONTIGMEM);
+       for (i = 0; i < RTE_CONTIGMEM_MAX_NUM_BUFS; i++) {
+               if (contigmem_buffers[i].addr != NULL)
+                       contigfree(contigmem_buffers[i].addr,
+                               contigmem_buffer_size, M_CONTIGMEM);
+               if (mtx_initialized(&contigmem_buffers[i].mtx))
+                       mtx_destroy(&contigmem_buffers[i].mtx);
+       }
 
        return 0;
 }
@@ -203,7 +231,7 @@ contigmem_physaddr(SYSCTL_HANDLER_ARGS)
        uint64_t        physaddr;
        int             index = (int)(uintptr_t)arg1;
 
-       physaddr = (uint64_t)vtophys(contigmem_buffers[index]);
+       physaddr = (uint64_t)vtophys(contigmem_buffers[index].addr);
        return sysctl_handle_64(oidp, &physaddr, 0, req);
 }
 
@@ -211,22 +239,121 @@ static int
 contigmem_open(struct cdev *cdev, int fflags, int devtype,
                struct thread *td)
 {
+
+       atomic_add_int(&contigmem_refcnt, 1);
+
+       return 0;
+}
+
+static int
+contigmem_close(struct cdev *cdev, int fflags, int devtype,
+               struct thread *td)
+{
+
+       atomic_subtract_int(&contigmem_refcnt, 1);
+
        return 0;
 }
 
 static int
-contigmem_mmap(struct cdev *cdev, vm_ooffset_t offset, vm_paddr_t *paddr,
-               int prot, vm_memattr_t *memattr)
+contigmem_cdev_pager_ctor(void *handle, vm_ooffset_t size, vm_prot_t prot,
+               vm_ooffset_t foff, struct ucred *cred, u_short *color)
 {
+       struct contigmem_vm_handle *vmh = handle;
+       struct contigmem_buffer *buf;
+
+       buf = &contigmem_buffers[vmh->buffer_index];
+
+       atomic_add_int(&contigmem_refcnt, 1);
+
+       mtx_lock(&buf->mtx);
+       if (buf->refcnt == 0)
+               memset(buf->addr, 0, contigmem_buffer_size);
+       buf->refcnt++;
+       mtx_unlock(&buf->mtx);
 
-       *paddr = offset;
        return 0;
 }
 
+static void
+contigmem_cdev_pager_dtor(void *handle)
+{
+       struct contigmem_vm_handle *vmh = handle;
+       struct contigmem_buffer *buf;
+
+       buf = &contigmem_buffers[vmh->buffer_index];
+
+       mtx_lock(&buf->mtx);
+       buf->refcnt--;
+       mtx_unlock(&buf->mtx);
+
+       free(vmh, M_CONTIGMEM);
+
+       atomic_subtract_int(&contigmem_refcnt, 1);
+}
+
+static int
+contigmem_cdev_pager_fault(vm_object_t object, vm_ooffset_t offset, int prot,
+               vm_page_t *mres)
+{
+       vm_paddr_t paddr;
+       vm_page_t m_paddr, page;
+       vm_memattr_t memattr, memattr1;
+
+       memattr = object->memattr;
+
+       VM_OBJECT_WUNLOCK(object);
+
+       paddr = offset;
+
+       m_paddr = vm_phys_paddr_to_vm_page(paddr);
+       if (m_paddr != NULL) {
+               memattr1 = pmap_page_get_memattr(m_paddr);
+               if (memattr1 != memattr)
+                       memattr = memattr1;
+       }
+
+       if (((*mres)->flags & PG_FICTITIOUS) != 0) {
+               /*
+                * If the passed in result page is a fake page, update it with
+                * the new physical address.
+                */
+               page = *mres;
+               VM_OBJECT_WLOCK(object);
+               vm_page_updatefake(page, paddr, memattr);
+       } else {
+               vm_page_t mret;
+               /*
+                * Replace the passed in reqpage page with our own fake page and
+                * free up the original page.
+                */
+               page = vm_page_getfake(paddr, memattr);
+               VM_OBJECT_WLOCK(object);
+               mret = vm_page_replace(page, object, (*mres)->pindex);
+               KASSERT(mret == *mres,
+                   ("invalid page replacement, old=%p, ret=%p", *mres, mret));
+               vm_page_lock(mret);
+               vm_page_free(mret);
+               vm_page_unlock(mret);
+               *mres = page;
+       }
+
+       page->valid = VM_PAGE_BITS_ALL;
+
+       return VM_PAGER_OK;
+}
+
+static struct cdev_pager_ops contigmem_cdev_pager_ops = {
+       .cdev_pg_ctor = contigmem_cdev_pager_ctor,
+       .cdev_pg_dtor = contigmem_cdev_pager_dtor,
+       .cdev_pg_fault = contigmem_cdev_pager_fault,
+};
+
 static int
 contigmem_mmap_single(struct cdev *cdev, vm_ooffset_t *offset, vm_size_t size,
                struct vm_object **obj, int nprot)
 {
+       struct contigmem_vm_handle *vmh;
        uint64_t buffer_index;
 
        /*
@@ -238,10 +365,17 @@ contigmem_mmap_single(struct cdev *cdev, vm_ooffset_t *offset, vm_size_t size,
        if (buffer_index >= contigmem_num_buffers)
                return EINVAL;
 
-       memset(contigmem_buffers[buffer_index], 0, contigmem_buffer_size);
-       *offset = (vm_ooffset_t)vtophys(contigmem_buffers[buffer_index]);
-       *obj = vm_pager_allocate(OBJT_DEVICE, cdev, size, nprot, *offset,
-                       curthread->td_ucred);
+       if (size > contigmem_buffer_size)
+               return EINVAL;
+
+       vmh = malloc(sizeof(*vmh), M_CONTIGMEM, M_NOWAIT | M_ZERO);
+       if (vmh == NULL)
+               return ENOMEM;
+       vmh->buffer_index = buffer_index;
+
+       *offset = (vm_ooffset_t)vtophys(contigmem_buffers[buffer_index].addr);
+       *obj = cdev_pager_allocate(vmh, OBJT_DEVICE, &contigmem_cdev_pager_ops,
+                       size, nprot, *offset, curthread->td_ucred);
 
        return 0;
 }