From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sedat Dilek Subject: Re: [PATCH v2] drm/gem: fix mmap vma size calculations Date: Tue, 30 Jul 2013 09:52:21 +0200 Message-ID: References: <1374830685-21602-1-git-send-email-dh.herrmann@gmail.com> <1374833372-21926-1-git-send-email-dh.herrmann@gmail.com> <20130726201527.GP6084@phenom.ffwll.local> Reply-To: sedat.dilek@gmail.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f43.google.com (mail-wg0-f43.google.com [74.125.82.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 13B59E6D00 for ; Tue, 30 Jul 2013 00:52:21 -0700 (PDT) Received: by mail-wg0-f43.google.com with SMTP id z12so5615172wgg.34 for ; Tue, 30 Jul 2013 00:52:21 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Daniel Vetter , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Tue, Jul 30, 2013 at 9:41 AM, Sedat Dilek wrote: > On Fri, Jul 26, 2013 at 10:15 PM, Daniel Vetter wrote: >> On Fri, Jul 26, 2013 at 12:09:32PM +0200, David Herrmann wrote: >>> The VMA manager is page-size based so drm_vma_node_size() returns the size >>> in pages. However, drm_gem_mmap_obj() requires the size in bytes. Apply >>> PAGE_SHIFT so we no longer get EINVAL during mmaps due to too small >>> buffers. >>> >>> This bug was introduced in commit: >>> 0de23977cfeb5b357ec884ba15417ae118ff9e9b >>> "drm/gem: convert to new unified vma manager" >>> >>> Fixes i915 gtt mmap failure reported by Sedat Dilek in: >>> Re: linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ] >>> >>> Cc: Daniel Vetter >>> Cc: Chris Wilson >>> Signed-off-by: David Herrmann >>> Reported-by: Sedat Dilek >>> Tested-by: Sedat Dilek >> >> I remember that I even checked whether v4 was consistent with pages vs. >> bytes ;-) So >> > > Hi David, Daniel, and Dave, > > Just reading quickly "drm: add unified vma offset manager"... is that > below in the docs? > > "The VMA manager is page-size based so drm_vma_node_size() returns the size > in pages. However, drm_gem_mmap_obj() requires the size in bytes. Apply > PAGE_SHIFT so we no longer get EINVAL during mmaps due to too small > buffers." > > If not, do you mind adding it? > To quote this two: [ include/drm/drm_vma_manager.h ] /** * drm_vma_node_size() - Return size (page-based) * @node: Node to inspect * * Return the size as number of pages for the given node. This is the same size * that was passed to drm_vma_offset_add(). If no offset is allocated for the * node, this is 0. * * RETURNS: * Size of @node as number of pages. 0 if the node does not have an offset * allocated. */ [ drivers/gpu/drm/drm_gem.c ] /** * drm_gem_mmap - memory map routine for GEM objects * @filp: DRM file pointer * @vma: VMA for the area to be mapped * * If a driver supports GEM object mapping, mmap calls on the DRM file * descriptor will end up here. * * Look up the GEM object based on the offset passed in (vma->vm_pgoff will * contain the fake offset we created when the GTT map ioctl was called on * the object) and map it with a call to drm_gem_mmap_obj(). */ - Sedat - > Thanks in advance! > > - Sedat - > >> Reviewed-by: Daniel Vetter >> >> on this little fixup. >> -Daniel >> >>> --- >>> drivers/gpu/drm/drm_gem.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >>> index 3613b50..1f76572 100644 >>> --- a/drivers/gpu/drm/drm_gem.c >>> +++ b/drivers/gpu/drm/drm_gem.c >>> @@ -666,7 +666,7 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) >>> } >>> >>> obj = container_of(node, struct drm_gem_object, vma_node); >>> - ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node), vma); >>> + ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT, vma); >>> >>> mutex_unlock(&dev->struct_mutex); >>> >>> -- >>> 1.8.3.4 >>> >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch