Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S967986AbdD0Hbt (ORCPT ); Thu, 27 Apr 2017 03:31:49 -0400 Received: from mail.fireflyinternet.com ([109.228.58.192]:57079 "EHLO fireflyinternet.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S964792AbdD0Hbk (ORCPT ); Thu, 27 Apr 2017 03:31:40 -0400 X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Date: Thu, 27 Apr 2017 08:31:19 +0100 From: Chris Wilson To: Laura Abbott Cc: Daniel Vetter , Sean Paul , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Sumit Semwal , Joonas Lahtinen Subject: Re: [RFC PATCHv2 3/3] drm/vgem: Enable dmabuf import interfaces Message-ID: <20170427073119.GW11432@nuc-i3427.alporthouse.com> Mail-Followup-To: Chris Wilson , Laura Abbott , Daniel Vetter , Sean Paul , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Sumit Semwal , Joonas Lahtinen References: <1493241150-21742-1-git-send-email-labbott@redhat.com> <1493241150-21742-4-git-send-email-labbott@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1493241150-21742-4-git-send-email-labbott@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3299 Lines: 117 On Wed, Apr 26, 2017 at 02:12:30PM -0700, Laura Abbott wrote: > Enable the GEM dma-buf import interfaces in addition to the export > interfaces. This lets vgem be used as a test source for other allocators > (e.g. Ion). > > Signed-off-by: Laura Abbott > --- > v2: Don't require vgem allocated buffers to existing in memory before importing. > --- > drivers/gpu/drm/vgem/vgem_drv.c | 133 +++++++++++++++++++++++++++++++--------- > drivers/gpu/drm/vgem/vgem_drv.h | 2 + > 2 files changed, 106 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c > index 1b02e56..73a619a 100644 > --- a/drivers/gpu/drm/vgem/vgem_drv.c > +++ b/drivers/gpu/drm/vgem/vgem_drv.c > @@ -34,6 +34,9 @@ > #include > #include > #include > + > +#include > + > #include "vgem_drv.h" > > #define DRIVER_NAME "vgem" > @@ -46,6 +49,12 @@ static void vgem_gem_free_object(struct drm_gem_object *obj) > { > struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj); > > + if (vgem_obj->pages) > + drm_free_large(vgem_obj->pages); Just drm_free_large(vgem_obj->pages); NULL -> kfree() which is NULL safe. The series lgtm, doesn't compromise on any of the existing tests. All 3, Reviewed-by: Chris Wilson You could always get Joonas to nitpick over style... > + > + if (obj->import_attach) > + drm_prime_gem_destroy(obj, vgem_obj->table); > + > drm_gem_object_release(obj); > kfree(vgem_obj); > } > @@ -56,26 +65,48 @@ static int vgem_gem_fault(struct vm_fault *vmf) > struct drm_vgem_gem_object *obj = vma->vm_private_data; > /* We don't use vmf->pgoff since that has the fake offset */ > unsigned long vaddr = vmf->address; > - struct page *page; > - > - page = shmem_read_mapping_page(file_inode(obj->base.filp)->i_mapping, > - (vaddr - vma->vm_start) >> PAGE_SHIFT); > - if (!IS_ERR(page)) { > - vmf->page = page; > - return 0; > - } else switch (PTR_ERR(page)) { > - case -ENOSPC: > - case -ENOMEM: > - return VM_FAULT_OOM; > - case -EBUSY: > - return VM_FAULT_RETRY; > - case -EFAULT: > - case -EINVAL: > - return VM_FAULT_SIGBUS; > - default: > - WARN_ON_ONCE(PTR_ERR(page)); > - return VM_FAULT_SIGBUS; > + int ret; > + loff_t num_pages; > + pgoff_t page_offset; > + page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT; > + > + num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE); > + > + if (page_offset > num_pages) > + return VM_FAULT_SIGBUS; > + > + if (obj->pages) { > + get_page(obj->pages[page_offset]); > + vmf->page = obj->pages[page_offset]; > + ret = 0; > + } else { > + struct page *page; > + > + page = shmem_read_mapping_page( > + file_inode(obj->base.filp)->i_mapping, > + page_offset); > + if (!IS_ERR(page)) { > + vmf->page = page; > + ret = 0; > + } else switch (PTR_ERR(page)) { > + case -ENOSPC: > + case -ENOMEM: > + ret = VM_FAULT_OOM; > + break; > + case -EBUSY: > + ret = VM_FAULT_RETRY; > + break; > + case -EFAULT: > + case -EINVAL: > + ret = VM_FAULT_SIGBUS; > + break; > + default: > + WARN_ON(PTR_ERR(page)); > + ret = VM_FAULT_SIGBUS; break; -Chris -- Chris Wilson, Intel Open Source Technology Centre