From: Chia-I Wu <olvaffe@gmail.com>
To: David Riley <davidriley@chromium.org>
Cc: "ML dri-devel" <dri-devel@lists.freedesktop.org>,
"open list:VIRTIO CORE,
NET AND BLOCK DRIVERS"
<virtualization@lists.linux-foundation.org>,
"David Airlie" <airlied@linux.ie>,
"open list" <linux-kernel@vger.kernel.org>,
"Gurchetan Singh" <gurchetansingh@chromium.org>,
"Gerd Hoffmann" <kraxel@redhat.com>,
"Stéphane Marchesin" <marcheu@chromium.org>
Subject: Re: [PATCH] drm/virtio: Use vmalloc for command buffer allocations.
Date: Fri, 30 Aug 2019 09:18:52 -0700 [thread overview]
Message-ID: <CAPaKu7SmqNLNDNNZr7c7gEi+ypAKdXJ3z5oYZFppk+53rsTygA@mail.gmail.com> (raw)
In-Reply-To: <20190829212417.257397-1-davidriley@chromium.org>
On Thu, Aug 29, 2019 at 2:24 PM David Riley <davidriley@chromium.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@chromium.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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-08-30 16:19 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-29 21:24 [PATCH] drm/virtio: Use vmalloc for command buffer allocations David Riley
2019-08-30 6:08 ` Gerd Hoffmann
[not found] ` <CAASgrz2v0DYb_5A3MnaWFM4Csx1DKkZe546v7DG7R+UyLOA8og@mail.gmail.com>
2019-08-30 11:16 ` Gerd Hoffmann
2019-08-30 16:24 ` Chia-I Wu
2019-09-02 5:19 ` Gerd Hoffmann
2019-08-30 17:49 ` David Riley
2019-09-02 5:28 ` Gerd Hoffmann
2019-09-03 20:27 ` David Riley
2019-08-30 16:18 ` Chia-I Wu [this message]
2019-09-05 22:00 ` [PATCH v2] " David Riley
2019-09-06 5:18 ` Gerd Hoffmann
2019-09-09 17:12 ` David Riley
2019-09-10 20:06 ` [PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version David Riley
2019-09-11 5:12 ` Gerd Hoffmann
2019-09-11 17:35 ` David Riley
2019-09-10 20:06 ` [PATCH v3 2/2] drm/virtio: Use vmalloc for command buffer allocations David Riley
2019-09-11 18:14 ` [PATCH v4 0/2] drm/virtio: Use vmalloc for command buffer alllocations David Riley
2019-09-11 18:14 ` [PATCH v4 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version David Riley
2019-09-11 18:14 ` [PATCH v4 2/2] drm/virtio: Use vmalloc for command buffer allocations David Riley
2019-09-12 7:51 ` Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPaKu7SmqNLNDNNZr7c7gEi+ypAKdXJ3z5oYZFppk+53rsTygA@mail.gmail.com \
--to=olvaffe@gmail.com \
--cc=airlied@linux.ie \
--cc=davidriley@chromium.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gurchetansingh@chromium.org \
--cc=kraxel@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=marcheu@chromium.org \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).