On Fri, Aug 16, 2024 at 08:46:25AM +0200, Sebastian Andrzej Siewior wrote:
> On 2024-08-15 21:41:03 [-0300], Luis Claudio R. Goncalves wrote:
> > The functions atc_advance_work() and atc_issue_pending() both have a
> > similar statement
> >
> > return spin_unlock_irqrestore(&atchan->lock, flags);
> >
> > That results in the following errors during the build:
> >
> > drivers/dma/at_hdmac.c: In function ‘atc_advance_work’:
> > ./include/linux/spinlock_rt.h:115:9: error: expected expression before ‘do’
> > 115 | do { \
> > | ^~
> > drivers/dma/at_hdmac.c:487:24: note: in expansion of macro ‘spin_unlock_irqrestore’
> > 487 | return spin_unlock_irqrestore(&atchan->lock, flags);
> > | ^~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/spinlock_rt.h:115:9: error: ‘return’ with a value, in function returning void [-Werror=return-type]
> > 115 | do { \
> > | ^~
> > drivers/dma/at_hdmac.c:487:24: note: in expansion of macro ‘spin_unlock_irqrestore’
> > 487 | return spin_unlock_irqrestore(&atchan->lock, flags);
> > | ^~~~~~~~~~~~~~~~~~~~~~
> >
> > Fix this by splitting the spin_unlock_irqrestore() call and the return
> > statement in both functions.
>
> 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.
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?
Thanks in advance,
Luis
> I don't see anything in v6.1.104. Unlike the recent RiscV fallout, this
> is not RT specific.
>
> Sebastian
>
---end quoted text---
