On Tue, Jun 07, 2022 at 06:16:42PM -0700, Chris Lew wrote: > In order to reduce the amount of copies in the rpmsg framework, it is > necessary for clients to take brief ownership of the receive buffer. > > Add the capability for clients to notify the rpmsg framework and the > underlying transports when it is going to hold onto a buffer and also > notify when the client is done with the buffer. > > In the .rx_cb of the rpmsg drivers, if they wish to use the received > buffer at a later point, they should return RPMSG_DEFER. Otherwise > returning RPMSG_HANDLED (0) will signal the framework that the client > is done with the resources and can continue with cleanup. > > The clients should check if their rpmsg endpoint supports the rx_done > operation with the new state variable in the rpmsg_endpoint since not > all endpoints will have the ability to support this operation. > > Signed-off-by: Chris Lew <quic_clew@xxxxxxxxxxx> > --- > drivers/rpmsg/rpmsg_core.c | 20 ++++++++++++++++++++ > drivers/rpmsg/rpmsg_internal.h | 1 + > include/linux/rpmsg.h | 24 ++++++++++++++++++++++++ > 3 files changed, 45 insertions(+) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index 290c1f02da10..359be643060f 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -351,6 +351,26 @@ ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept) > } > EXPORT_SYMBOL(rpmsg_get_mtu); > > +/** > + * rpmsg_rx_done() - release resources related to @data from a @rx_cb > + * @ept: the rpmsg endpoint > + * @data: payload from a message > + * > + * Returns 0 on success and an appropriate error value on failure. > + */ > +int rpmsg_rx_done(struct rpmsg_endpoint *ept, void *data) > +{ > + if (WARN_ON(!ept)) > + return -EINVAL; > + if (!ept->ops->rx_done) > + return -ENXIO; > + if (!ept->rx_done) > + return -EINVAL; > + > + return ept->ops->rx_done(ept, data); > +} > +EXPORT_SYMBOL(rpmsg_rx_done); > + > /* > * match a rpmsg channel with a channel info struct. > * this is used to make sure we're not creating rpmsg devices for channels > diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h > index a22cd4abe7d1..99cb86ce638e 100644 > --- a/drivers/rpmsg/rpmsg_internal.h > +++ b/drivers/rpmsg/rpmsg_internal.h > @@ -76,6 +76,7 @@ struct rpmsg_endpoint_ops { > __poll_t (*poll)(struct rpmsg_endpoint *ept, struct file *filp, > poll_table *wait); > ssize_t (*get_mtu)(struct rpmsg_endpoint *ept); > + int (*rx_done)(struct rpmsg_endpoint *ept, void *data); > }; > > struct device *rpmsg_find_device(struct device *parent, > diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h > index 523c98b96cb4..8e34222e8bca 100644 > --- a/include/linux/rpmsg.h > +++ b/include/linux/rpmsg.h > @@ -63,6 +63,18 @@ struct rpmsg_device { > const struct rpmsg_device_ops *ops; > }; > > +/** > + * rpmsg rx callback return definitions > + * @RPMSG_HANDLED: rpmsg user is done processing data, framework can free the > + * resources related to the buffer > + * @RPMSG_DEFER: rpmsg user is not done processing data, framework will hold > + * onto resources related to the buffer until rpmsg_rx_done is > + * called. User should check their endpoint to see if rx_done > + * is a supported operation. > + */ > +#define RPMSG_HANDLED 0 > +#define RPMSG_DEFER 1 > + > typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32); > > /** > @@ -71,6 +83,7 @@ typedef int (*rpmsg_rx_cb_t)(struct rpmsg_device *, void *, int, void *, u32); > * @refcount: when this drops to zero, the ept is deallocated > * @cb: rx callback handler > * @cb_lock: must be taken before accessing/changing @cb > + * @rx_done: if set, rpmsg endpoint supports rpmsg_rx_done > * @addr: local rpmsg address > * @priv: private data for the driver's use > * > @@ -93,6 +106,7 @@ struct rpmsg_endpoint { > struct kref refcount; > rpmsg_rx_cb_t cb; > struct mutex cb_lock; > + bool rx_done; Do you see a scenario where rpmsg_endpoint_ops::rx_done holds a valid pointer but rpmsg_epndpoint::rx_done is set to false? If not please remove. > u32 addr; > void *priv; > > @@ -192,6 +206,8 @@ __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp, > > ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept); > > +int rpmsg_rx_done(struct rpmsg_endpoint *ept, void *data); > + > #else > > static inline int rpmsg_register_device_override(struct rpmsg_device *rpdev, > @@ -316,6 +332,14 @@ static inline ssize_t rpmsg_get_mtu(struct rpmsg_endpoint *ept) > return -ENXIO; > } > > +static inline int rpmsg_rx_done(struct rpmsg_endpoint *ept, void *data) > +{ > + /* This shouldn't be possible */ > + WARN_ON(1); > + > + return -ENXIO; > +} > + > #endif /* IS_ENABLED(CONFIG_RPMSG) */ > > /* use a macro to avoid include chaining to get THIS_MODULE */ > -- > 2.7.4 >