nuttx/nuttx-mirror
1
2
Fork 0
mirror of https://github.com/apache/nuttx.git synced 2025-01-13 13:18:50 +08:00
Code Issues Projects Releases Packages Wiki Activity Actions

tcp_send_buffered.c: fix iob allocation deadlock

Browse source 
Ensure to put the wrb back onto the write_q when blocking
on iob allocation. Otherwise, it can deadlock with other
threads doing the same thing.
This commit is contained in:
YAMAMOTO Takashi 2021-07-20 09:10:43 +09:00 committed by Xiang Xiao
parent 5b1f2dff5f
commit 98ec46d726
1 changed files with 92 additions and 77 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

169
net/tcp/tcp_send_buffered.c
View file

@ -1129,69 +1129,73 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
}
#endif /* CONFIG_NET_SEND_BUFSIZE */
/* Allocate a write buffer. Careful, the network will be momentarily
* unlocked here.
*/
/* Try to coalesce into the last wrb.
*
* But only when it might yield larger segments.
* (REVISIT: It might make sense to lift this condition.
* IOB boundaries and segment boundaries usually do not match.
* It makes sense to save the number of IOBs.)
*
* Also, for simplicity, do it only when we haven't sent anything
* from the the wrb yet.
*/
max_wrb_size = tcp_max_wrb_size(conn);
wrb = (FAR struct tcp_wrbuffer_s *)sq_tail(&conn->write_q);
if (wrb != NULL && TCP_WBSENT(wrb) == 0 && TCP_WBNRTX(wrb) == 0 &&
TCP_WBPKTLEN(wrb) < max_wrb_size &&
(TCP_WBPKTLEN(wrb) % conn->mss) != 0)
while (true)
{
wrb = (FAR struct tcp_wrbuffer_s *)sq_remlast(&conn->write_q);
ninfo("coalesce %zu bytes to wrb %p (%" PRIu16 ")\n", len, wrb,
TCP_WBPKTLEN(wrb));
DEBUGASSERT(TCP_WBPKTLEN(wrb) > 0);
}
else if (nonblock)
{
wrb = tcp_wrbuffer_tryalloc();
ninfo("new wrb %p (non blocking)\n", wrb);
}
else
{
wrb = tcp_wrbuffer_alloc();
ninfo("new wrb %p\n", wrb);
}
struct iob_s *iob;
if (wrb == NULL)
{
/* A buffer allocation error occurred */
/* Allocate a write buffer. Careful, the network will be
* momentarily unlocked here.
*/
nerr("ERROR: Failed to allocate write buffer\n");
ret = nonblock ? -EAGAIN : -ENOMEM;
goto errout_with_lock;
}
/* Try to coalesce into the last wrb.
*
* But only when it might yield larger segments.
* (REVISIT: It might make sense to lift this condition.
* IOB boundaries and segment boundaries usually do not match.
* It makes sense to save the number of IOBs.)
*
* Also, for simplicity, do it only when we haven't sent anything
* from the the wrb yet.
*/
/* Initialize the write buffer */
max_wrb_size = tcp_max_wrb_size(conn);
wrb = (FAR struct tcp_wrbuffer_s *)sq_tail(&conn->write_q);
if (wrb != NULL && TCP_WBSENT(wrb) == 0 && TCP_WBNRTX(wrb) == 0 &&
TCP_WBPKTLEN(wrb) < max_wrb_size &&
(TCP_WBPKTLEN(wrb) % conn->mss) != 0)
{
wrb = (FAR struct tcp_wrbuffer_s *)sq_remlast(&conn->write_q);
ninfo("coalesce %zu bytes to wrb %p (%" PRIu16 ")\n", len, wrb,
TCP_WBPKTLEN(wrb));
DEBUGASSERT(TCP_WBPKTLEN(wrb) > 0);
}
else if (nonblock)
{
wrb = tcp_wrbuffer_tryalloc();
ninfo("new wrb %p (non blocking)\n", wrb);
DEBUGASSERT(TCP_WBPKTLEN(wrb) == 0);
}
else
{
wrb = tcp_wrbuffer_alloc();
ninfo("new wrb %p\n", wrb);
DEBUGASSERT(TCP_WBPKTLEN(wrb) == 0);
}
TCP_WBSEQNO(wrb) = (unsigned)-1;
TCP_WBNRTX(wrb) = 0;
if (wrb == NULL)
{
/* A buffer allocation error occurred */
off = TCP_WBPKTLEN(wrb);
if (off + chunk_len > max_wrb_size)
{
chunk_len = max_wrb_size - off;
}
nerr("ERROR: Failed to allocate write buffer\n");
ret = nonblock ? -EAGAIN : -ENOMEM;
goto errout_with_lock;
}
/* Copy the user data into the write buffer. We cannot wait for
* buffer space if the socket was opened non-blocking.
*/
/* Initialize the write buffer */
TCP_WBSEQNO(wrb) = (unsigned)-1;
TCP_WBNRTX(wrb) = 0;
off = TCP_WBPKTLEN(wrb);
if (off + chunk_len > max_wrb_size)
{
chunk_len = max_wrb_size - off;
}
/* Copy the user data into the write buffer. We cannot wait for
* buffer space.
*/
if (nonblock)
{
/* The return value from TCP_WBTRYCOPYIN is either OK or
* -ENOMEM if less than the entire data chunk could be allocated.
* If -ENOMEM is returned, check if at least a part of the data
@ -1220,34 +1224,48 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
}
else
{
nerr("ERROR: Failed to add data to the I/O chain\n");
ret = -EWOULDBLOCK;
goto errout_with_wrb;
chunk_result = 0;
}
}
else
{
DEBUGASSERT(chunk_result == chunk_len);
}
}
else
{
unsigned int count;
int blresult;
/* iob_copyin might wait for buffers to be freed, but if network is
* locked this might never happen, since network driver is also
* locked, therefore we need to break the lock
if (chunk_result > 0)
{
break;
}
/* release wrb */
if (TCP_WBPKTLEN(wrb) > 0)
{
DEBUGASSERT(TCP_WBSENT(wrb) == 0);
DEBUGASSERT(TCP_WBPKTLEN(wrb) > 0);
sq_addlast(&wrb->wb_node, &conn->write_q);
}
else
{
tcp_wrbuffer_release(wrb);
}
if (nonblock)
{
nerr("ERROR: Failed to add data to the I/O chain\n");
ret = -EAGAIN;
goto errout_with_lock;
}
/* Wait for at least one IOB getting available.
*
* Note: net_ioballoc releases the network lock when blocking.
* It allows our write_q being drained in the meantime. Otherwise,
* we risk a deadlock with other threads competing on IOBs.
*/
blresult = net_breaklock(&count);
ninfo("starting copyin to wrb %p\n", wrb);
chunk_result = TCP_WBCOPYIN(wrb, cp, chunk_len, off);
ninfo("finished copyin to wrb %p\n", wrb);
if (blresult >= 0)
{
net_restorelock(count);
}
iob = net_ioballoc(true, IOBUSER_NET_TCP_WRITEBUFFER);
iob_free_chain(iob, IOBUSER_NET_TCP_WRITEBUFFER);
}
/* Dump I/O buffer chain */
@ -1311,9 +1329,6 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
return result;
errout_with_wrb:
tcp_wrbuffer_release(wrb);
errout_with_lock:
net_unlock();