mbox series

[v3,0/5] QRTR flow control improvements

Message ID 20200107054713.3909260-1-bjorn.andersson@linaro.org
Headers show
Series QRTR flow control improvements | expand

Message

Bjorn Andersson Jan. 7, 2020, 5:47 a.m. UTC 
In order to prevent overconsumption of resources on the remote side QRTR
implements a flow control mechanism.

Move the handling of the incoming confirm_rx to the receiving process to
ensure incoming flow is controlled. Then implement outgoing flow
control, using the recommended algorithm of counting outstanding
non-confirmed messages and blocking when hitting a limit. The last three
patches refactors the node assignment and port lookup, in order to
remove the worker in the receive path.

Bjorn Andersson (5):
  net: qrtr: Move resume-tx transmission to recvmsg
  net: qrtr: Implement outgoing flow control
  net: qrtr: Migrate node lookup tree to spinlock
  net: qrtr: Make qrtr_port_lookup() use RCU
  net: qrtr: Remove receive worker

 net/qrtr/qrtr.c | 314 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 242 insertions(+), 72 deletions(-)

Comments

David Miller Jan. 7, 2020, 9:16 p.m. UTC | #1 
From: Bjorn Andersson <bjorn.andersson@linaro.org>
Date: Mon,  6 Jan 2020 21:47:10 -0800

>  static void __qrtr_node_release(struct kref *kref)
>  {
> +	struct radix_tree_iter iter;
>  	struct qrtr_node *node = container_of(kref, struct qrtr_node, ref);
> +	void __rcu **slot;

Reverse christmas tree here, please.

Thank you.
Chris Lew Jan. 10, 2020, 3:07 a.m. UTC | #2 
Hey Bjorn,

Some minor comments.

On 1/6/2020 9:47 PM, Bjorn Andersson wrote:
> +/**
> + * qrtr_tx_flow_failed() - flag that tx of confirm_rx flagged messages failed
> + * @node:	qrtr_node that the packet is to be send to
> + * @dest_node:	node id of the destination
> + * @dest_port:	port number of the destination
> + *
> + * Signal that the transmission of a message with confirm_rx flag failed. The
> + * flow's "pending" counter will keep incrementing towards QRTR_TX_FLOW_HIGH,
> + * at which point transmission would stall forever waiting for the resume TX
> + * message associated with the dropped confirm_rx message.
> + * Work around this by marking the flow as having a failed transmission and
> + * cause the next transmission attempt to be sent with the confirm_rx.
> + */
> +static void qrtr_tx_flow_failed(struct qrtr_node *node, int dest_node,
> +				int dest_port)
> +{
> +	unsigned long key = (u64)dest_node << 32 | dest_port;
> +	struct qrtr_tx_flow *flow;
> +
> +	flow = radix_tree_lookup(&node->qrtr_tx_flow, key);
> +	if (flow) {
> +		spin_lock_irq(&flow->resume_tx.lock);
> +		flow->tx_failed = 1;
> +		spin_unlock_irq(&flow->resume_tx.lock);
> +	}

Might be good to take qrtr_tx_lock when accessing the qrtr_tx_flow radix 
tree here.

> @@ -408,6 +570,8 @@ int qrtr_endpoint_register(struct qrtr_endpoint *ep, unsigned int nid)
>   	node->nid = QRTR_EP_NID_AUTO;
>   	node->ep = ep;
>   
> +	INIT_RADIX_TREE(&node->qrtr_tx_flow, GFP_KERNEL);
> +

mutex_init(&node->qrtr_tx_lock);

>   	qrtr_node_assign(node, nid);
>   
>   	mutex_lock(&qrtr_node_lock);

Thanks,

Chris

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project