| Message ID | 20220909133938.3518520-9-abel.vesa@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | misc: fastrpc: Add audiopd support | expand |
On 09/09/2022 14:39, Abel Vesa wrote: > If the userspace daemon is killed in the middle of an invoke (e.g. > audiopd listerner invoke), we need to skip the unmapping on device > release, otherwise the DSP will crash. So lets safekeep all the maps > only if there is in invoke interrupted, by attaching them to the channel > context (which is resident until RPMSG driver is removed), and free them > on RPMSG driver remove. > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- > drivers/misc/fastrpc.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index 6b2a552dbdba..bc1e8f003d7a 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -275,6 +275,7 @@ struct fastrpc_channel_ctx { > struct fastrpc_device *secure_fdevice; > struct fastrpc_device *fdevice; > struct fastrpc_buf *remote_heap; > + struct list_head invoke_interrupted_mmaps; > bool secure; > bool unsigned_support; > }; > @@ -1119,6 +1120,8 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, > struct fastrpc_invoke_args *args) > { > struct fastrpc_invoke_ctx *ctx = NULL; > + struct fastrpc_buf *buf, *b; > + > int err = 0; > > if (!fl->sctx) > @@ -1182,6 +1185,13 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, > fastrpc_context_put(ctx); > } > > + if (err == -ERESTARTSYS) { > + list_for_each_entry_safe(buf, b, &fl->mmaps, node) { > + list_del(&buf->node); > + list_add_tail(&buf->node, &fl->cctx->invoke_interrupted_mmaps); > + } > + } > + > if (err) > dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err); > > @@ -2277,6 +2287,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev) > dev_set_drvdata(&rpdev->dev, data); > dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32)); > INIT_LIST_HEAD(&data->users); > + INIT_LIST_HEAD(&data->invoke_interrupted_mmaps); > spin_lock_init(&data->lock); > idr_init(&data->ctx_idr); > data->domain_id = domain_id; > @@ -2301,6 +2312,7 @@ static void fastrpc_notify_users(struct fastrpc_user *user) > static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev) > { > struct fastrpc_channel_ctx *cctx = dev_get_drvdata(&rpdev->dev); > + struct fastrpc_buf *buf, *b; > struct fastrpc_user *user; > unsigned long flags; > > @@ -2315,6 +2327,9 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev) > if (cctx->secure_fdevice) > misc_deregister(&cctx->secure_fdevice->miscdev); > > + list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node) > + list_del(&buf->node); > + When would you free these? looks like we are leaking even after dsp is down.. Should we not do fastrpc_buf_free() here? --srini > if (cctx->remote_heap) > fastrpc_buf_free(cctx->remote_heap); >
On 22-09-16 13:58:35, Srinivas Kandagatla wrote: > > > On 09/09/2022 14:39, Abel Vesa wrote: > > If the userspace daemon is killed in the middle of an invoke (e.g. > > audiopd listerner invoke), we need to skip the unmapping on device > > release, otherwise the DSP will crash. So lets safekeep all the maps > > only if there is in invoke interrupted, by attaching them to the channel > > context (which is resident until RPMSG driver is removed), and free them > > on RPMSG driver remove. > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > > --- > > drivers/misc/fastrpc.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > > index 6b2a552dbdba..bc1e8f003d7a 100644 > > --- a/drivers/misc/fastrpc.c > > +++ b/drivers/misc/fastrpc.c > > @@ -275,6 +275,7 @@ struct fastrpc_channel_ctx { > > struct fastrpc_device *secure_fdevice; > > struct fastrpc_device *fdevice; > > struct fastrpc_buf *remote_heap; > > + struct list_head invoke_interrupted_mmaps; > > bool secure; > > bool unsigned_support; > > }; > > @@ -1119,6 +1120,8 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, > > struct fastrpc_invoke_args *args) > > { > > struct fastrpc_invoke_ctx *ctx = NULL; > > + struct fastrpc_buf *buf, *b; > > + > > int err = 0; > > if (!fl->sctx) > > @@ -1182,6 +1185,13 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, > > fastrpc_context_put(ctx); > > } > > + if (err == -ERESTARTSYS) { > > + list_for_each_entry_safe(buf, b, &fl->mmaps, node) { > > + list_del(&buf->node); > > + list_add_tail(&buf->node, &fl->cctx->invoke_interrupted_mmaps); > > + } > > + } > > + > > if (err) > > dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err); > > @@ -2277,6 +2287,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev) > > dev_set_drvdata(&rpdev->dev, data); > > dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32)); > > INIT_LIST_HEAD(&data->users); > > + INIT_LIST_HEAD(&data->invoke_interrupted_mmaps); > > spin_lock_init(&data->lock); > > idr_init(&data->ctx_idr); > > data->domain_id = domain_id; > > @@ -2301,6 +2312,7 @@ static void fastrpc_notify_users(struct fastrpc_user *user) > > static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev) > > { > > struct fastrpc_channel_ctx *cctx = dev_get_drvdata(&rpdev->dev); > > + struct fastrpc_buf *buf, *b; > > struct fastrpc_user *user; > > unsigned long flags; > > @@ -2315,6 +2327,9 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev) > > if (cctx->secure_fdevice) > > misc_deregister(&cctx->secure_fdevice->miscdev); > > + list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node) > > + list_del(&buf->node); > > + > When would you free these? > looks like we are leaking even after dsp is down.. > Should we not do fastrpc_buf_free() here? Yes, we should. I forgot to add it. Will send a new version. Thanks. > > > > --srini > > > if (cctx->remote_heap) > > fastrpc_buf_free(cctx->remote_heap);
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 6b2a552dbdba..bc1e8f003d7a 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -275,6 +275,7 @@ struct fastrpc_channel_ctx { struct fastrpc_device *secure_fdevice; struct fastrpc_device *fdevice; struct fastrpc_buf *remote_heap; + struct list_head invoke_interrupted_mmaps; bool secure; bool unsigned_support; }; @@ -1119,6 +1120,8 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, struct fastrpc_invoke_args *args) { struct fastrpc_invoke_ctx *ctx = NULL; + struct fastrpc_buf *buf, *b; + int err = 0; if (!fl->sctx) @@ -1182,6 +1185,13 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, fastrpc_context_put(ctx); } + if (err == -ERESTARTSYS) { + list_for_each_entry_safe(buf, b, &fl->mmaps, node) { + list_del(&buf->node); + list_add_tail(&buf->node, &fl->cctx->invoke_interrupted_mmaps); + } + } + if (err) dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err); @@ -2277,6 +2287,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev) dev_set_drvdata(&rpdev->dev, data); dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32)); INIT_LIST_HEAD(&data->users); + INIT_LIST_HEAD(&data->invoke_interrupted_mmaps); spin_lock_init(&data->lock); idr_init(&data->ctx_idr); data->domain_id = domain_id; @@ -2301,6 +2312,7 @@ static void fastrpc_notify_users(struct fastrpc_user *user) static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev) { struct fastrpc_channel_ctx *cctx = dev_get_drvdata(&rpdev->dev); + struct fastrpc_buf *buf, *b; struct fastrpc_user *user; unsigned long flags; @@ -2315,6 +2327,9 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev) if (cctx->secure_fdevice) misc_deregister(&cctx->secure_fdevice->miscdev); + list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node) + list_del(&buf->node); + if (cctx->remote_heap) fastrpc_buf_free(cctx->remote_heap);
If the userspace daemon is killed in the middle of an invoke (e.g. audiopd listerner invoke), we need to skip the unmapping on device release, otherwise the DSP will crash. So lets safekeep all the maps only if there is in invoke interrupted, by attaching them to the channel context (which is resident until RPMSG driver is removed), and free them on RPMSG driver remove. Signed-off-by: Abel Vesa <abel.vesa@linaro.org> --- drivers/misc/fastrpc.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)