On 9/19/2023 10:33 PM, Sricharan Ramabadhran wrote:
@@ -122,6 +123,9 @@ static DEFINE_XARRAY_ALLOC(qrtr_ports);
* @qrtr_tx_lock: lock for qrtr_tx_flow inserts
* @rx_queue: receive queue
* @item: list item for broadcast list
+ * @kworker: worker thread for recv work
+ * @task: task to run the worker thread
+ * @read_data: scheduled work for recv work
I think I made these descriptions a bit ambiguous with "recv work".
Since we are only parsing DEL_PROC messages at the moment, the
descriptions should be more accurate on what they are for.
*/
struct qrtr_node {
struct mutex ep_lock;
@@ -134,6 +138,9 @@ struct qrtr_node {
struct sk_buff_head rx_queue;
struct list_head item;
+ struct kthread_worker kworker;
+ struct task_struct *task;
+ struct kthread_work read_data;
I think our own kthread here might have been overkill. I forget why we
needed it instead of using a workqueue.
+ if (cb->type == QRTR_TYPE_DEL_PROC) {
+ /* Free tx flow counters */
+ mutex_lock(&node->qrtr_tx_lock);
+ radix_tree_for_each_slot(slot, &node->qrtr_tx_flow, &iter, 0) {
+ flow = rcu_dereference_raw(*slot);
+ wake_up_interruptible_all(&flow->resume_tx);
+ }
+ mutex_unlock(&node->qrtr_tx_lock);
+
I don't see any other places where we use rcu_dereference_raw for the
flow. Does this need to be updated for the rest of the places we get the
flow?
The same loop is done in qrtr_endpoint_unregister() so maybe we should
look into adding a helper for this logic?
+ /* Translate DEL_PROC to BYE for local enqueue */
+ cb->type = QRTR_TYPE_BYE;
+ pkt = (struct qrtr_ctrl_pkt *)skb->data;
+ memset(pkt, 0, sizeof(*pkt));
+ pkt->cmd = cpu_to_le32(QRTR_TYPE_BYE);
+
Are we relying on the remote to program the destination of this packet
to be the control port?
Thanks,
Chris
