Introduce rpmsg_release_tx_buffer API #401
Conversation
arnopo
force-pushed
the
tx_release
branch
2 times, most recently
from
6cc8537 to
d9a5406
Compare
|
@carlocaione , @edmooring could you have a look to the PR? |
| @@ -414,6 +444,32 @@ static int rpmsg_virtio_send_offchannel_nocopy(struct rpmsg_device *rdev, | |||
| return len; | |||
| } | |||
|
|
|||
| static int rpmsg_virtio_release_tx_buffer(struct rpmsg_device *rdev, void *txbuf) | |||
There was a problem hiding this comment.
What happens if buggy code tries to release the same buffer twice? Maybe I'm missing something here.
There was a problem hiding this comment.
That's a good point, I need to add protection by checking that the buffer is not already in the list.
There was a problem hiding this comment.
After more investigations, the detection of a double release is not simple. If it quite simple to test that the buffer is not in the reclaimer list, it is not possible to detect the case if the buffer as been recycled ( so no more in the list) or even already used to send a rpmsg.
I added a note in rpmsg_virtio_release_tx_buffer header:
c172737#diff-145cb24aa08983b6c2538aedc198375a454527d8311596cb87d970b917168971R347
| * | ||
| * Note that the rpmsg virtio is not able to detect if a buffer has already been released. | ||
| * The user must prevent a double release (e.g. by resetting its buffer pointer to zero after | ||
| * the release). |
There was a problem hiding this comment.
This still makes me uncomfortable. If there is a way of preventing users from shooting themselves in the foot, we should provide it.
There was a problem hiding this comment.
As mentioned in a previous comment , seems to me no way to guaranty, for instance in this in following cases:
- the user send the buffer and try to release it after
- the process A release a buffer, the proccess B get it from the recycler, the process A release it again.
This is quite similar to using a pointer after its associated memory has been freed.
There was a problem hiding this comment.
I see your point. In resource-constrained environments, this is about all that can be done.
Add an API to be able to release unused TX buffer that will not be sent. For instance this API can be called in case of error between the buffer reservation and the send to the remote side. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Add short description of the no-copy user interface. Add some sub-chapters to increase the readability. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
As it possible to get the buffer length we need also to retrieve the address associated to the descriptor index. This is need by rpmsg virtio to implement the buffer recycler. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
With the no-copy feature a tx buffer can be get, filled and then sent to the remote side. In Some error cases the application can need to release it instead of sending it to the remote side. As the virtqueue is updated when the buffer it get, it is not possible to manage this use case at virtqueue level. This patchset implements the release based on a buffer recycler. The principle is to store the released buffer in a 'reclaimer' list. On next get pmsg_virtio_get_tx_buffer call the buffer is reused. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Add test to validate the no copy and the associated Tx buffer recycler. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Add an API to be able to release unused TX buffers that will not be used. For example, this API can be called in case of an error between the buffer reservation and the sending to the remote side.
Associated with this API, a mechanism is proposed to temporarily store the virtio buffer that can not be put back into the vrings.
These buffers are stored in a "recycle" table to be reused as a priority on the next
rpmsg_get_tx_payload_buffercall.Note that the recycler is not based on a metal list but on a static array to respect the static memory allocation constraint.