Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9e8fc471-5ca7-bacb-92c4-ae46e74c3e7c@quicinc.com>
Date:   Tue, 11 Jul 2023 17:58:04 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.2
Subject: Re: [PATCH 2/2] rpmsg: glink: change intent work queue type
Content-Language: en-US
From:   Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
To:     <agross@kernel.org>, <andersson@kernel.org>,
        <konrad.dybcio@linaro.org>, <mathieu.poirier@linaro.org>,
        <linux-arm-msm@vger.kernel.org>,
        <linux-remoteproc@vger.kernel.org>, <linux-kernel@vger.kernel.org>
CC:     <quic_srichara@quicinc.com>, <quic_sjaganat@quicinc.com>,
        <quic_kathirav@quicinc.com>, <quic_anusha@quicinc.com>,
        <quic_varada@quicinc.com>
References: <20230607121731.26958-1-quic_mmanikan@quicinc.com>
 <20230607121731.26958-3-quic_mmanikan@quicinc.com>
In-Reply-To: <20230607121731.26958-3-quic_mmanikan@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 6/7/2023 5:47 PM, Manikanta Mylavarapu wrote:
> QDSP6 will clear heap memory once it's received
> RX_DONE event from APPS. Under heavy cpu load
> intent worker thread not able to get cpu slot
> because it's bound to kernel global work queue.
> Due to this QDSP6 firmware faces OOM and it leads
> to Q6 crash. Changing intent work queue type to
> UNBOUND workqueue ensures intent worker thread
> will be executed as early as possible.
> 
> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
> ---
>   drivers/rpmsg/qcom_glink_native.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index 6f9a439e5046..c3e076bb863f 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -140,6 +140,7 @@ enum {
>    * @liids:	idr of all local intents
>    * @riids:	idr of all remote intents
>    * @intent_work: worker responsible for transmitting rx_done packets
> + * @intent_wq:	work queue of intent_work
>    * @done_intents: list of intents that needs to be announced rx_done
>    * @buf:	receive buffer, for gathering fragments
>    * @buf_offset:	write offset in @buf
> @@ -169,6 +170,7 @@ struct glink_channel {
>   	struct idr liids;
>   	struct idr riids;
>   	struct work_struct intent_work;
> +	struct workqueue_struct *intent_wq;
>   	struct list_head done_intents;
>   
>   	struct glink_core_rx_intent *buf;
> @@ -231,6 +233,14 @@ static struct glink_channel *qcom_glink_alloc_channel(struct qcom_glink *glink,
>   	INIT_LIST_HEAD(&channel->done_intents);
>   	INIT_WORK(&channel->intent_work, qcom_glink_rx_done_work);
>   
> +	channel->intent_wq = alloc_workqueue("intent_wq", WQ_UNBOUND, 1);
> +	if (!channel->intent_wq) {
> +		pr_err("failed to create %s channel intent work queue\n",
> +		       channel->name);
> +		kfree(channel);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
>   	idr_init(&channel->liids);
>   	idr_init(&channel->riids);
>   	kref_init(&channel->refcount);
> @@ -270,6 +280,7 @@ static void qcom_glink_channel_release(struct kref *ref)
>   	idr_destroy(&channel->riids);
>   	spin_unlock_irqrestore(&channel->intent_lock, flags);
>   
> +	destroy_workqueue(channel->intent_wq);
>   	kfree(channel->name);
>   	kfree(channel);
>   }
> @@ -573,7 +584,7 @@ static void qcom_glink_rx_done(struct qcom_glink *glink,
>   	list_add_tail(&intent->node, &channel->done_intents);
>   	spin_unlock(&channel->intent_lock);
>   
> -	schedule_work(&channel->intent_work);
> +	queue_work(channel->intent_wq, &channel->intent_work);
>   }
>   
>   /**

Gentle reminder for review!

Thanks,
Manikanta.