Received-SPF: pass (google.com: domain of linux-kernel+bounces-40933-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <065cbefc-031e-7595-9d48-f869585dfcd3@quicinc.com>
Date: Fri, 26 Jan 2024 16:23:43 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH] qcom: smem: remove hwspinlock from item get routine
Content-Language: en-US
To: Tao Zhang <quic_taozhan@quicinc.com>, <andersson@kernel.org>,
        <konrad.dybcio@linaro.org>
CC: <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <kernel@quicinc.com>
References: <20240102022512.999635-1-quic_taozhan@quicinc.com>
From: Chris Lew <quic_clew@quicinc.com>
In-Reply-To: <20240102022512.999635-1-quic_taozhan@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 1/1/2024 6:25 PM, Tao Zhang wrote:
> During an SSR(Sub-System Restart) process, the remoteproc driver will
> try to read the crash reason from SMEM. The qcom_smem_get() backing such
> operations does however take the hwspinlock (tcsr mutex), which might be
> held by the dying remoteproc.
> 
> The associated timeout on the hwspin_lock_timeout_irqsave() would take
> care of the system not hanging forever, but the get operation will fail,
> unnecessarily delaying the process for the 'HWSPINLOCK_TIMEOUT' duration
> (currently is '1s'), and finally resulting in failure to get crash
> information from SMEM.
> 
> This timeout can be avoided by removing the hwspinlock in the
> qcom_smem_get routine. SMEM ensures that the allocated item will only be
> visible after the new item is safe to use by following a specific order
> of updates.
> 
> In the private partition case, qcom_smem_get_private() will use
> 'offset_free_uncached' as a loop boundary when looking for existing
> allocated items. The corresponding allocation will only update
> offset_free_uncached once the item is fully initialized.
> 
>      hdr->canary = SMEM_PRIVATE_CANARY;
>      hdr->item = cpu_to_le16(item);
>      hdr->size = cpu_to_le32(ALIGN(size, 8));
>      hdr->padding_data = cpu_to_le16(le32_to_cpu(hdr->size) - size);
>      hdr->padding_hdr = 0;
> 
>      wmb();
>      le32_add_cpu(&phdr->offset_free_uncached, alloc_size);
> 
> The global partition is similar but uses the "entry->allocated" variable
> to ensure the item is not visible to qcom_smem_get_global().
> 
> Signed-off-by: Tao Zhang <quic_taozhan@quicinc.com>
> ---

Reviewed-by: Chris Lew <quic_clew@quicinc.com>