Re: [PATCH] qcom: smem: remove hwspinlock from item get routine

[Chris Lew]

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@xxxxxxxxxxx>
> ---

Reviewed-by: Chris Lew <quic_clew@xxxxxxxxxxx>