Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: address "unknown TX stage" errors #372

Open
wants to merge 3 commits into
base: development
Choose a base branch
from
Open

RFC: address "unknown TX stage" errors #372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
54c660e
list.h: introduce list_next_entry_or_null
redlicha Nov 11, 2024
4798d83
xio_tcp_datapath: don't use list_first_entry_or_null on list entries
redlicha Nov 11, 2024
426eafa
Remove xio_tcp_transport.tx_ready_tasks_num
redlicha Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/usr/linux/list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,15 @@ static inline void list_splice_tail_init(struct list_head *list,
#define list_next_entry(pos, member) \
list_entry((pos)->member.next, typeof(*(pos)), member)

/**
* list_next_entry_or_null - get the next element in list unless it's the head
* @head: the list head
* @pos: cursor: pointer to the current entry
* @member: the name of the list_head within the entry.
*/
#define list_next_entry_or_null(head, pos, member) \
(((pos)->member.next == head) ? NULL : list_next_entry(pos, member))

/**
* list_prev_entry - get the prev element in list
* @pos: the type * to cursor
Expand Down Expand Up @@ -772,4 +781,3 @@ static inline void hlist_move_list(struct hlist_head *old,


#endif

49 changes: 20 additions & 29 deletions src/usr/transport/tcp/xio_tcp_datapath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@ static void xio_tcp_tx_completion_handler(int actual_timeout_ms, void *xio_task)
&tcp_hndl->disconnect_event);
} else {
xio_context_disable_event(&tcp_hndl->disconnect_event);
if (tcp_hndl->tx_ready_tasks_num)
if (!list_empty(&tcp_hndl->tx_ready_list))
xio_tcp_xmit(tcp_hndl);
}
}
Expand Down Expand Up @@ -1093,7 +1093,7 @@ int xio_tcp_xmit(struct xio_tcp_transport *tcp_hndl)
unsigned int iov_len;
uint64_t bytes_sent;

if (tcp_hndl->tx_ready_tasks_num == 0 ||
if (list_empty(&tcp_hndl->tx_ready_list) ||
tcp_hndl->tx_comp_cnt > COMPLETION_BATCH_MAX ||
tcp_hndl->state != XIO_TRANSPORT_STATE_CONNECTED) {
xio_set_error(XIO_EAGAIN);
Expand All @@ -1104,11 +1104,11 @@ int xio_tcp_xmit(struct xio_tcp_transport *tcp_hndl)
tasks_list_entry);

/* if "ready to send queue" is not empty */
while (likely(tcp_hndl->tx_ready_tasks_num &&
while (likely(!list_empty(&tcp_hndl->tx_ready_list) &&
(tcp_hndl->tx_comp_cnt < COMPLETION_BATCH_MAX))) {
next_task = list_first_entry_or_null(&task->tasks_list_entry,
struct xio_task,
tasks_list_entry);
next_task = list_next_entry_or_null(&tcp_hndl->tx_ready_list,
task,
tasks_list_entry);
next_tcp_task = next_task ?
(struct xio_tcp_task *)next_task->dd_data : NULL;

Expand Down Expand Up @@ -1138,7 +1138,6 @@ int xio_tcp_xmit(struct xio_tcp_transport *tcp_hndl)

++batch_count;
if (batch_count != batch_nr &&
batch_count != tcp_hndl->tx_ready_tasks_num &&
next_task &&
next_tcp_task->txd.stage
<= XIO_TCP_TX_IN_SEND_CTL) {
Expand Down Expand Up @@ -1173,10 +1172,9 @@ int xio_tcp_xmit(struct xio_tcp_transport *tcp_hndl)
tcp_task = (struct xio_tcp_task *)task->dd_data;
tcp_task->txd.stage = XIO_TCP_TX_IN_SEND_DATA;
tcp_task->txd.ctl_msg_len = 0;
task = list_first_entry_or_null(
&task->tasks_list_entry,
struct xio_task,
tasks_list_entry);
task = list_next_entry_or_null(&tcp_hndl->tx_ready_list,
task,
tasks_list_entry);
}
if (tcp_hndl->tmp_work.msg.msg_iovlen) {
tcp_task = (struct xio_tcp_task *)task->dd_data;
Expand Down Expand Up @@ -1239,7 +1237,6 @@ int xio_tcp_xmit(struct xio_tcp_transport *tcp_hndl)

++batch_count;
if (batch_count != batch_nr &&
batch_count != tcp_hndl->tx_ready_tasks_num &&
next_task &&
(next_tcp_task->txd.stage ==
XIO_TCP_TX_IN_SEND_DATA) &&
Expand Down Expand Up @@ -1274,8 +1271,6 @@ int xio_tcp_xmit(struct xio_tcp_transport *tcp_hndl)
iov_len -= tcp_task->txd.msg.msg_iovlen;
bytes_sent -= tcp_task->txd.tot_iov_byte_len;

tcp_hndl->tx_ready_tasks_num--;

list_move_tail(&task->tasks_list_entry,
&tcp_hndl->in_flight_list);

Expand Down Expand Up @@ -1605,8 +1600,6 @@ static int xio_tcp_send_req(struct xio_tcp_transport *tcp_hndl,
else
list_move_tail(&task->tasks_list_entry, &tcp_hndl->tx_ready_list);

tcp_hndl->tx_ready_tasks_num++;

retval = xio_tcp_xmit(tcp_hndl);
if (retval) {
if (xio_errno() != XIO_EAGAIN) {
Expand Down Expand Up @@ -1991,8 +1984,6 @@ static int xio_tcp_send_rsp(struct xio_tcp_transport *tcp_hndl,
else
list_move_tail(&task->tasks_list_entry, &tcp_hndl->tx_ready_list);

tcp_hndl->tx_ready_tasks_num++;

retval = xio_tcp_xmit(tcp_hndl);
if (retval) {
/* no need xio_get_last_error here */
Expand Down Expand Up @@ -3055,9 +3046,9 @@ int xio_tcp_rx_data_handler(struct xio_tcp_transport *tcp_hndl, int batch_nr)
if (tcp_task->rxd.stage != XIO_TCP_RX_IO_DATA)
break;

next_task = list_first_entry_or_null(
&task->tasks_list_entry,
struct xio_task, tasks_list_entry);
next_task = list_next_entry_or_null(&tcp_hndl->rx_list,
task,
tasks_list_entry);
next_tcp_task = next_task ? (struct xio_tcp_task *)
next_task->dd_data : NULL;
next_rxd_work = (next_tcp_task &&
Expand Down Expand Up @@ -3120,9 +3111,9 @@ int xio_tcp_rx_data_handler(struct xio_tcp_transport *tcp_hndl, int batch_nr)
iov_len -= rxd_work->msg.msg_iovlen;
bytes_recv -= rxd_work->tot_iov_byte_len;

task = list_first_entry(&task->tasks_list_entry,
struct xio_task,
tasks_list_entry);
task = list_next_entry_or_null(&tcp_hndl->rx_list,
task,
tasks_list_entry);
}
if (tcp_hndl->tmp_work.msg.msg_iovlen) {
tcp_task = (struct xio_tcp_task *)task->dd_data;
Expand Down Expand Up @@ -3203,7 +3194,7 @@ int xio_tcp_rx_data_handler(struct xio_tcp_transport *tcp_hndl, int batch_nr)
tasks_list_entry);
}

if (tcp_hndl->tx_ready_tasks_num) {
if (!list_empty(&tcp_hndl->tx_ready_list)) {
retval = xio_tcp_xmit(tcp_hndl);
if (retval < 0) {
if (xio_errno() != XIO_EAGAIN) {
Expand Down Expand Up @@ -3361,8 +3352,9 @@ int xio_tcp_rx_ctl_handler(struct xio_tcp_transport *tcp_hndl, int batch_nr)
if (tcp_task->rxd.tot_iov_byte_len)
tcp_hndl->io_waiting_tasks++;

task = list_first_entry(&task->tasks_list_entry,
struct xio_task, tasks_list_entry);
task = list_next_entry_or_null(&tcp_hndl->rx_list,
task,
tasks_list_entry);
}

if (count == 0)
Expand All @@ -3373,7 +3365,7 @@ int xio_tcp_rx_ctl_handler(struct xio_tcp_transport *tcp_hndl, int batch_nr)
return retval;
count = retval;

if (tcp_hndl->tx_ready_tasks_num) {
if (!list_empty(&tcp_hndl->tx_ready_list)) {
retval = xio_tcp_xmit(tcp_hndl);
if (retval < 0) {
if (xio_errno() != XIO_EAGAIN) {
Expand Down Expand Up @@ -3429,4 +3421,3 @@ int xio_tcp_poll(struct xio_transport_base *transport,

return nr_comp;
}

4 changes: 1 addition & 3 deletions src/usr/transport/tcp/xio_tcp_management.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,6 @@ static int xio_tcp_flush_all_tasks(struct xio_tcp_transport *tcp_hndl)
xio_tasks_list_flush(&tcp_hndl->rx_list);
}

tcp_hndl->tx_ready_tasks_num = 0;

return 0;
}

Expand Down Expand Up @@ -907,7 +905,7 @@ struct xio_tcp_transport *xio_tcp_transport_create(
tcp_hndl->tmp_rx_buf_cur = NULL;
tcp_hndl->tmp_rx_buf_len = 0;

tcp_hndl->tx_ready_tasks_num = 0;
tcp_hndl->_pad = 0;
tcp_hndl->tx_comp_cnt = 0;

memset(&tcp_hndl->tmp_work, 0, sizeof(struct xio_tcp_work_req));
Expand Down
4 changes: 2 additions & 2 deletions src/usr/transport/tcp/xio_tcp_transport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ struct xio_tcp_transport {

/* tx parameters */
size_t max_inline_buf_sz;

int tx_ready_tasks_num;
/* this used to be `tx_ready_tasks_num` - keeping it to satisfy `-Werror=padded` */
int _pad;

uint16_t tx_comp_cnt;

Expand Down