Hi Greg,
Here are two patches that address issues that were noticed by Christian
when reviewing the very similar IIO DMABUF interface (but the FunctionFS
patchset was already merged at that point).
Based on 5.9-rc2.
Cheers,
-Paul
Paul Cercueil (2):
usb: gadget: functionfs: Fix inverted DMA fence direction
usb: gadget: functionfs: Wait for fences before enqueueing DMABUF
drivers/usb/gadget/function/f_fs.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
--
2.43.0
A "read" fence was installed when the DMABUF was to be written to,
and a "write" fence was installed when the DMABUF was to be read from.
Besides, dma_resv_usage_rw() should only be used when waiting for
fences.
Fixes: 7b07a2a7ca02 ("usb: gadget: functionfs: Add DMABUF import interface")
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/usb/gadget/function/f_fs.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index bffbc1dc651f..70e8532127ad 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1578,6 +1578,7 @@ static int ffs_dmabuf_transfer(struct file *file,
struct ffs_dmabuf_priv *priv;
struct ffs_dma_fence *fence;
struct usb_request *usb_req;
+ enum dma_resv_usage resv_dir;
struct dma_buf *dmabuf;
struct ffs_ep *ep;
bool cookie;
@@ -1665,8 +1666,9 @@ static int ffs_dmabuf_transfer(struct file *file,
dma_fence_init(&fence->base, &ffs_dmabuf_fence_ops,
&priv->lock, priv->context, seqno);
- dma_resv_add_fence(dmabuf->resv, &fence->base,
- dma_resv_usage_rw(epfile->in));
+ resv_dir = epfile->in ? DMA_RESV_USAGE_WRITE : DMA_RESV_USAGE_READ;
+
+ dma_resv_add_fence(dmabuf->resv, &fence->base, resv_dir);
dma_resv_unlock(dmabuf->resv);
/* Now that the dma_fence is in place, queue the transfer. */
--
2.43.0
Instead of bailing when fences have already been installed on the
DMABUF, wait for them (with a timeout) when doing a blocking operation.
This fixes the issue where userspace would submit a DMABUF with fences
already installed, with the (correct) expectation that it would just
work.
Fixes: 7b07a2a7ca02 ("usb: gadget: functionfs: Add DMABUF import interface")
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/usb/gadget/function/f_fs.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 70e8532127ad..f855f1fc8e5e 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -46,6 +46,8 @@
#define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
+#define DMABUF_ENQUEUE_TIMEOUT_MS 5000
+
MODULE_IMPORT_NS(DMA_BUF);
/* Reference counter handling */
@@ -1580,9 +1582,11 @@ static int ffs_dmabuf_transfer(struct file *file,
struct usb_request *usb_req;
enum dma_resv_usage resv_dir;
struct dma_buf *dmabuf;
+ unsigned long timeout;
struct ffs_ep *ep;
bool cookie;
u32 seqno;
+ long retl;
int ret;
if (req->flags & ~USB_FFS_DMABUF_TRANSFER_MASK)
@@ -1616,17 +1620,14 @@ static int ffs_dmabuf_transfer(struct file *file,
goto err_attachment_put;
/* Make sure we don't have writers */
- if (!dma_resv_test_signaled(dmabuf->resv, DMA_RESV_USAGE_WRITE)) {
- pr_vdebug("FFS WRITE fence is not signaled\n");
- ret = -EBUSY;
- goto err_resv_unlock;
- }
-
- /* If we're writing to the DMABUF, make sure we don't have readers */
- if (epfile->in &&
- !dma_resv_test_signaled(dmabuf->resv, DMA_RESV_USAGE_READ)) {
- pr_vdebug("FFS READ fence is not signaled\n");
- ret = -EBUSY;
+ timeout = nonblock ? 0 : msecs_to_jiffies(DMABUF_ENQUEUE_TIMEOUT_MS);
+ retl = dma_resv_wait_timeout(dmabuf->resv,
+ dma_resv_usage_rw(epfile->in),
+ true, timeout);
+ if (retl == 0)
+ retl = -EBUSY;
+ if (retl < 0) {
+ ret = (int)retl;
goto err_resv_unlock;
}
--
2.43.0
On Tue, Apr 02, 2024 at 01:09:49PM +0200, Paul Cercueil wrote:
> Hi Greg,
>
> Here are two patches that address issues that were noticed by Christian
> when reviewing the very similar IIO DMABUF interface (but the FunctionFS
> patchset was already merged at that point).
>
> Based on 5.9-rc2.
I think you mean "6.9-rc2", right?
Le mardi 02 avril 2024 à 14:41 +0200, Greg Kroah-Hartman a écrit :
> On Tue, Apr 02, 2024 at 01:09:49PM +0200, Paul Cercueil wrote:
> > Hi Greg,
> >
> > Here are two patches that address issues that were noticed by
> > Christian
> > when reviewing the very similar IIO DMABUF interface (but the
> > FunctionFS
> > patchset was already merged at that point).
> >
> > Based on 5.9-rc2.
>
> I think you mean "6.9-rc2", right?
>
Yes, sorry.
-Paul