Skip to content

ras: aest: fix duplicate cpu_pm notifier registration - #881

Open
Shiraz Hashim (shashim-quic) wants to merge 1 commit into
qualcomm-linux:qcom-6.18.yfrom
shashim-quic:cr-4618661
Open

ras: aest: fix duplicate cpu_pm notifier registration#881
Shiraz Hashim (shashim-quic) wants to merge 1 commit into
qualcomm-linux:qcom-6.18.yfrom
shashim-quic:cr-4618661

Conversation

@shashim-quic

Copy link
Copy Markdown

Summary

aest_device_probe() in drivers/ras/aest/aest-core.c unconditionally
registers a single, shared static cpu_pm notifier on every probe of the
"AEST" platform driver. On QCS8300/Monaco three AEST platform devices are
created from DT (aest-processor-0, aest-l3-cluster0,
aest-l3-cluster1), so the driver probes three times and tries to
register the same notifier three times. The kernel's
notifier_chain_register() detects the re-add of an already-linked
notifier and fires WARN(1, "notifier callback %ps already registered", ...)
on the 2nd and 3rd probe, producing a call trace on every boot
(non-fatal — boot completes normally, but the WARN is 100% reproducible
and taints the kernel).

Fixes the shared notifier registration by refcounting
aest_cpu_pm_init()/aest_cpu_pm_exit() so it is registered exactly
once regardless of how many AEST devices this target exposes, and
unregistered only once every probed device has been torn down.

Fixes: 0132f7b ("WORKAROUND: Move CPU PM notifier registration to Probe")

Test plan

  • checkpatch.pl --strict: 0 errors, 0 warnings, 0 checks
  • Adversarially reviewed: 2 independent reviewers (Correctness &
    Efficacy; Concurrency/Memory Ordering & Caller Impact), full 2/2
    consensus reached after 4 rounds — see
    debate log
  • Boot QCS8300/Monaco (iq-8275-evk, RB4 DVT) and confirm no
    notifier callback aest_cpu_pm_notify already registered WARN
    appears across the 3 AEST probes

CRs-Fixed: CR4618661

aest_device_probe() unconditionally calls aest_cpu_pm_init(), which
registers the driver's single static cpu_pm notifier_block. On targets
that expose more than one AEST platform device (e.g. QCS8300/Monaco,
which has aest-processor-0, aest-l3-cluster0 and aest-l3-cluster1), the
"AEST" platform driver probes once per device, so the same notifier is
registered multiple times. cpu_pm's notifier_chain_register() detects
the re-add of an already-linked notifier_block and fires WARN(1,
"notifier callback %ps already registered", ...), producing a call
trace on every boot for the 2nd and later probes.

Refcount aest_cpu_pm_init()/aest_cpu_pm_exit() so the notifier is
registered exactly once regardless of how many AEST devices this
target exposes, and unregistered only once every probed device has
been torn down. aest_device_remove() now balances aest_cpu_pm_init()
with aest_cpu_pm_exit() for every device type, and the now-redundant
direct aest_cpu_pm_exit() call in aest_exit() is removed since
platform_driver_unregister() already drives aest_device_remove() for
each bound device.

Fixes: 0132f7b ("WORKAROUND: Move CPU PM notifier registration to Probe")
Signed-off-by: Shiraz Hashim <shiraz.hashim@oss.qualcomm.com>
@qswat-orbit-external

Copy link
Copy Markdown

Merge Check Failed: No CR Numbers Found

Error: No Change Request numbers were found.

Please add Change Request numbers to your pull request description in the format CRs-Fixed: 12345 or link GitHub issues that are associated with Change Requests.

@qlijarvis

Copy link
Copy Markdown

PR #881 — validate-patch

PR: #881

Verdict Issues Detailed Report
0 Full report

Final Summary

  1. Lore link present: No — no lore.kernel.org link found in commit message; validation cannot proceed per skill Step 1
  2. Lore link matches PR commits: N/A — no lore link to compare against
  3. Upstream patch status: Unknown — cannot determine without lore link; Fixes tag references vendor WORKAROUND: commit suggesting this is vendor-specific
  4. PR present in qcom-next/topics: Partial - 1/1 commit(s) only have partial integration evidence
Verdict: ❌ — click to expand

🔍 Patch Validation

