| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAF6AEGtnQAAM-p=Q83Ha=9_PyzaXF9omhY4O1UOv=4+45xvUXg@mail.gmail.com>
Date: Mon, 17 Feb 2014 11:38:17 -0500
From: Rob Clark <robdclark@...il.com>
To: Maarten Lankhorst <maarten.lankhorst@...onical.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-arch@...r.kernel.org, Colin Cross <ccross@...gle.com>,
"linaro-mm-sig@...ts.linaro.org" <linaro-mm-sig@...ts.linaro.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
Daniel Vetter <daniel@...ll.ch>,
Sumit Semwal <sumit.semwal@...aro.org>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>
Subject: Re: [PATCH 6/6] dma-buf: add poll support, v2
On Mon, Feb 17, 2014 at 10:58 AM, Maarten Lankhorst
<maarten.lankhorst@...onical.com> wrote:
> Thanks to Fengguang Wu for spotting a missing static cast.
>
> v2:
> - Kill unused variable need_shared.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@...onical.com>
> ---
> drivers/base/dma-buf.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/dma-buf.h | 12 ++++++
> 2 files changed, 113 insertions(+)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 85e792c2c909..77ea621ab59d 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -30,6 +30,7 @@
> #include <linux/export.h>
> #include <linux/debugfs.h>
> #include <linux/seq_file.h>
> +#include <linux/poll.h>
> #include <linux/reservation.h>
>
> static inline int is_dma_buf_file(struct file *);
> @@ -52,6 +53,13 @@ static int dma_buf_release(struct inode *inode, struct file *file)
>
> BUG_ON(dmabuf->vmapping_counter);
>
> + /*
> + * Any fences that a dma-buf poll can wait on should be signaled
> + * before releasing dma-buf. This is the responsibility of each
> + * driver that uses the reservation objects.
minor nit.. but wouldn't hurt to mention in the comment that if you
hit this BUG_ON it is because someone isn't holding a ref to the
dmabuf while there is a pending fence
> + */
> + BUG_ON(dmabuf->cb_shared.active || dmabuf->cb_excl.active);
> +
> dmabuf->ops->release(dmabuf);
>
> mutex_lock(&db_list.lock);
> @@ -108,10 +116,99 @@ static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence)
> return base + offset;
> }
>
> +static void dma_buf_poll_cb(struct fence *fence, struct fence_cb *cb)
> +{
> + struct dma_buf_poll_cb_t *dcb = (struct dma_buf_poll_cb_t*) cb;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dcb->poll->lock, flags);
> + wake_up_locked_poll(dcb->poll, dcb->active);
> + dcb->active = 0;
> + spin_unlock_irqrestore(&dcb->poll->lock, flags);
> +}
> +
> +static unsigned int dma_buf_poll(struct file *file, poll_table *poll)
> +{
> + struct dma_buf *dmabuf;
> + struct reservation_object *resv;
> + unsigned long events;
> +
> + dmabuf = file->private_data;
> + if (!dmabuf || !dmabuf->resv)
> + return POLLERR;
> +
> + resv = dmabuf->resv;
> +
> + poll_wait(file, &dmabuf->poll, poll);
> +
> + events = poll_requested_events(poll) & (POLLIN | POLLOUT);
> + if (!events)
> + return 0;
> +
> + ww_mutex_lock(&resv->lock, NULL);
> +
> + if (resv->fence_excl && (!(events & POLLOUT) || resv->fence_shared_count == 0)) {
> + struct dma_buf_poll_cb_t *dcb = &dmabuf->cb_excl;
> + unsigned long pevents = POLLIN;
> +
> + if (resv->fence_shared_count == 0)
> + pevents |= POLLOUT;
> +
> + spin_lock_irq(&dmabuf->poll.lock);
> + if (dcb->active) {
> + dcb->active |= pevents;
> + events &= ~pevents;
> + } else
> + dcb->active = pevents;
> + spin_unlock_irq(&dmabuf->poll.lock);
> +
> + if (events & pevents) {
> + if (!fence_add_callback(resv->fence_excl,
> + &dcb->cb, dma_buf_poll_cb))
> + events &= ~pevents;
> + else
> + // No callback queued, wake up any additional waiters.
couple spots w/ c++ comments, which I assume you didn't mean to leave?
Anyways, other than those minor things,
Reviewed-by: Rob Clark <robdclark@...il.com>
> + dma_buf_poll_cb(NULL, &dcb->cb);
> + }
> + }
> +
> + if ((events & POLLOUT) && resv->fence_shared_count > 0) {
> + struct dma_buf_poll_cb_t *dcb = &dmabuf->cb_shared;
> + int i;
> +
> + /* Only queue a new callback if no event has fired yet */
> + spin_lock_irq(&dmabuf->poll.lock);
> + if (dcb->active)
> + events &= ~POLLOUT;
> + else
> + dcb->active = POLLOUT;
> + spin_unlock_irq(&dmabuf->poll.lock);
> +
> + if (!(events & POLLOUT))
> + goto out;
> +
> + for (i = 0; i < resv->fence_shared_count; ++i)
> + if (!fence_add_callback(resv->fence_shared[i],
> + &dcb->cb, dma_buf_poll_cb)) {
> + events &= ~POLLOUT;
> + break;
> + }
> +
> + // No callback queued, wake up any additional waiters.
> + if (i == resv->fence_shared_count)
> + dma_buf_poll_cb(NULL, &dcb->cb);
> + }
> +
> +out:
> + ww_mutex_unlock(&resv->lock);
> + return events;
> +}
> +
> static const struct file_operations dma_buf_fops = {
> .release = dma_buf_release,
> .mmap = dma_buf_mmap_internal,
> .llseek = dma_buf_llseek,
> + .poll = dma_buf_poll,
> };
>
> /*
> @@ -171,6 +268,10 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
> dmabuf->ops = ops;
> dmabuf->size = size;
> dmabuf->exp_name = exp_name;
> + init_waitqueue_head(&dmabuf->poll);
> + dmabuf->cb_excl.poll = dmabuf->cb_shared.poll = &dmabuf->poll;
> + dmabuf->cb_excl.active = dmabuf->cb_shared.active = 0;
> +
> if (!resv) {
> resv = (struct reservation_object*)&dmabuf[1];
> reservation_object_init(resv);
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 34cfbac52c03..e1df18f584ef 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -30,6 +30,8 @@
> #include <linux/list.h>
> #include <linux/dma-mapping.h>
> #include <linux/fs.h>
> +#include <linux/fence.h>
> +#include <linux/wait.h>
>
> struct device;
> struct dma_buf;
> @@ -130,6 +132,16 @@ struct dma_buf {
> struct list_head list_node;
> void *priv;
> struct reservation_object *resv;
> +
> + /* poll support */
> + wait_queue_head_t poll;
> +
> + struct dma_buf_poll_cb_t {
> + struct fence_cb cb;
> + wait_queue_head_t *poll;
> +
> + unsigned long active;
> + } cb_excl, cb_shared;
> };
>
> /**
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists