Re: [PATCH v5.10-rt] rt: dma: fix build issue in at_hdmac

Sebastian Andrzej Siewior
2024-08-16T10:28:41Z

On 2024-08-16 06:44:21 [-0300], Luis Claudio R. Goncalves wrote:
> > If I see this right, then this code has been replaced by commit
> > ac803b56860f6 ("dmaengine: at_hdmac: Convert driver to use virt-dma")
> >
> > which has been merged in v6.2-rc1. This has been introduced in commits
> > fcd37565efdaf ("dmaengine: at_hdmac: Fix premature completion of desc in issue_pending")
> > v6.1-rc5
> > c6babed879fbe ("dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all()")
> > v6.1-rc5
> >
> > This means v6.1 is affected and the earlier version got it via the
> > stable queue.
> > This compiles here with and without RT on v6.1 with gcc version 14.2.0.
> > The question would, while it is bad in your case and I don't see. Also
> > if this is visible in your RT queue, it should be visible in the stable
> > queue without -RT, too. And then once all details known I would like a
> > patch that goes upstream and fixes the breakage at its root.
>
> Sebastian, I do believe this problem is unique to v5.10-rt. My bad for not
> stating that clearly on the description. Also, specific to aarch64 AT91-based
> boards as AT_HDMAC requires ARCH_AT91 to build.

It is also part of one of ARM's defconfig which I tested with. So this
is not so unique.

> The definition of spin_unlock_irqrestore() at include/linux/spinlock_rt.h
> is slightly different between v5.10-rt and v5.15-rt:
>
> $ git diff origin/v5.10-rt origin/v5.15-rt include/linux/spinlock_rt.h
> ...
>
> -#define spin_unlock_irqrestore(lock, flags) \
> - do { \
> - typecheck(unsigned long, flags); \
> - (void) flags; \
> - spin_unlock(lock); \
> - } while (0)
> +static __always_inline void spin_unlock_irqrestore(spinlock_t *lock,
> + unsigned long flags)
> +{
> + rt_spin_unlock(lock);
> +}
>
> ...
>
> And that seems to be why the problem pops up v5.10 and not on newer RT
> versions.
>
> I see that there was a revamp of the rtmutex code between v5.10-rt and
> v5.15-rt, but given the advanced stage of v5.10-rt in its lifetime it
> sounded more reasonable to simply fix the symptom.
>
> Do you have any suggestion on how to proceed?

I would start by explaining that this only affects <= v5.10 because of
the different spin_unlock_irqrestore() implementation (macro vs
function) and the compiler does not complain about the latter.
This would also make clear why it affects certain RT versions and why
mainline is not affected in general. Also a reference to why this is an
issue now and not earlier.
This information looks more important to me than one line of compiler's
complain/ warning.
That said, with this additional information I don't mind getting this
into the affected kernels.

> Thanks in advance,
> Luis

Sebastian