[Intel-gfx] [PATCH 33/70] drm/i915: Use a separate slab for requests

[Robert Beckett]

On 07/04/2015 16:20, Chris Wilson wrote:
> requests are even more frequently allocated than objects and equally
> benefit from having a dedicated slab.
>
> v2: Rebase
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_dma.c         | 12 ++++++----
>   drivers/gpu/drm/i915/i915_drv.h         |  4 +++-
>   drivers/gpu/drm/i915/i915_gem.c         | 41 +++++++++++++++++++--------------
>   drivers/gpu/drm/i915/intel_ringbuffer.c |  1 -
>   4 files changed, 35 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 7b0109e2ab23..135fbcad367f 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1010,8 +1010,10 @@ out_regs:
>   put_bridge:
>   	pci_dev_put(dev_priv->bridge_dev);
>   free_priv:
> -	if (dev_priv->slab)
> -		kmem_cache_destroy(dev_priv->slab);
> +	if (dev_priv->requests)
> +		kmem_cache_destroy(dev_priv->requests);
> +	if (dev_priv->objects)
> +		kmem_cache_destroy(dev_priv->objects);
>   	kfree(dev_priv);
>   	return ret;
>   }
> @@ -1094,8 +1096,10 @@ int i915_driver_unload(struct drm_device *dev)
>   	if (dev_priv->regs != NULL)
>   		pci_iounmap(dev->pdev, dev_priv->regs);
>
> -	if (dev_priv->slab)
> -		kmem_cache_destroy(dev_priv->slab);
> +	if (dev_priv->requests)
> +		kmem_cache_destroy(dev_priv->requests);
> +	if (dev_priv->objects)
> +		kmem_cache_destroy(dev_priv->objects);
>
>   	pci_dev_put(dev_priv->bridge_dev);
>   	kfree(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 600b6d4a0139..ad08aa532456 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1578,7 +1578,8 @@ struct i915_virtual_gpu {
>
>   struct drm_i915_private {
>   	struct drm_device *dev;
> -	struct kmem_cache *slab;
> +	struct kmem_cache *objects;
> +	struct kmem_cache *requests;
>
>   	const struct intel_device_info info;
>
> @@ -2070,6 +2071,7 @@ struct drm_i915_gem_request {
>   	struct kref ref;
>
>   	/** On Which ring this request was generated */
> +	struct drm_i915_private *i915;
>   	struct intel_engine_cs *ring;
>
>   	/** GEM sequence number associated with this request. */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1f07cd17be04..a4a62592f0f8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -381,13 +381,13 @@ out:
>   void *i915_gem_object_alloc(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	return kmem_cache_zalloc(dev_priv->slab, GFP_KERNEL);
> +	return kmem_cache_zalloc(dev_priv->objects, GFP_KERNEL);
>   }
>
>   void i915_gem_object_free(struct drm_i915_gem_object *obj)
>   {
>   	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> -	kmem_cache_free(dev_priv->slab, obj);
> +	kmem_cache_free(dev_priv->objects, obj);
>   }
>
>   static int
> @@ -2633,43 +2633,45 @@ void i915_gem_request_free(struct kref *req_ref)
>   		i915_gem_context_unreference(ctx);
>   	}
>
> -	kfree(req);
> +	kmem_cache_free(req->i915->requests, req);
>   }
>
>   int i915_gem_request_alloc(struct intel_engine_cs *ring,
>   			   struct intel_context *ctx)
>   {
> +	struct drm_i915_private *dev_priv = to_i915(ring->dev);
> +	struct drm_i915_gem_request *rq;
>   	int ret;
> -	struct drm_i915_gem_request *request;
> -	struct drm_i915_private *dev_private = ring->dev->dev_private;
>
>   	if (ring->outstanding_lazy_request)
>   		return 0;
>
> -	request = kzalloc(sizeof(*request), GFP_KERNEL);
> -	if (request == NULL)
> +	rq = kmem_cache_zalloc(dev_priv->requests, GFP_KERNEL);
> +	if (rq == NULL)
>   		return -ENOMEM;
>
> -	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
> +	kref_init(&rq->ref);
> +	rq->i915 = dev_priv;
> +
> +	ret = i915_gem_get_seqno(ring->dev, &rq->seqno);
>   	if (ret) {
> -		kfree(request);
> +		kfree(rq);
>   		return ret;
>   	}
>
> -	kref_init(&request->ref);
> -	request->ring = ring;
> -	request->uniq = dev_private->request_uniq++;
> +	rq->ring = ring;
> +	rq->uniq = dev_priv->request_uniq++;
>
>   	if (i915.enable_execlists)
> -		ret = intel_logical_ring_alloc_request_extras(request, ctx);
> +		ret = intel_logical_ring_alloc_request_extras(rq, ctx);
>   	else
> -		ret = intel_ring_alloc_request_extras(request);
> +		ret = intel_ring_alloc_request_extras(rq);
>   	if (ret) {
> -		kfree(request);
> +		kfree(rq);
>   		return ret;
>   	}
>
> -	ring->outstanding_lazy_request = request;
> +	ring->outstanding_lazy_request = rq;
>   	return 0;
>   }
>
> @@ -5204,11 +5206,16 @@ i915_gem_load(struct drm_device *dev)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	int i;
>
> -	dev_priv->slab =
> +	dev_priv->objects =
>   		kmem_cache_create("i915_gem_object",
>   				  sizeof(struct drm_i915_gem_object), 0,
>   				  SLAB_HWCACHE_ALIGN,
>   				  NULL);
> +	dev_priv->requests =
> +		kmem_cache_create("i915_gem_request",
> +				  sizeof(struct drm_i915_gem_request), 0,
> +				  SLAB_HWCACHE_ALIGN,
> +				  NULL);
>
>   	INIT_LIST_HEAD(&dev_priv->vm_list);
>   	i915_init_vm(dev_priv, &dev_priv->gtt.base);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 99a1fdff4924..bf7837d30388 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2162,7 +2162,6 @@ int intel_ring_idle(struct intel_engine_cs *ring)
>   int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
>   {
>   	request->ringbuf = request->ring->buffer;
> -
>   	return 0;
>   }
>
>

You missed a request allocation in execlists_context_queue in 
intel_lrc.c when !request. By the look of it that code could be changed 
to use i915_gem_request_alloc, unless there is any reason not to set the 
outstanding_lazy_request to the dummy request.