PR: #881 - ras: aest: fix duplicate cpu_pm notifier registration
Upstream commit: N/A (no lore.kernel.org link found)
Verdict: ❌ FAIL

Commit Message

Check Status Note
Subject matches upstream N/A No upstream lore link to compare against
Body preserves rationale Clear problem description and fix rationale
Fixes tag present/correct ⚠️ References 0132f7bc9b90 with WORKAROUND: prefix - vendor-specific commit
Authorship preserved N/A No upstream source to verify against
Backport note (if applicable) N/A Not a backport

Diff

File Status Notes
drivers/ras/aest/aest-core.c ⚠️ Cannot verify against upstream - no lore link provided

Issues

  1. Missing lore.kernel.org link: No Link: tag found in the commit message. Per kernel contribution guidelines, patches should reference their upstream posting or indicate their vendor-specific nature with an appropriate prefix.

  2. Missing commit prefix: The commit lacks a prefix (UPSTREAM:, FROMLIST:, BACKPORT:, QCLINUX:, PENDING:, WORKAROUND:). Since the Fixes tag references a WORKAROUND: commit (0132f7bc9b90), this appears to be a fix for vendor-specific code and should carry a QCLINUX: or similar prefix.

  3. Integration presence: Per integration_presence_report.md, this commit shows only "partial" evidence in qcom-next (subject or partial tree evidence found, but full change not verified). This suggests the commit may exist in a different form or is incomplete.

Verdict

Do not merge without correction. This commit requires either:

  • A QCLINUX: prefix if it's fixing vendor-specific AEST code, OR
  • A Link: tag pointing to the upstream lore.kernel.org posting if this fix has been or will be submitted upstream

The partial integration presence and lack of clear provenance make this unsuitable for merge in its current form.

Final Summary

  1. Lore link present: No — no lore.kernel.org link found in commit message; validation cannot proceed per skill Step 1
  2. Lore link matches PR commits: N/A — no lore link to compare against
  3. Upstream patch status: Unknown — cannot determine without lore link; Fixes tag references vendor WORKAROUND: commit suggesting this is vendor-specific
  4. PR present in qcom-next/topics: Partial — per integration_presence_report.md: "partial - subject or partial tree evidence found, but full change was not verified" (0/1 fully present, 1/1 partial)

Deterministic Integration Presence

Integration Presence Report

This report is generated by Jarvis before validate-patch runs.
It is the authoritative source for whether PR changes are already present
in qcom-next or in the kernel topic branches.

Kernel repo: /local/mnt/workspace/sgaud/Qgenie/image_pipeline/kernel
qcom-next ref: 36229361f23e694eb897225a6a859064e11471c5
topics remote: topics -> https://github.com/qualcomm-linux/kernel-topics
topics fetch: fetched

Commit Subject qcom-next topics Final
1/1 [PATCH] ras: aest: fix duplicate cpu_pm notifier registration partial - subject or partial tree evidence found, but full change was not verified missing - no subject, patch-id, or full tree-content match found partial

Final Status

overall_status: PARTIAL
present_commits: 0/1
partial_commits: 1/1
missing_commits: 0/1
topics_checked_for_commits: 1/1
final_summary: PR present in qcom-next/topics: Partial - 1/1 commit(s) only have partial integration evidence

@qlijarvis

Copy link
Copy Markdown

PR #881 — checker-log-analyzer

PR: #881
Checker run: https://github.com/qualcomm-linux/kernel-config/actions/runs/30575343185

Checker Result Summary
Checker Result Summary
checkpatch No style issues
dt-binding-check ⏭️ Skipped (no binding changes)
dtb-check ⏭️ Skipped (no DTS changes)
sparse-check No warnings
check-uapi-headers No UAPI changes
check-patch-compliance Missing subject prefix
tag-check Missing required prefix (mandatory for qcom-6.18.y)

Detailed report: Full report

Checker analysis — click to expand

🤖 CI Checker Analysis (checker-log-analyzer)

PR: #881 - ras: aest: fix duplicate cpu_pm notifier registration
Source: https://github.com/qualcomm-linux/kernel-config/actions/runs/30575343185
Target branch: qcom-6.18.y

Checker Result Summary
checkpatch No style issues
dt-binding-check ⏭️ Skipped (no binding changes)
dtb-check ⏭️ Skipped (no DTS changes)
sparse-check No warnings
check-uapi-headers No UAPI changes
check-patch-compliance Missing subject prefix
tag-check Missing required prefix (mandatory for qcom-6.18.y)

❌ check-patch-compliance

Root cause: Commit subject does not start with a required prefix tag.

Failure details:

Checking commit: ras: aest: fix duplicate cpu_pm notifier registration
Commit summary does not start with a required prefix

Analysis:

The commit subject ras: aest: fix duplicate cpu_pm notifier registration lacks a mandatory prefix tag. The target branch is qcom-6.18.y, which is not qcom-next or qcom-next-staging, so every commit must start with one of these prefixes:

  • FROMLIST: — Patch posted to lore.kernel.org mailing list
  • FROMGIT: — Patch taken from a maintainer git tree
  • UPSTREAM: — Patch merged into Linus's mainline tree
  • BACKPORT: — Upstream patch backported with modifications
  • QCLINUX: — Vendor-only change with no upstream equivalent
  • PENDING: — Work-in-progress, not yet posted upstream
  • WORKAROUND: — Temporary fix not suitable for upstream

The commit message body references Fixes: 0132f7bc9b90 ("WORKAROUND: Move CPU PM notifier registration to Probe"), which suggests this is fixing a previous vendor-specific workaround. Since this appears to be a vendor-only fix for a vendor-specific issue (multiple AEST platform devices on QCS8300/Monaco), the appropriate prefix is likely QCLINUX:.

Fix:

git rebase -i d9ebca924ccc940f0b3b64c702fee8608abba8b7   # mark commit 738e97ee as 'edit'
git commit --amend -m "QCLINUX: ras: aest: fix duplicate cpu_pm notifier registration

aest_device_probe() unconditionally calls aest_cpu_pm_init(), which
registers the driver's single static cpu_pm notifier_block. On targets
that expose more than one AEST platform device (e.g. QCS8300/Monaco,
which has aest-processor-0, aest-l3-cluster0 and aest-l3-cluster1), the
\"AEST\" platform driver probes once per device, so the same notifier is
registered multiple times. cpu_pm's notifier_chain_register() detects
the re-add of an already-linked notifier_block and fires WARN(1,
\"notifier callback %ps already registered\", ...), producing a call
trace on every boot for the 2nd and later probes.

Refcount aest_cpu_pm_init()/aest_cpu_pm_exit() so the notifier is
registered exactly once regardless of how many AEST devices this
target exposes, and unregistered only once every probed device has
been torn down. aest_device_remove() now balances aest_cpu_pm_init()
with aest_cpu_pm_exit() for every device type, and the now-redundant
direct aest_cpu_pm_exit() call in aest_exit() is removed since
platform_driver_unregister() already drives aest_device_remove() for
each bound device.

Fixes: 0132f7bc9b90 (\"WORKAROUND: Move CPU PM notifier registration to Probe\")
Signed-off-by: Shiraz Hashim <shiraz.hashim@oss.qualcomm.com>"
git rebase --continue
git push --force-with-lease

Reproduce locally:

cd /path/to/kernel
./scripts/checkpatch.pl --strict --ignore FILE_PATH_CHANGES --git d9ebca924ccc..738e97eefad8
# Or use the kernel-checkers script:
bash kernel-checkers/check-patch-compliance.sh --kernel-src . \
  --base d9ebca924ccc940f0b3b64c702fee8608abba8b7 \
  --head 738e97eefad8bf9137104b19caf23cda9865f7df

❌ tag-check

Root cause: Subject prefix is mandatory for all branches except qcom-next and qcom-next-staging.

Failure details:

Commit 738e97eefad8 ("ras: aest: fix duplicate cpu_pm notifier registration") targets branch qcom-6.18.y, which requires a subject-line prefix tag. The commit subject does not start with any of the required prefixes.

Fix:

Same as above — add QCLINUX: prefix to the commit subject.


Verdict

1 blocker must be fixed before merge:

The commit subject must be amended to include a valid prefix tag. Since this is a vendor-specific fix for a Qualcomm-specific AEST multi-device scenario (QCS8300/Monaco), the recommended prefix is QCLINUX:.

After adding the prefix, re-push the PR and re-trigger CI. All other checkers passed cleanly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants