| Message ID | 20220207125933.81634-8-paul@crapouillou.net (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | iio: buffer-dma: write() and new DMABUF based API | expand |
On Mon, 7 Feb 2022 12:59:28 +0000 Paul Cercueil <paul@crapouillou.net> wrote: > Enhance the current fileio code by using DMABUF objects instead of > custom buffers. > > This adds more code than it removes, but: > - a lot of the complexity can be dropped, e.g. custom kref and > iio_buffer_block_put_atomic() are not needed anymore; > - it will be much easier to introduce an API to export these DMABUF > objects to userspace in a following patch. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> Hi Paul, I'm a bit rusty on dma mappings, but you seem to have a mixture of streaming and coherent mappings going on in here. Is it the case that the current code is using the coherent mappings and a potential 'other user' of the dma buffer might need streaming mappings? Jonathan > --- > drivers/iio/buffer/industrialio-buffer-dma.c | 192 ++++++++++++------- > include/linux/iio/buffer-dma.h | 8 +- > 2 files changed, 122 insertions(+), 78 deletions(-) > > diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c b/drivers/iio/buffer/industrialio-buffer-dma.c > index 15ea7bc3ac08..54e6000cd2ee 100644 > --- a/drivers/iio/buffer/industrialio-buffer-dma.c > +++ b/drivers/iio/buffer/industrialio-buffer-dma.c > @@ -14,6 +14,7 @@ > #include <linux/poll.h> > #include <linux/iio/buffer_impl.h> > #include <linux/iio/buffer-dma.h> > +#include <linux/dma-buf.h> > #include <linux/dma-mapping.h> > #include <linux/sizes.h> > > @@ -90,103 +91,145 @@ > * callback is called from within the custom callback. > */ > > -static void iio_buffer_block_release(struct kref *kref) > -{ > - struct iio_dma_buffer_block *block = container_of(kref, > - struct iio_dma_buffer_block, kref); > - > - WARN_ON(block->state != IIO_BLOCK_STATE_DEAD); > - > - dma_free_coherent(block->queue->dev, PAGE_ALIGN(block->size), > - block->vaddr, block->phys_addr); > - > - iio_buffer_put(&block->queue->buffer); > - kfree(block); > -} > - > -static void iio_buffer_block_get(struct iio_dma_buffer_block *block) > -{ > - kref_get(&block->kref); > -} > - > -static void iio_buffer_block_put(struct iio_dma_buffer_block *block) > -{ > - kref_put(&block->kref, iio_buffer_block_release); > -} > - > -/* > - * dma_free_coherent can sleep, hence we need to take some special care to be > - * able to drop a reference from an atomic context. > - */ > -static LIST_HEAD(iio_dma_buffer_dead_blocks); > -static DEFINE_SPINLOCK(iio_dma_buffer_dead_blocks_lock); > - > -static void iio_dma_buffer_cleanup_worker(struct work_struct *work) > -{ > - struct iio_dma_buffer_block *block, *_block; > - LIST_HEAD(block_list); > - > - spin_lock_irq(&iio_dma_buffer_dead_blocks_lock); > - list_splice_tail_init(&iio_dma_buffer_dead_blocks, &block_list); > - spin_unlock_irq(&iio_dma_buffer_dead_blocks_lock); > - > - list_for_each_entry_safe(block, _block, &block_list, head) > - iio_buffer_block_release(&block->kref); > -} > -static DECLARE_WORK(iio_dma_buffer_cleanup_work, iio_dma_buffer_cleanup_worker); > - > -static void iio_buffer_block_release_atomic(struct kref *kref) > -{ > +struct iio_buffer_dma_buf_attachment { > + struct scatterlist sgl; > + struct sg_table sg_table; > struct iio_dma_buffer_block *block; > - unsigned long flags; > - > - block = container_of(kref, struct iio_dma_buffer_block, kref); > - > - spin_lock_irqsave(&iio_dma_buffer_dead_blocks_lock, flags); > - list_add_tail(&block->head, &iio_dma_buffer_dead_blocks); > - spin_unlock_irqrestore(&iio_dma_buffer_dead_blocks_lock, flags); > - > - schedule_work(&iio_dma_buffer_cleanup_work); > -} > - > -/* > - * Version of iio_buffer_block_put() that can be called from atomic context > - */ > -static void iio_buffer_block_put_atomic(struct iio_dma_buffer_block *block) > -{ > - kref_put(&block->kref, iio_buffer_block_release_atomic); > -} > +}; > > static struct iio_dma_buffer_queue *iio_buffer_to_queue(struct iio_buffer *buf) > { > return container_of(buf, struct iio_dma_buffer_queue, buffer); > } > > +static struct iio_buffer_dma_buf_attachment * > +to_iio_buffer_dma_buf_attachment(struct sg_table *table) > +{ > + return container_of(table, struct iio_buffer_dma_buf_attachment, sg_table); > +} > + > +static void iio_buffer_block_get(struct iio_dma_buffer_block *block) > +{ > + get_dma_buf(block->dmabuf); > +} > + > +static void iio_buffer_block_put(struct iio_dma_buffer_block *block) > +{ > + dma_buf_put(block->dmabuf); > +} > + > +static int iio_buffer_dma_buf_attach(struct dma_buf *dbuf, > + struct dma_buf_attachment *at) > +{ > + at->priv = dbuf->priv; > + > + return 0; > +} > + > +static struct sg_table *iio_buffer_dma_buf_map(struct dma_buf_attachment *at, > + enum dma_data_direction dma_dir) > +{ > + struct iio_dma_buffer_block *block = at->priv; > + struct iio_buffer_dma_buf_attachment *dba; > + int ret; > + > + dba = kzalloc(sizeof(*dba), GFP_KERNEL); > + if (!dba) > + return ERR_PTR(-ENOMEM); > + > + sg_init_one(&dba->sgl, block->vaddr, PAGE_ALIGN(block->size)); > + dba->sg_table.sgl = &dba->sgl; > + dba->sg_table.nents = 1; > + dba->block = block; > + > + ret = dma_map_sgtable(at->dev, &dba->sg_table, dma_dir, 0); > + if (ret) { > + kfree(dba); > + return ERR_PTR(ret); > + } > + > + return &dba->sg_table; > +} > + > +static void iio_buffer_dma_buf_unmap(struct dma_buf_attachment *at, > + struct sg_table *sg_table, > + enum dma_data_direction dma_dir) > +{ > + struct iio_buffer_dma_buf_attachment *dba = > + to_iio_buffer_dma_buf_attachment(sg_table); > + > + dma_unmap_sgtable(at->dev, &dba->sg_table, dma_dir, 0); > + kfree(dba); > +} > + > +static void iio_buffer_dma_buf_release(struct dma_buf *dbuf) > +{ > + struct iio_dma_buffer_block *block = dbuf->priv; > + struct iio_dma_buffer_queue *queue = block->queue; > + > + WARN_ON(block->state != IIO_BLOCK_STATE_DEAD); > + > + mutex_lock(&queue->lock); > + > + dma_free_coherent(queue->dev, PAGE_ALIGN(block->size), > + block->vaddr, block->phys_addr); > + kfree(block); > + > + mutex_unlock(&queue->lock); > + iio_buffer_put(&queue->buffer); > +} > + > +static const struct dma_buf_ops iio_dma_buffer_dmabuf_ops = { > + .attach = iio_buffer_dma_buf_attach, > + .map_dma_buf = iio_buffer_dma_buf_map, > + .unmap_dma_buf = iio_buffer_dma_buf_unmap, > + .release = iio_buffer_dma_buf_release, > +}; > + > static struct iio_dma_buffer_block *iio_dma_buffer_alloc_block( > struct iio_dma_buffer_queue *queue, size_t size) > { > struct iio_dma_buffer_block *block; > + DEFINE_DMA_BUF_EXPORT_INFO(einfo); > + struct dma_buf *dmabuf; > + int err = -ENOMEM; > > block = kzalloc(sizeof(*block), GFP_KERNEL); > if (!block) > - return NULL; > + return ERR_PTR(err); > > block->vaddr = dma_alloc_coherent(queue->dev, PAGE_ALIGN(size), > &block->phys_addr, GFP_KERNEL); > - if (!block->vaddr) { > - kfree(block); > - return NULL; > + if (!block->vaddr) > + goto err_free_block; > + > + einfo.ops = &iio_dma_buffer_dmabuf_ops; > + einfo.size = PAGE_ALIGN(size); > + einfo.priv = block; > + einfo.flags = O_RDWR; > + > + dmabuf = dma_buf_export(&einfo); > + if (IS_ERR(dmabuf)) { > + err = PTR_ERR(dmabuf); > + goto err_free_dma; > } > > + block->dmabuf = dmabuf; > block->size = size; > block->state = IIO_BLOCK_STATE_DONE; > block->queue = queue; > INIT_LIST_HEAD(&block->head); > - kref_init(&block->kref); > > iio_buffer_get(&queue->buffer); > > return block; > + > +err_free_dma: > + dma_free_coherent(queue->dev, PAGE_ALIGN(size), > + block->vaddr, block->phys_addr); > +err_free_block: > + kfree(block); > + return ERR_PTR(err); > } > > static void _iio_dma_buffer_block_done(struct iio_dma_buffer_block *block) > @@ -223,7 +266,7 @@ void iio_dma_buffer_block_done(struct iio_dma_buffer_block *block) > _iio_dma_buffer_block_done(block); > spin_unlock_irqrestore(&queue->list_lock, flags); > > - iio_buffer_block_put_atomic(block); > + iio_buffer_block_put(block); > iio_dma_buffer_queue_wake(queue); > } > EXPORT_SYMBOL_GPL(iio_dma_buffer_block_done); > @@ -249,7 +292,8 @@ void iio_dma_buffer_block_list_abort(struct iio_dma_buffer_queue *queue, > list_del(&block->head); > block->bytes_used = 0; > _iio_dma_buffer_block_done(block); > - iio_buffer_block_put_atomic(block); > + > + iio_buffer_block_put(block); > } > spin_unlock_irqrestore(&queue->list_lock, flags); > > @@ -340,8 +384,8 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer) > > if (!block) { > block = iio_dma_buffer_alloc_block(queue, size); > - if (!block) { > - ret = -ENOMEM; > + if (IS_ERR(block)) { > + ret = PTR_ERR(block); > goto out_unlock; > } > queue->fileio.blocks[i] = block; > diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h > index 490b93f76fa8..6b3fa7d2124b 100644 > --- a/include/linux/iio/buffer-dma.h > +++ b/include/linux/iio/buffer-dma.h > @@ -8,7 +8,6 @@ > #define __INDUSTRIALIO_DMA_BUFFER_H__ > > #include <linux/list.h> > -#include <linux/kref.h> > #include <linux/spinlock.h> > #include <linux/mutex.h> > #include <linux/iio/buffer_impl.h> > @@ -16,6 +15,7 @@ > struct iio_dma_buffer_queue; > struct iio_dma_buffer_ops; > struct device; > +struct dma_buf; > > /** > * enum iio_block_state - State of a struct iio_dma_buffer_block > @@ -39,8 +39,8 @@ enum iio_block_state { > * @vaddr: Virutal address of the blocks memory > * @phys_addr: Physical address of the blocks memory > * @queue: Parent DMA buffer queue > - * @kref: kref used to manage the lifetime of block > * @state: Current state of the block > + * @dmabuf: Underlying DMABUF object > */ > struct iio_dma_buffer_block { > /* May only be accessed by the owner of the block */ > @@ -56,13 +56,13 @@ struct iio_dma_buffer_block { > size_t size; > struct iio_dma_buffer_queue *queue; > > - /* Must not be accessed outside the core. */ > - struct kref kref; > /* > * Must not be accessed outside the core. Access needs to hold > * queue->list_lock if the block is not owned by the core. > */ > enum iio_block_state state; > + > + struct dma_buf *dmabuf; > }; > > /**
Am 28.03.22 um 19:54 schrieb Jonathan Cameron: > On Mon, 7 Feb 2022 12:59:28 +0000 > Paul Cercueil <paul@crapouillou.net> wrote: > >> Enhance the current fileio code by using DMABUF objects instead of >> custom buffers. >> >> This adds more code than it removes, but: >> - a lot of the complexity can be dropped, e.g. custom kref and >> iio_buffer_block_put_atomic() are not needed anymore; >> - it will be much easier to introduce an API to export these DMABUF >> objects to userspace in a following patch. >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> > Hi Paul, > > I'm a bit rusty on dma mappings, but you seem to have > a mixture of streaming and coherent mappings going on in here. > > Is it the case that the current code is using the coherent mappings > and a potential 'other user' of the dma buffer might need > streaming mappings? Streaming mappings are generally not supported by DMA-buf. You always have only coherent mappings. Regards, Christian. > > Jonathan > >> --- >> drivers/iio/buffer/industrialio-buffer-dma.c | 192 ++++++++++++------- >> include/linux/iio/buffer-dma.h | 8 +- >> 2 files changed, 122 insertions(+), 78 deletions(-) >> >> diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c b/drivers/iio/buffer/industrialio-buffer-dma.c >> index 15ea7bc3ac08..54e6000cd2ee 100644 >> --- a/drivers/iio/buffer/industrialio-buffer-dma.c >> +++ b/drivers/iio/buffer/industrialio-buffer-dma.c >> @@ -14,6 +14,7 @@ >> #include <linux/poll.h> >> #include <linux/iio/buffer_impl.h> >> #include <linux/iio/buffer-dma.h> >> +#include <linux/dma-buf.h> >> #include <linux/dma-mapping.h> >> #include <linux/sizes.h> >> >> @@ -90,103 +91,145 @@ >> * callback is called from within the custom callback. >> */ >> >> -static void iio_buffer_block_release(struct kref *kref) >> -{ >> - struct iio_dma_buffer_block *block = container_of(kref, >> - struct iio_dma_buffer_block, kref); >> - >> - WARN_ON(block->state != IIO_BLOCK_STATE_DEAD); >> - >> - dma_free_coherent(block->queue->dev, PAGE_ALIGN(block->size), >> - block->vaddr, block->phys_addr); >> - >> - iio_buffer_put(&block->queue->buffer); >> - kfree(block); >> -} >> - >> -static void iio_buffer_block_get(struct iio_dma_buffer_block *block) >> -{ >> - kref_get(&block->kref); >> -} >> - >> -static void iio_buffer_block_put(struct iio_dma_buffer_block *block) >> -{ >> - kref_put(&block->kref, iio_buffer_block_release); >> -} >> - >> -/* >> - * dma_free_coherent can sleep, hence we need to take some special care to be >> - * able to drop a reference from an atomic context. >> - */ >> -static LIST_HEAD(iio_dma_buffer_dead_blocks); >> -static DEFINE_SPINLOCK(iio_dma_buffer_dead_blocks_lock); >> - >> -static void iio_dma_buffer_cleanup_worker(struct work_struct *work) >> -{ >> - struct iio_dma_buffer_block *block, *_block; >> - LIST_HEAD(block_list); >> - >> - spin_lock_irq(&iio_dma_buffer_dead_blocks_lock); >> - list_splice_tail_init(&iio_dma_buffer_dead_blocks, &block_list); >> - spin_unlock_irq(&iio_dma_buffer_dead_blocks_lock); >> - >> - list_for_each_entry_safe(block, _block, &block_list, head) >> - iio_buffer_block_release(&block->kref); >> -} >> -static DECLARE_WORK(iio_dma_buffer_cleanup_work, iio_dma_buffer_cleanup_worker); >> - >> -static void iio_buffer_block_release_atomic(struct kref *kref) >> -{ >> +struct iio_buffer_dma_buf_attachment { >> + struct scatterlist sgl; >> + struct sg_table sg_table; >> struct iio_dma_buffer_block *block; >> - unsigned long flags; >> - >> - block = container_of(kref, struct iio_dma_buffer_block, kref); >> - >> - spin_lock_irqsave(&iio_dma_buffer_dead_blocks_lock, flags); >> - list_add_tail(&block->head, &iio_dma_buffer_dead_blocks); >> - spin_unlock_irqrestore(&iio_dma_buffer_dead_blocks_lock, flags); >> - >> - schedule_work(&iio_dma_buffer_cleanup_work); >> -} >> - >> -/* >> - * Version of iio_buffer_block_put() that can be called from atomic context >> - */ >> -static void iio_buffer_block_put_atomic(struct iio_dma_buffer_block *block) >> -{ >> - kref_put(&block->kref, iio_buffer_block_release_atomic); >> -} >> +}; >> >> static struct iio_dma_buffer_queue *iio_buffer_to_queue(struct iio_buffer *buf) >> { >> return container_of(buf, struct iio_dma_buffer_queue, buffer); >> } >> >> +static struct iio_buffer_dma_buf_attachment * >> +to_iio_buffer_dma_buf_attachment(struct sg_table *table) >> +{ >> + return container_of(table, struct iio_buffer_dma_buf_attachment, sg_table); >> +} >> + >> +static void iio_buffer_block_get(struct iio_dma_buffer_block *block) >> +{ >> + get_dma_buf(block->dmabuf); >> +} >> + >> +static void iio_buffer_block_put(struct iio_dma_buffer_block *block) >> +{ >> + dma_buf_put(block->dmabuf); >> +} >> + >> +static int iio_buffer_dma_buf_attach(struct dma_buf *dbuf, >> + struct dma_buf_attachment *at) >> +{ >> + at->priv = dbuf->priv; >> + >> + return 0; >> +} >> + >> +static struct sg_table *iio_buffer_dma_buf_map(struct dma_buf_attachment *at, >> + enum dma_data_direction dma_dir) >> +{ >> + struct iio_dma_buffer_block *block = at->priv; >> + struct iio_buffer_dma_buf_attachment *dba; >> + int ret; >> + >> + dba = kzalloc(sizeof(*dba), GFP_KERNEL); >> + if (!dba) >> + return ERR_PTR(-ENOMEM); >> + >> + sg_init_one(&dba->sgl, block->vaddr, PAGE_ALIGN(block->size)); >> + dba->sg_table.sgl = &dba->sgl; >> + dba->sg_table.nents = 1; >> + dba->block = block; >> + >> + ret = dma_map_sgtable(at->dev, &dba->sg_table, dma_dir, 0); >> + if (ret) { >> + kfree(dba); >> + return ERR_PTR(ret); >> + } >> + >> + return &dba->sg_table; >> +} >> + >> +static void iio_buffer_dma_buf_unmap(struct dma_buf_attachment *at, >> + struct sg_table *sg_table, >> + enum dma_data_direction dma_dir) >> +{ >> + struct iio_buffer_dma_buf_attachment *dba = >> + to_iio_buffer_dma_buf_attachment(sg_table); >> + >> + dma_unmap_sgtable(at->dev, &dba->sg_table, dma_dir, 0); >> + kfree(dba); >> +} >> + >> +static void iio_buffer_dma_buf_release(struct dma_buf *dbuf) >> +{ >> + struct iio_dma_buffer_block *block = dbuf->priv; >> + struct iio_dma_buffer_queue *queue = block->queue; >> + >> + WARN_ON(block->state != IIO_BLOCK_STATE_DEAD); >> + >> + mutex_lock(&queue->lock); >> + >> + dma_free_coherent(queue->dev, PAGE_ALIGN(block->size), >> + block->vaddr, block->phys_addr); >> + kfree(block); >> + >> + mutex_unlock(&queue->lock); >> + iio_buffer_put(&queue->buffer); >> +} >> + >> +static const struct dma_buf_ops iio_dma_buffer_dmabuf_ops = { >> + .attach = iio_buffer_dma_buf_attach, >> + .map_dma_buf = iio_buffer_dma_buf_map, >> + .unmap_dma_buf = iio_buffer_dma_buf_unmap, >> + .release = iio_buffer_dma_buf_release, >> +}; >> + >> static struct iio_dma_buffer_block *iio_dma_buffer_alloc_block( >> struct iio_dma_buffer_queue *queue, size_t size) >> { >> struct iio_dma_buffer_block *block; >> + DEFINE_DMA_BUF_EXPORT_INFO(einfo); >> + struct dma_buf *dmabuf; >> + int err = -ENOMEM; >> >> block = kzalloc(sizeof(*block), GFP_KERNEL); >> if (!block) >> - return NULL; >> + return ERR_PTR(err); >> >> block->vaddr = dma_alloc_coherent(queue->dev, PAGE_ALIGN(size), >> &block->phys_addr, GFP_KERNEL); >> - if (!block->vaddr) { >> - kfree(block); >> - return NULL; >> + if (!block->vaddr) >> + goto err_free_block; >> + >> + einfo.ops = &iio_dma_buffer_dmabuf_ops; >> + einfo.size = PAGE_ALIGN(size); >> + einfo.priv = block; >> + einfo.flags = O_RDWR; >> + >> + dmabuf = dma_buf_export(&einfo); >> + if (IS_ERR(dmabuf)) { >> + err = PTR_ERR(dmabuf); >> + goto err_free_dma; >> } >> >> + block->dmabuf = dmabuf; >> block->size = size; >> block->state = IIO_BLOCK_STATE_DONE; >> block->queue = queue; >> INIT_LIST_HEAD(&block->head); >> - kref_init(&block->kref); >> >> iio_buffer_get(&queue->buffer); >> >> return block; >> + >> +err_free_dma: >> + dma_free_coherent(queue->dev, PAGE_ALIGN(size), >> + block->vaddr, block->phys_addr); >> +err_free_block: >> + kfree(block); >> + return ERR_PTR(err); >> } >> >> static void _iio_dma_buffer_block_done(struct iio_dma_buffer_block *block) >> @@ -223,7 +266,7 @@ void iio_dma_buffer_block_done(struct iio_dma_buffer_block *block) >> _iio_dma_buffer_block_done(block); >> spin_unlock_irqrestore(&queue->list_lock, flags); >> >> - iio_buffer_block_put_atomic(block); >> + iio_buffer_block_put(block); >> iio_dma_buffer_queue_wake(queue); >> } >> EXPORT_SYMBOL_GPL(iio_dma_buffer_block_done); >> @@ -249,7 +292,8 @@ void iio_dma_buffer_block_list_abort(struct iio_dma_buffer_queue *queue, >> list_del(&block->head); >> block->bytes_used = 0; >> _iio_dma_buffer_block_done(block); >> - iio_buffer_block_put_atomic(block); >> + >> + iio_buffer_block_put(block); >> } >> spin_unlock_irqrestore(&queue->list_lock, flags); >> >> @@ -340,8 +384,8 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer) >> >> if (!block) { >> block = iio_dma_buffer_alloc_block(queue, size); >> - if (!block) { >> - ret = -ENOMEM; >> + if (IS_ERR(block)) { >> + ret = PTR_ERR(block); >> goto out_unlock; >> } >> queue->fileio.blocks[i] = block; >> diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h >> index 490b93f76fa8..6b3fa7d2124b 100644 >> --- a/include/linux/iio/buffer-dma.h >> +++ b/include/linux/iio/buffer-dma.h >> @@ -8,7 +8,6 @@ >> #define __INDUSTRIALIO_DMA_BUFFER_H__ >> >> #include <linux/list.h> >> -#include <linux/kref.h> >> #include <linux/spinlock.h> >> #include <linux/mutex.h> >> #include <linux/iio/buffer_impl.h> >> @@ -16,6 +15,7 @@ >> struct iio_dma_buffer_queue; >> struct iio_dma_buffer_ops; >> struct device; >> +struct dma_buf; >> >> /** >> * enum iio_block_state - State of a struct iio_dma_buffer_block >> @@ -39,8 +39,8 @@ enum iio_block_state { >> * @vaddr: Virutal address of the blocks memory >> * @phys_addr: Physical address of the blocks memory >> * @queue: Parent DMA buffer queue >> - * @kref: kref used to manage the lifetime of block >> * @state: Current state of the block >> + * @dmabuf: Underlying DMABUF object >> */ >> struct iio_dma_buffer_block { >> /* May only be accessed by the owner of the block */ >> @@ -56,13 +56,13 @@ struct iio_dma_buffer_block { >> size_t size; >> struct iio_dma_buffer_queue *queue; >> >> - /* Must not be accessed outside the core. */ >> - struct kref kref; >> /* >> * Must not be accessed outside the core. Access needs to hold >> * queue->list_lock if the block is not owned by the core. >> */ >> enum iio_block_state state; >> + >> + struct dma_buf *dmabuf; >> }; >> >> /**
Hi Jonathan, Le lun., mars 28 2022 at 18:54:25 +0100, Jonathan Cameron <jic23@kernel.org> a écrit : > On Mon, 7 Feb 2022 12:59:28 +0000 > Paul Cercueil <paul@crapouillou.net> wrote: > >> Enhance the current fileio code by using DMABUF objects instead of >> custom buffers. >> >> This adds more code than it removes, but: >> - a lot of the complexity can be dropped, e.g. custom kref and >> iio_buffer_block_put_atomic() are not needed anymore; >> - it will be much easier to introduce an API to export these DMABUF >> objects to userspace in a following patch. >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> > Hi Paul, > > I'm a bit rusty on dma mappings, but you seem to have > a mixture of streaming and coherent mappings going on in here. That's OK, so am I. What do you call "streaming mappings"? > Is it the case that the current code is using the coherent mappings > and a potential 'other user' of the dma buffer might need > streaming mappings? Something like that. There are two different things; on both cases, userspace needs to create a DMABUF with IIO_BUFFER_DMABUF_ALLOC_IOCTL, and the backing memory is allocated with dma_alloc_coherent(). - For the userspace interface, you then have a "cpu access" IOCTL (DMA_BUF_IOCTL_SYNC), that allows userspace to inform when it will start/finish to process the buffer in user-space (which will sync/invalidate the data cache if needed). A buffer can then be enqueued for DMA processing (TX or RX) with the new IIO_BUFFER_DMABUF_ENQUEUE_IOCTL. - When the DMABUF created via the IIO core is sent to another driver through the driver's custom DMABUF import function, this driver will call dma_buf_attach(), which will call iio_buffer_dma_buf_map(). Since it has to return a "struct sg_table *", this function then simply creates a sgtable with one entry that points to the backing memory. Note that I added the iio_buffer_dma_buf_map() / _unmap() functions because the dma-buf core would WARN() if these were not provided. But since this code doesn't yet support importing/exporting DMABUFs to other drivers, these are never called, and I should probably just make them return a ERR_PTR() unconditionally. Cheers, -Paul > Jonathan > >> --- >> drivers/iio/buffer/industrialio-buffer-dma.c | 192 >> ++++++++++++------- >> include/linux/iio/buffer-dma.h | 8 +- >> 2 files changed, 122 insertions(+), 78 deletions(-) >> >> diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c >> b/drivers/iio/buffer/industrialio-buffer-dma.c >> index 15ea7bc3ac08..54e6000cd2ee 100644 >> --- a/drivers/iio/buffer/industrialio-buffer-dma.c >> +++ b/drivers/iio/buffer/industrialio-buffer-dma.c >> @@ -14,6 +14,7 @@ >> #include <linux/poll.h> >> #include <linux/iio/buffer_impl.h> >> #include <linux/iio/buffer-dma.h> >> +#include <linux/dma-buf.h> >> #include <linux/dma-mapping.h> >> #include <linux/sizes.h> >> >> @@ -90,103 +91,145 @@ >> * callback is called from within the custom callback. >> */ >> >> -static void iio_buffer_block_release(struct kref *kref) >> -{ >> - struct iio_dma_buffer_block *block = container_of(kref, >> - struct iio_dma_buffer_block, kref); >> - >> - WARN_ON(block->state != IIO_BLOCK_STATE_DEAD); >> - >> - dma_free_coherent(block->queue->dev, PAGE_ALIGN(block->size), >> - block->vaddr, block->phys_addr); >> - >> - iio_buffer_put(&block->queue->buffer); >> - kfree(block); >> -} >> - >> -static void iio_buffer_block_get(struct iio_dma_buffer_block >> *block) >> -{ >> - kref_get(&block->kref); >> -} >> - >> -static void iio_buffer_block_put(struct iio_dma_buffer_block >> *block) >> -{ >> - kref_put(&block->kref, iio_buffer_block_release); >> -} >> - >> -/* >> - * dma_free_coherent can sleep, hence we need to take some special >> care to be >> - * able to drop a reference from an atomic context. >> - */ >> -static LIST_HEAD(iio_dma_buffer_dead_blocks); >> -static DEFINE_SPINLOCK(iio_dma_buffer_dead_blocks_lock); >> - >> -static void iio_dma_buffer_cleanup_worker(struct work_struct *work) >> -{ >> - struct iio_dma_buffer_block *block, *_block; >> - LIST_HEAD(block_list); >> - >> - spin_lock_irq(&iio_dma_buffer_dead_blocks_lock); >> - list_splice_tail_init(&iio_dma_buffer_dead_blocks, &block_list); >> - spin_unlock_irq(&iio_dma_buffer_dead_blocks_lock); >> - >> - list_for_each_entry_safe(block, _block, &block_list, head) >> - iio_buffer_block_release(&block->kref); >> -} >> -static DECLARE_WORK(iio_dma_buffer_cleanup_work, >> iio_dma_buffer_cleanup_worker); >> - >> -static void iio_buffer_block_release_atomic(struct kref *kref) >> -{ >> +struct iio_buffer_dma_buf_attachment { >> + struct scatterlist sgl; >> + struct sg_table sg_table; >> struct iio_dma_buffer_block *block; >> - unsigned long flags; >> - >> - block = container_of(kref, struct iio_dma_buffer_block, kref); >> - >> - spin_lock_irqsave(&iio_dma_buffer_dead_blocks_lock, flags); >> - list_add_tail(&block->head, &iio_dma_buffer_dead_blocks); >> - spin_unlock_irqrestore(&iio_dma_buffer_dead_blocks_lock, flags); >> - >> - schedule_work(&iio_dma_buffer_cleanup_work); >> -} >> - >> -/* >> - * Version of iio_buffer_block_put() that can be called from >> atomic context >> - */ >> -static void iio_buffer_block_put_atomic(struct >> iio_dma_buffer_block *block) >> -{ >> - kref_put(&block->kref, iio_buffer_block_release_atomic); >> -} >> +}; >> >> static struct iio_dma_buffer_queue *iio_buffer_to_queue(struct >> iio_buffer *buf) >> { >> return container_of(buf, struct iio_dma_buffer_queue, buffer); >> } >> >> +static struct iio_buffer_dma_buf_attachment * >> +to_iio_buffer_dma_buf_attachment(struct sg_table *table) >> +{ >> + return container_of(table, struct iio_buffer_dma_buf_attachment, >> sg_table); >> +} >> + >> +static void iio_buffer_block_get(struct iio_dma_buffer_block >> *block) >> +{ >> + get_dma_buf(block->dmabuf); >> +} >> + >> +static void iio_buffer_block_put(struct iio_dma_buffer_block >> *block) >> +{ >> + dma_buf_put(block->dmabuf); >> +} >> + >> +static int iio_buffer_dma_buf_attach(struct dma_buf *dbuf, >> + struct dma_buf_attachment *at) >> +{ >> + at->priv = dbuf->priv; >> + >> + return 0; >> +} >> + >> +static struct sg_table *iio_buffer_dma_buf_map(struct >> dma_buf_attachment *at, >> + enum dma_data_direction dma_dir) >> +{ >> + struct iio_dma_buffer_block *block = at->priv; >> + struct iio_buffer_dma_buf_attachment *dba; >> + int ret; >> + >> + dba = kzalloc(sizeof(*dba), GFP_KERNEL); >> + if (!dba) >> + return ERR_PTR(-ENOMEM); >> + >> + sg_init_one(&dba->sgl, block->vaddr, PAGE_ALIGN(block->size)); >> + dba->sg_table.sgl = &dba->sgl; >> + dba->sg_table.nents = 1; >> + dba->block = block; >> + >> + ret = dma_map_sgtable(at->dev, &dba->sg_table, dma_dir, 0); >> + if (ret) { >> + kfree(dba); >> + return ERR_PTR(ret); >> + } >> + >> + return &dba->sg_table; >> +} >> + >> +static void iio_buffer_dma_buf_unmap(struct dma_buf_attachment *at, >> + struct sg_table *sg_table, >> + enum dma_data_direction dma_dir) >> +{ >> + struct iio_buffer_dma_buf_attachment *dba = >> + to_iio_buffer_dma_buf_attachment(sg_table); >> + >> + dma_unmap_sgtable(at->dev, &dba->sg_table, dma_dir, 0); >> + kfree(dba); >> +} >> + >> +static void iio_buffer_dma_buf_release(struct dma_buf *dbuf) >> +{ >> + struct iio_dma_buffer_block *block = dbuf->priv; >> + struct iio_dma_buffer_queue *queue = block->queue; >> + >> + WARN_ON(block->state != IIO_BLOCK_STATE_DEAD); >> + >> + mutex_lock(&queue->lock); >> + >> + dma_free_coherent(queue->dev, PAGE_ALIGN(block->size), >> + block->vaddr, block->phys_addr); >> + kfree(block); >> + >> + mutex_unlock(&queue->lock); >> + iio_buffer_put(&queue->buffer); >> +} >> + >> +static const struct dma_buf_ops iio_dma_buffer_dmabuf_ops = { >> + .attach = iio_buffer_dma_buf_attach, >> + .map_dma_buf = iio_buffer_dma_buf_map, >> + .unmap_dma_buf = iio_buffer_dma_buf_unmap, >> + .release = iio_buffer_dma_buf_release, >> +}; >> + >> static struct iio_dma_buffer_block *iio_dma_buffer_alloc_block( >> struct iio_dma_buffer_queue *queue, size_t size) >> { >> struct iio_dma_buffer_block *block; >> + DEFINE_DMA_BUF_EXPORT_INFO(einfo); >> + struct dma_buf *dmabuf; >> + int err = -ENOMEM; >> >> block = kzalloc(sizeof(*block), GFP_KERNEL); >> if (!block) >> - return NULL; >> + return ERR_PTR(err); >> >> block->vaddr = dma_alloc_coherent(queue->dev, PAGE_ALIGN(size), >> &block->phys_addr, GFP_KERNEL); >> - if (!block->vaddr) { >> - kfree(block); >> - return NULL; >> + if (!block->vaddr) >> + goto err_free_block; >> + >> + einfo.ops = &iio_dma_buffer_dmabuf_ops; >> + einfo.size = PAGE_ALIGN(size); >> + einfo.priv = block; >> + einfo.flags = O_RDWR; >> + >> + dmabuf = dma_buf_export(&einfo); >> + if (IS_ERR(dmabuf)) { >> + err = PTR_ERR(dmabuf); >> + goto err_free_dma; >> } >> >> + block->dmabuf = dmabuf; >> block->size = size; >> block->state = IIO_BLOCK_STATE_DONE; >> block->queue = queue; >> INIT_LIST_HEAD(&block->head); >> - kref_init(&block->kref); >> >> iio_buffer_get(&queue->buffer); >> >> return block; >> + >> +err_free_dma: >> + dma_free_coherent(queue->dev, PAGE_ALIGN(size), >> + block->vaddr, block->phys_addr); >> +err_free_block: >> + kfree(block); >> + return ERR_PTR(err); >> } >> >> static void _iio_dma_buffer_block_done(struct iio_dma_buffer_block >> *block) >> @@ -223,7 +266,7 @@ void iio_dma_buffer_block_done(struct >> iio_dma_buffer_block *block) >> _iio_dma_buffer_block_done(block); >> spin_unlock_irqrestore(&queue->list_lock, flags); >> >> - iio_buffer_block_put_atomic(block); >> + iio_buffer_block_put(block); >> iio_dma_buffer_queue_wake(queue); >> } >> EXPORT_SYMBOL_GPL(iio_dma_buffer_block_done); >> @@ -249,7 +292,8 @@ void iio_dma_buffer_block_list_abort(struct >> iio_dma_buffer_queue *queue, >> list_del(&block->head); >> block->bytes_used = 0; >> _iio_dma_buffer_block_done(block); >> - iio_buffer_block_put_atomic(block); >> + >> + iio_buffer_block_put(block); >> } >> spin_unlock_irqrestore(&queue->list_lock, flags); >> >> @@ -340,8 +384,8 @@ int iio_dma_buffer_request_update(struct >> iio_buffer *buffer) >> >> if (!block) { >> block = iio_dma_buffer_alloc_block(queue, size); >> - if (!block) { >> - ret = -ENOMEM; >> + if (IS_ERR(block)) { >> + ret = PTR_ERR(block); >> goto out_unlock; >> } >> queue->fileio.blocks[i] = block; >> diff --git a/include/linux/iio/buffer-dma.h >> b/include/linux/iio/buffer-dma.h >> index 490b93f76fa8..6b3fa7d2124b 100644 >> --- a/include/linux/iio/buffer-dma.h >> +++ b/include/linux/iio/buffer-dma.h >> @@ -8,7 +8,6 @@ >> #define __INDUSTRIALIO_DMA_BUFFER_H__ >> >> #include <linux/list.h> >> -#include <linux/kref.h> >> #include <linux/spinlock.h> >> #include <linux/mutex.h> >> #include <linux/iio/buffer_impl.h> >> @@ -16,6 +15,7 @@ >> struct iio_dma_buffer_queue; >> struct iio_dma_buffer_ops; >> struct device; >> +struct dma_buf; >> >> /** >> * enum iio_block_state - State of a struct iio_dma_buffer_block >> @@ -39,8 +39,8 @@ enum iio_block_state { >> * @vaddr: Virutal address of the blocks memory >> * @phys_addr: Physical address of the blocks memory >> * @queue: Parent DMA buffer queue >> - * @kref: kref used to manage the lifetime of block >> * @state: Current state of the block >> + * @dmabuf: Underlying DMABUF object >> */ >> struct iio_dma_buffer_block { >> /* May only be accessed by the owner of the block */ >> @@ -56,13 +56,13 @@ struct iio_dma_buffer_block { >> size_t size; >> struct iio_dma_buffer_queue *queue; >> >> - /* Must not be accessed outside the core. */ >> - struct kref kref; >> /* >> * Must not be accessed outside the core. Access needs to hold >> * queue->list_lock if the block is not owned by the core. >> */ >> enum iio_block_state state; >> + >> + struct dma_buf *dmabuf; >> }; >> >> /** >
On Mon, Mar 28, 2022 at 11:30 PM Paul Cercueil <paul@crapouillou.net> wrote: > Le lun., mars 28 2022 at 18:54:25 +0100, Jonathan Cameron > <jic23@kernel.org> a écrit : > > On Mon, 7 Feb 2022 12:59:28 +0000 > > Paul Cercueil <paul@crapouillou.net> wrote: > > > >> Enhance the current fileio code by using DMABUF objects instead of > >> custom buffers. > >> > >> This adds more code than it removes, but: > >> - a lot of the complexity can be dropped, e.g. custom kref and > >> iio_buffer_block_put_atomic() are not needed anymore; > >> - it will be much easier to introduce an API to export these DMABUF > >> objects to userspace in a following patch. > > I'm a bit rusty on dma mappings, but you seem to have > > a mixture of streaming and coherent mappings going on in here. > > That's OK, so am I. What do you call "streaming mappings"? dma_*_coherent() are for coherent mappings (usually you do it once and cache coherency is guaranteed by accessing this memory by device or CPU). dma_map_*() are for streaming, which means that you often want to map arbitrary pages during the transfer (usually used for the cases when you want to keep previous data and do something with a new coming, or when a new coming data is supplied by different virtual address, and hence has to be mapped for DMA). > > Is it the case that the current code is using the coherent mappings > > and a potential 'other user' of the dma buffer might need > > streaming mappings? > > Something like that. There are two different things; on both cases, > userspace needs to create a DMABUF with IIO_BUFFER_DMABUF_ALLOC_IOCTL, > and the backing memory is allocated with dma_alloc_coherent(). > > - For the userspace interface, you then have a "cpu access" IOCTL > (DMA_BUF_IOCTL_SYNC), that allows userspace to inform when it will > start/finish to process the buffer in user-space (which will > sync/invalidate the data cache if needed). A buffer can then be > enqueued for DMA processing (TX or RX) with the new > IIO_BUFFER_DMABUF_ENQUEUE_IOCTL. > > - When the DMABUF created via the IIO core is sent to another driver > through the driver's custom DMABUF import function, this driver will > call dma_buf_attach(), which will call iio_buffer_dma_buf_map(). Since > it has to return a "struct sg_table *", this function then simply > creates a sgtable with one entry that points to the backing memory. ... > >> + ret = dma_map_sgtable(at->dev, &dba->sg_table, dma_dir, 0); > >> + if (ret) { > >> + kfree(dba); > >> + return ERR_PTR(ret); > >> + } Missed DMA mapping error check. > >> + > >> + return &dba->sg_table; > >> +} ... > >> - /* Must not be accessed outside the core. */ > >> - struct kref kref; > >> + struct dma_buf *dmabuf; Is it okay to access outside the core? If no, why did you remove (actually not modify) the comment?
diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c b/drivers/iio/buffer/industrialio-buffer-dma.c index 15ea7bc3ac08..54e6000cd2ee 100644 --- a/drivers/iio/buffer/industrialio-buffer-dma.c +++ b/drivers/iio/buffer/industrialio-buffer-dma.c @@ -14,6 +14,7 @@ #include <linux/poll.h> #include <linux/iio/buffer_impl.h> #include <linux/iio/buffer-dma.h> +#include <linux/dma-buf.h> #include <linux/dma-mapping.h> #include <linux/sizes.h> @@ -90,103 +91,145 @@ * callback is called from within the custom callback. */ -static void iio_buffer_block_release(struct kref *kref) -{ - struct iio_dma_buffer_block *block = container_of(kref, - struct iio_dma_buffer_block, kref); - - WARN_ON(block->state != IIO_BLOCK_STATE_DEAD); - - dma_free_coherent(block->queue->dev, PAGE_ALIGN(block->size), - block->vaddr, block->phys_addr); - - iio_buffer_put(&block->queue->buffer); - kfree(block); -} - -static void iio_buffer_block_get(struct iio_dma_buffer_block *block) -{ - kref_get(&block->kref); -} - -static void iio_buffer_block_put(struct iio_dma_buffer_block *block) -{ - kref_put(&block->kref, iio_buffer_block_release); -} - -/* - * dma_free_coherent can sleep, hence we need to take some special care to be - * able to drop a reference from an atomic context. - */ -static LIST_HEAD(iio_dma_buffer_dead_blocks); -static DEFINE_SPINLOCK(iio_dma_buffer_dead_blocks_lock); - -static void iio_dma_buffer_cleanup_worker(struct work_struct *work) -{ - struct iio_dma_buffer_block *block, *_block; - LIST_HEAD(block_list); - - spin_lock_irq(&iio_dma_buffer_dead_blocks_lock); - list_splice_tail_init(&iio_dma_buffer_dead_blocks, &block_list); - spin_unlock_irq(&iio_dma_buffer_dead_blocks_lock); - - list_for_each_entry_safe(block, _block, &block_list, head) - iio_buffer_block_release(&block->kref); -} -static DECLARE_WORK(iio_dma_buffer_cleanup_work, iio_dma_buffer_cleanup_worker); - -static void iio_buffer_block_release_atomic(struct kref *kref) -{ +struct iio_buffer_dma_buf_attachment { + struct scatterlist sgl; + struct sg_table sg_table; struct iio_dma_buffer_block *block; - unsigned long flags; - - block = container_of(kref, struct iio_dma_buffer_block, kref); - - spin_lock_irqsave(&iio_dma_buffer_dead_blocks_lock, flags); - list_add_tail(&block->head, &iio_dma_buffer_dead_blocks); - spin_unlock_irqrestore(&iio_dma_buffer_dead_blocks_lock, flags); - - schedule_work(&iio_dma_buffer_cleanup_work); -} - -/* - * Version of iio_buffer_block_put() that can be called from atomic context - */ -static void iio_buffer_block_put_atomic(struct iio_dma_buffer_block *block) -{ - kref_put(&block->kref, iio_buffer_block_release_atomic); -} +}; static struct iio_dma_buffer_queue *iio_buffer_to_queue(struct iio_buffer *buf) { return container_of(buf, struct iio_dma_buffer_queue, buffer); } +static struct iio_buffer_dma_buf_attachment * +to_iio_buffer_dma_buf_attachment(struct sg_table *table) +{ + return container_of(table, struct iio_buffer_dma_buf_attachment, sg_table); +} + +static void iio_buffer_block_get(struct iio_dma_buffer_block *block) +{ + get_dma_buf(block->dmabuf); +} + +static void iio_buffer_block_put(struct iio_dma_buffer_block *block) +{ + dma_buf_put(block->dmabuf); +} + +static int iio_buffer_dma_buf_attach(struct dma_buf *dbuf, + struct dma_buf_attachment *at) +{ + at->priv = dbuf->priv; + + return 0; +} + +static struct sg_table *iio_buffer_dma_buf_map(struct dma_buf_attachment *at, + enum dma_data_direction dma_dir) +{ + struct iio_dma_buffer_block *block = at->priv; + struct iio_buffer_dma_buf_attachment *dba; + int ret; + + dba = kzalloc(sizeof(*dba), GFP_KERNEL); + if (!dba) + return ERR_PTR(-ENOMEM); + + sg_init_one(&dba->sgl, block->vaddr, PAGE_ALIGN(block->size)); + dba->sg_table.sgl = &dba->sgl; + dba->sg_table.nents = 1; + dba->block = block; + + ret = dma_map_sgtable(at->dev, &dba->sg_table, dma_dir, 0); + if (ret) { + kfree(dba); + return ERR_PTR(ret); + } + + return &dba->sg_table; +} + +static void iio_buffer_dma_buf_unmap(struct dma_buf_attachment *at, + struct sg_table *sg_table, + enum dma_data_direction dma_dir) +{ + struct iio_buffer_dma_buf_attachment *dba = + to_iio_buffer_dma_buf_attachment(sg_table); + + dma_unmap_sgtable(at->dev, &dba->sg_table, dma_dir, 0); + kfree(dba); +} + +static void iio_buffer_dma_buf_release(struct dma_buf *dbuf) +{ + struct iio_dma_buffer_block *block = dbuf->priv; + struct iio_dma_buffer_queue *queue = block->queue; + + WARN_ON(block->state != IIO_BLOCK_STATE_DEAD); + + mutex_lock(&queue->lock); + + dma_free_coherent(queue->dev, PAGE_ALIGN(block->size), + block->vaddr, block->phys_addr); + kfree(block); + + mutex_unlock(&queue->lock); + iio_buffer_put(&queue->buffer); +} + +static const struct dma_buf_ops iio_dma_buffer_dmabuf_ops = { + .attach = iio_buffer_dma_buf_attach, + .map_dma_buf = iio_buffer_dma_buf_map, + .unmap_dma_buf = iio_buffer_dma_buf_unmap, + .release = iio_buffer_dma_buf_release, +}; + static struct iio_dma_buffer_block *iio_dma_buffer_alloc_block( struct iio_dma_buffer_queue *queue, size_t size) { struct iio_dma_buffer_block *block; + DEFINE_DMA_BUF_EXPORT_INFO(einfo); + struct dma_buf *dmabuf; + int err = -ENOMEM; block = kzalloc(sizeof(*block), GFP_KERNEL); if (!block) - return NULL; + return ERR_PTR(err); block->vaddr = dma_alloc_coherent(queue->dev, PAGE_ALIGN(size), &block->phys_addr, GFP_KERNEL); - if (!block->vaddr) { - kfree(block); - return NULL; + if (!block->vaddr) + goto err_free_block; + + einfo.ops = &iio_dma_buffer_dmabuf_ops; + einfo.size = PAGE_ALIGN(size); + einfo.priv = block; + einfo.flags = O_RDWR; + + dmabuf = dma_buf_export(&einfo); + if (IS_ERR(dmabuf)) { + err = PTR_ERR(dmabuf); + goto err_free_dma; } + block->dmabuf = dmabuf; block->size = size; block->state = IIO_BLOCK_STATE_DONE; block->queue = queue; INIT_LIST_HEAD(&block->head); - kref_init(&block->kref); iio_buffer_get(&queue->buffer); return block; + +err_free_dma: + dma_free_coherent(queue->dev, PAGE_ALIGN(size), + block->vaddr, block->phys_addr); +err_free_block: + kfree(block); + return ERR_PTR(err); } static void _iio_dma_buffer_block_done(struct iio_dma_buffer_block *block) @@ -223,7 +266,7 @@ void iio_dma_buffer_block_done(struct iio_dma_buffer_block *block) _iio_dma_buffer_block_done(block); spin_unlock_irqrestore(&queue->list_lock, flags); - iio_buffer_block_put_atomic(block); + iio_buffer_block_put(block); iio_dma_buffer_queue_wake(queue); } EXPORT_SYMBOL_GPL(iio_dma_buffer_block_done); @@ -249,7 +292,8 @@ void iio_dma_buffer_block_list_abort(struct iio_dma_buffer_queue *queue, list_del(&block->head); block->bytes_used = 0; _iio_dma_buffer_block_done(block); - iio_buffer_block_put_atomic(block); + + iio_buffer_block_put(block); } spin_unlock_irqrestore(&queue->list_lock, flags); @@ -340,8 +384,8 @@ int iio_dma_buffer_request_update(struct iio_buffer *buffer) if (!block) { block = iio_dma_buffer_alloc_block(queue, size); - if (!block) { - ret = -ENOMEM; + if (IS_ERR(block)) { + ret = PTR_ERR(block); goto out_unlock; } queue->fileio.blocks[i] = block; diff --git a/include/linux/iio/buffer-dma.h b/include/linux/iio/buffer-dma.h index 490b93f76fa8..6b3fa7d2124b 100644 --- a/include/linux/iio/buffer-dma.h +++ b/include/linux/iio/buffer-dma.h @@ -8,7 +8,6 @@ #define __INDUSTRIALIO_DMA_BUFFER_H__ #include <linux/list.h> -#include <linux/kref.h> #include <linux/spinlock.h> #include <linux/mutex.h> #include <linux/iio/buffer_impl.h> @@ -16,6 +15,7 @@ struct iio_dma_buffer_queue; struct iio_dma_buffer_ops; struct device; +struct dma_buf; /** * enum iio_block_state - State of a struct iio_dma_buffer_block @@ -39,8 +39,8 @@ enum iio_block_state { * @vaddr: Virutal address of the blocks memory * @phys_addr: Physical address of the blocks memory * @queue: Parent DMA buffer queue - * @kref: kref used to manage the lifetime of block * @state: Current state of the block + * @dmabuf: Underlying DMABUF object */ struct iio_dma_buffer_block { /* May only be accessed by the owner of the block */ @@ -56,13 +56,13 @@ struct iio_dma_buffer_block { size_t size; struct iio_dma_buffer_queue *queue; - /* Must not be accessed outside the core. */ - struct kref kref; /* * Must not be accessed outside the core. Access needs to hold * queue->list_lock if the block is not owned by the core. */ enum iio_block_state state; + + struct dma_buf *dmabuf; }; /**
Enhance the current fileio code by using DMABUF objects instead of custom buffers. This adds more code than it removes, but: - a lot of the complexity can be dropped, e.g. custom kref and iio_buffer_block_put_atomic() are not needed anymore; - it will be much easier to introduce an API to export these DMABUF objects to userspace in a following patch. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- drivers/iio/buffer/industrialio-buffer-dma.c | 192 ++++++++++++------- include/linux/iio/buffer-dma.h | 8 +- 2 files changed, 122 insertions(+), 78 deletions(-)