NACK/Cmnt: [SRU][N][PATCH v2 0/1] Coresight fails to build on 6.8.0-101 due to 'atomic_t' member accessed as 'int'

[Abdur Rahman]

On 2/24/26 2:33 AM, Noah Wager wrote:
> BugLink:https://bugs.launchpad.net/bugs/2142336
>
> SRU Justification:
>
> [Impact]
>
> When CONFIG_CORESIGHT is enabled, noble 6.8.0-101 fails to build with the following errors:
>
> ```
> drivers/hwtracing/coresight/coresight-catu.c: In function 'catu_enable':
> drivers/hwtracing/coresight/coresight-catu.c:465:27: error: invalid operands to binary == (have 'atomic_t' and 'int')
>    465 |         if (csdev->refcnt == 0) {
>        |             ~~~~~~~~~~~~~ ^~
>        |                  |
>        |                  atomic_t
> drivers/hwtracing/coresight/coresight-catu.c:471:30: error: wrong type argument to increment
>    471 |                 csdev->refcnt++;
>        |                              ^~
> drivers/hwtracing/coresight/coresight-catu.c: In function 'catu_disable':
> drivers/hwtracing/coresight/coresight-catu.c:498:13: error: wrong type argument to decrement
>    498 |         if (--csdev->refcnt == 0) {
>        |             ^~
> ```
>
> These errors are a result of including the following patch in upstream stable
> updates, but missing some dependencies from the original patch set:
>
>    Error description: refcnt accessed as int but defined as atomic_t
>    Break commit: n/linux 0e2198d37507 ("coresight: catu: Introduce refcount and spinlock for enabling/disabling")
>    Missing commit: 4545b38ef004 ("coresight: Remove atomic type from refcnt")
>    Patch series: "coresight: Separate sysfs and Perf usage and some other cleanups"
>                  (https://lists.infradead.org/pipermail/linux-arm-kernel/2024-January/899269.html)
>
> [Fix]
>
> Cherry-pick upstream commit:
> 4545b38ef004 ("coresight: Remove atomic type from refcnt")
> which requires a few context fixups due to missing commits from the original
> patchset. Those missing patches are not required for this commit, so we can
> just fix and apply it alone to reduce the number of changes.
>
> NOTE: These are the missing dependent patches for a clean application:
>
> a0fef3f05cf3 ("coresight: Make language around "activated" sinks consistent")
> a11ebe138b8d ("coresight: Remove ops callback checks")
> 9cae77cf23e3 ("coresight: Move mode to struct coresight_device")
> d5e83f97eb56 ("coresight: Remove the 'enable' field.")
> 1f5149c7751c ("coresight: Move all sysfs code to sysfs file")
>
> These contain pretty significant refactoring and cleanup (although they don't
> alter behaviour) so they were omitted to reduce changes to a stable kernel.
>
> [Test Plan]
>
> Ensure CONFIG_CORESIGHT and CONFIG_CORESIGHT_CATU are enabled, then compile test.
>
> [Where problems could occur]
>
> There is a low chance of regression since this patch is purely cleanup and
> does not modify observable behaviour, as stated by the author in the original
> patch set. Additionally, coresight failed to build before this, so this is
> a guaranteed improvement in that respect. There is a risk of breakage if I
> missed some dependent commits, or if my fixups were incorrect.
>
>
> James Clark (1):
>    coresight: Remove atomic type from refcnt
>
>   drivers/hwtracing/coresight/coresight-core.c  |  9 +++++----
>   drivers/hwtracing/coresight/coresight-etb10.c | 11 +++++-----
>   .../hwtracing/coresight/coresight-tmc-etf.c   | 20 ++++++++++---------
>   .../hwtracing/coresight/coresight-tmc-etr.c   | 13 ++++++------
>   drivers/hwtracing/coresight/coresight-tpiu.c  | 14 +++++++++++--
>   drivers/hwtracing/coresight/ultrasoc-smb.c    |  9 +++++----
>   include/linux/coresight.h                     |  7 +++++--
>   7 files changed, 51 insertions(+), 32 deletions(-)
>
Thanks for the backport effort. +1 on Stefan's comments and I would 
agree that we
can take this fix.

However, there seems to be some minor issues with the patch.

It would be nice to explicitly mention the coresight-tpda.c file in the 
backport message
and why changes aren't applicable to that file.

The reason for NACK are as follows:
In the following hunk:
@@ -391,7 +391,7 @@ int coresight_enable_source(struct coresight_device 
*csdev, enum cs_mode mode, csdev->enable = true; } - 
atomic_inc(&csdev->refcnt); + csdev->refcnt--; return 0; }
atomic_inc is replaced by decrement. This seems wrong.

Since a V3 is required, I would recommend adding the BugLink as well.

Nacked-by: Abdur Rahman <abdur.rahman at canonical.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20260227/e96dfc7a/attachment.html>