Re: [PATCH] drm/radeon/kms: forbid big bo allocation (fdo 31708) v3

From: "Michel Dänzer" <michel@daenzer.net>
To: jglisse@redhat.com
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/radeon/kms: forbid big bo allocation (fdo 31708) v3
Date: Thu, 09 Dec 2010 10:43:37 +0100	[thread overview]
Message-ID: <1291887817.11622.51.camel@thor.local> (raw)
In-Reply-To: <1291412299-5145-1-git-send-email-jglisse@redhat.com>

On Fre, 2010-12-03 at 16:38 -0500, jglisse@redhat.com wrote: 
> From: Jerome Glisse <jglisse@redhat.com>
> 
> Forbid allocating buffer bigger than visible VRAM or GTT, also
> properly set lpfn field.
> 
> v2 - use max macro
>    - silence warning
> v3 - don't explicitly set range limit
>    - use min macro
> 
> Cc: stable <stable@kernel.org>
> 
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> ---
>  drivers/gpu/drm/radeon/radeon_object.c |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 1d06774..a598d00 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -91,7 +91,8 @@ int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
>  {
>  	struct radeon_bo *bo;
>  	enum ttm_bo_type type;
> -	int page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
> +	unsigned long page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
> +	unsigned long max_size = 0;

The initialization of max_size is superfluous.

> @@ -104,6 +105,14 @@ int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
>  	}
>  	*bo_ptr = NULL;
>  
> +	/* maximun bo size is the minimun btw visible vram and gtt size */
> +	max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
> +	if ((page_align << PAGE_SHIFT) >= max_size) {
> +		printk(KERN_WARNING "%s:%d alloc size %ldM bigger than %ldMb limit\n",
> +			__func__, __LINE__, page_align  >> (20 - PAGE_SHIFT), max_size >> 20);
> +		return -ENOMEM;
> +	}

Still not sure this simple minimum is quite the right solution... e.g.
if GTT is larger than visible VRAM, what's the problem with having a BO
that fits in GTT but not in visible VRAM? On systems with 8 MB of VRAM,
this will effectively limit the maximum texture size to 2048x1024.

(That's ignoring the fact that userspace can't use the full size of GTT
or VRAM due to the BOs pinned to visible VRAM and the ring and indirect
buffers at the start of GTT)

P.S. I think it's better not to submit patches to stable@kernel.org
while they're being reviewed. Once a patch lands in mainline, the Cc:
stable@kernel.org tag in the commit message will cause it to be queued
for the stable branch(es).

-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel