diff mbox series

[V1,2/6] rpmsg: glink: Deny intent request if reusable intent fits

Message ID 1596086296-28529-3-git-send-email-deesin@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series Glink native fixes upstreaming | expand

Commit Message

Deepak Kumar Singh July 30, 2020, 5:18 a.m. UTC
From: Chris Lew <clew@codeaurora.org>

In high traffic scenarios a remote may request extra intents to send
data faster. If the work thread that handles these intent requests is
starved of cpu time, then these requests can build up. Some remote
procs may not be able to handle this burst of built up intent requests.

In order to prevent intent build up, deny intent requests that can be
fulfilled by default intents that are reusable.

Signed-off-by: Chris Lew <clew@codeaurora.org>
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
---
 drivers/rpmsg/qcom_glink_native.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Bjorn Andersson Oct. 15, 2021, 3:14 p.m. UTC | #1
On Thu 30 Jul 00:18 CDT 2020, Deepak Kumar Singh wrote:

> From: Chris Lew <clew@codeaurora.org>
> 
> In high traffic scenarios a remote may request extra intents to send
> data faster. If the work thread that handles these intent requests is
> starved of cpu time, then these requests can build up. Some remote
> procs may not be able to handle this burst of built up intent requests.
> 
> In order to prevent intent build up, deny intent requests that can be
> fulfilled by default intents that are reusable.
> 
> Signed-off-by: Chris Lew <clew@codeaurora.org>
> Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
> ---
>  drivers/rpmsg/qcom_glink_native.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 2668c66..df3c608 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -734,9 +734,11 @@ static void qcom_glink_handle_rx_done(struct qcom_glink *glink,
>  static void qcom_glink_handle_intent_req(struct qcom_glink *glink,
>  					 u32 cid, size_t size)
>  {
> -	struct glink_core_rx_intent *intent;
> +	struct glink_core_rx_intent *intent = NULL;
> +	struct glink_core_rx_intent *tmp;
>  	struct glink_channel *channel;
>  	unsigned long flags;
> +	int iid;
>  
>  	spin_lock_irqsave(&glink->idr_lock, flags);
>  	channel = idr_find(&glink->rcids, cid);
> @@ -747,6 +749,19 @@ static void qcom_glink_handle_intent_req(struct qcom_glink *glink,
>  		return;
>  	}
>  
> +	spin_lock_irqsave(&channel->intent_lock, flags);
> +	idr_for_each_entry(&channel->liids, tmp, iid) {
> +		if (tmp->size >= size && tmp->reuse) {
> +			intent = tmp;
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&channel->intent_lock, flags);
> +	if (intent) {
> +		qcom_glink_send_intent_req_ack(glink, channel, !!intent);

This looks reasonable, but you know that !!intent is true in this block,
and looking more at it I think the end result would look better as:

	/* Try to find a reusable intent to fit the request */
	spin_lock()
	intent = findit();
	spin_unlock()

	/* Allocate a new intent if none was found */
	if (!intent) {
		intent = qcom_glink_alloc_intent(glink, channel, size, false);
		if (intent)
			qcom_glink_advertise_intent(glink, channel, intent);
	}

	qcom_glink_send_intent_req_ack(glink, channel, !!intent);

Regards,
Bjorn

> +		return;
> +	}
> +
>  	intent = qcom_glink_alloc_intent(glink, channel, size, false);
>  	if (intent)
>  		qcom_glink_advertise_intent(glink, channel, intent);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
diff mbox series

Patch

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 2668c66..df3c608 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -734,9 +734,11 @@  static void qcom_glink_handle_rx_done(struct qcom_glink *glink,
 static void qcom_glink_handle_intent_req(struct qcom_glink *glink,
 					 u32 cid, size_t size)
 {
-	struct glink_core_rx_intent *intent;
+	struct glink_core_rx_intent *intent = NULL;
+	struct glink_core_rx_intent *tmp;
 	struct glink_channel *channel;
 	unsigned long flags;
+	int iid;
 
 	spin_lock_irqsave(&glink->idr_lock, flags);
 	channel = idr_find(&glink->rcids, cid);
@@ -747,6 +749,19 @@  static void qcom_glink_handle_intent_req(struct qcom_glink *glink,
 		return;
 	}
 
+	spin_lock_irqsave(&channel->intent_lock, flags);
+	idr_for_each_entry(&channel->liids, tmp, iid) {
+		if (tmp->size >= size && tmp->reuse) {
+			intent = tmp;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&channel->intent_lock, flags);
+	if (intent) {
+		qcom_glink_send_intent_req_ack(glink, channel, !!intent);
+		return;
+	}
+
 	intent = qcom_glink_alloc_intent(glink, channel, size, false);
 	if (intent)
 		qcom_glink_advertise_intent(glink, channel, intent);