[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPaKu7SmqNLNDNNZr7c7gEi+ypAKdXJ3z5oYZFppk+53rsTygA@mail.gmail.com>
Date: Fri, 30 Aug 2019 09:18:52 -0700
From: Chia-I Wu <olvaffe@...il.com>
To: David Riley <davidriley@...omium.org>
Cc: ML dri-devel <dri-devel@...ts.freedesktop.org>,
"open list:VIRTIO CORE, NET AND BLOCK DRIVERS"
<virtualization@...ts.linux-foundation.org>,
David Airlie <airlied@...ux.ie>,
open list <linux-kernel@...r.kernel.org>,
Gurchetan Singh <gurchetansingh@...omium.org>,
Gerd Hoffmann <kraxel@...hat.com>,
Stéphane Marchesin <marcheu@...omium.org>
Subject: Re: [PATCH] drm/virtio: Use vmalloc for command buffer allocations.
On Thu, Aug 29, 2019 at 2:24 PM David Riley <davidriley@...omium.org> wrote:
>
> Userspace requested command buffer allocations could be too large
> to make as a contiguous allocation. Use vmalloc if necessary to
> satisfy those allocations.
>
> Signed-off-by: David Riley <davidriley@...omium.org>
> ---
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 74 ++++++++++++++++++++++++--
> 2 files changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index ac60be9b5c19..a8732a8af766 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -195,7 +195,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> if (ret)
> goto out_free;
>
> - buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
> + buf = vmemdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
> if (IS_ERR(buf)) {
> ret = PTR_ERR(buf);
> goto out_unresv;
> @@ -230,7 +230,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> return 0;
>
> out_memdup:
> - kfree(buf);
> + kvfree(buf);
> out_unresv:
> ttm_eu_backoff_reservation(&ticket, &validate_list);
> out_free:
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 981ee16e3ee9..bcbc48b7284f 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -154,7 +154,7 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
> {
> if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
> kfree(vbuf->resp_buf);
> - kfree(vbuf->data_buf);
> + kvfree(vbuf->data_buf);
> kmem_cache_free(vgdev->vbufs, vbuf);
> }
>
> @@ -251,6 +251,59 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct *work)
> wake_up(&vgdev->cursorq.ack_queue);
> }
>
> +/* How many bytes left in this page. */
> +static unsigned int rest_of_page(void *data)
> +{
> + return PAGE_SIZE - offset_in_page(data);
> +}
> +
> +/* Create sg_table from a vmalloc'd buffer. */
> +static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size)
> +{
> + int nents, ret, s, i;
> + struct sg_table *sgt;
> + struct scatterlist *sg;
> + struct page *pg;
> +
> + sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
> + if (!sgt)
> + return NULL;
> +
> + nents = DIV_ROUND_UP(size, PAGE_SIZE) + 1;
> + ret = sg_alloc_table(sgt, nents, GFP_KERNEL);
> + if (ret) {
> + kfree(sgt);
> + return NULL;
> + }
> +
> + for_each_sg(sgt->sgl, sg, nents, i) {
> + pg = vmalloc_to_page(data);
> + if (!pg) {
> + sg_free_table(sgt);
> + kfree(sgt);
> + return NULL;
> + }
> +
> + s = rest_of_page(data);
> + if (s > size)
> + s = size;
> +
> + sg_set_page(sg, pg, s, offset_in_page(data));
> +
> + size -= s;
> + data += s;
> +
> + if (size) {
> + sg_unmark_end(sg);
> + } else {
> + sg_mark_end(sg);
> + break;
> + }
> + }
> +
> + return sgt;
> +}
> +
> static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_vbuffer *vbuf)
> __releases(&vgdev->ctrlq.qlock)
> @@ -260,6 +313,7 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
> struct scatterlist *sgs[3], vcmd, vout, vresp;
> int outcnt = 0, incnt = 0;
> int ret;
> + struct sg_table *sgt = NULL;
>
> if (!vgdev->vqs_ready)
> return -ENODEV;
> @@ -269,8 +323,17 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
> outcnt++;
>
> if (vbuf->data_size) {
> - sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
> - sgs[outcnt + incnt] = &vout;
> + if (is_vmalloc_addr(vbuf->data_buf)) {
> + spin_unlock(&vgdev->ctrlq.qlock);
> + sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size);
> + spin_lock(&vgdev->ctrlq.qlock);
> + if (!sgt)
> + return -ENOMEM;
> + sgs[outcnt + incnt] = sgt->sgl;
If the construction of sgs is no longer atomic, it should be moved out
of the critical section.
> + } else {
> + sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
> + sgs[outcnt + incnt] = &vout;
> + }
> outcnt++;
> }
>
> @@ -294,6 +357,11 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
> virtqueue_kick(vq);
> }
>
> + if (sgt) {
> + sg_free_table(sgt);
> + kfree(sgt);
> + }
> +
> if (!ret)
> ret = vq->num_free;
> return ret;
> --
> 2.23.0.187.g17f5b7556c-goog
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Powered by blists - more mailing lists