Skip to content

Update scsi_bsg_mpi3mr.h#48

Open
ayukum-hyd wants to merge 1 commit intomainfrom
ayukum-hyd-patch-12
Open

Update scsi_bsg_mpi3mr.h#48
ayukum-hyd wants to merge 1 commit intomainfrom
ayukum-hyd-patch-12

Conversation

@ayukum-hyd
Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: ayukum-hyd <ayukum@qti.qualcomm.com>
@rahujosh
Copy link
Copy Markdown

PR #48 — validate-patch

PR: #48

Verdict Issues Detailed Report
5 Full report
Verdict: ❌ — click to expand

🔍 Patch Validation

PR: Update scsi_bsg_mpi3mr.h — PR #48 (qualcomm-linux-stg/kernel)
Upstream commit: N/A — no upstream reference provided; change appears to be a local/vendor-only deletion with no corresponding mainline commit cited
Verdict: ❌ FAIL

Commit Message

Check Status Note
Subject matches upstream Subject is "Update scsi_bsg_mpi3mr.h" — a generic, non-descriptive title with no upstream equivalent found; does not follow kernel convention (subsystem: short imperative description)
Body preserves rationale Commit body is completely absent — no explanation of why struct mpi3_driver_info_layout and its use in mpi3mr_bsg_in_adpinfo are being removed
Fixes tag present/correct No Fixes: tag; if this is a bug fix or ABI correction, the tag is mandatory
Authorship preserved ⚠️ Author is ayukum-hyd <ayukum@qti.qualcomm.com> — a Qualcomm-internal alias/handle; no From: or Co-developed-by: preserving an upstream author, and the email domain is @qti.qualcomm.com (internal), not a public upstream identity
Backport note (if applicable) No [ upstream commit <sha> ] note; if this is a backport or vendor-local change derived from upstream, the lineage must be stated

Diff

File Status Notes
include/uapi/scsi/scsi_bsg_mpi3mr.h ⚠️ Deletes struct mpi3_driver_info_layout (9 lines) and its embedded use in struct mpi3mr_bsg_in_adpinfo — structurally coherent, but no upstream commit or rationale is cited to justify removing a UAPI-exported struct; this is a UAPI ABI-breaking change with zero justification in the commit message

Issues

  • Missing commit body: The commit has no description at all. Removing a uapi struct is a significant, potentially ABI-breaking change — the rationale (e.g., "struct was never used by userspace", "replaced by X", "fixes ABI mismatch") must be documented.
  • Non-descriptive subject line: "Update scsi_bsg_mpi3mr.h" does not follow Linux kernel commit message conventions. It should be something like scsi: mpi3mr: remove unused mpi3_driver_info_layout from UAPI header.
  • No upstream traceability: There is no Link:, Fixes:, or [ upstream commit ] tag. It is impossible to verify whether this deletion is aligned with mainline, is a vendor-local decision, or is a regression.
  • Author identity: The author ayukum-hyd is an internal Qualcomm handle. Upstream submissions require a real name and a publicly reachable email address per Documentation/process/submitting-patches.rst.
  • UAPI impact undocumented: include/uapi/ headers are part of the stable kernel–userspace ABI. Removing struct mpi3_driver_info_layout without a Fixes: tag or ABI-break justification is a red flag for any stable or vendor tree.

Verdict

Reject: the commit message is entirely missing a body and rationale, the subject is non-descriptive, there is no upstream traceability for a UAPI-breaking struct removal, and the author identity does not meet upstream kernel standards — all must be corrected before this can be merged.

@rahujosh
Copy link
Copy Markdown

PR #48 — checker-log-analyzer

PR: #48
Checker run: https://github.com/qualcomm-linux-stg/kernel-config-test/actions/runs/23713793043

Checker Result Summary
Checker Result Summary
checkpatch ⏭️ N/A No log present; not triggered for this run
dt-binding-check ⏭️ N/A No log present; not triggered for this run
dtb-check ✅ Pass "No changes in Devicetree" — skipped cleanly
sparse-check ⏭️ N/A No log present; not triggered for this run
check-uapi-headers ❌ Fail ABI-breaking deletion in include/uapi/scsi/scsi_bsg_mpi3mr.h
check-patch-compliance ⏭️ N/A No log present; not triggered for this run
tag-check ⏭️ N/A No log present; not triggered for this run
qcom-next-check ⏭️ N/A No log present; not triggered for this run

Detailed report: Full report

Checker analysis — click to expand

🤖 CI Checker Analysis (checker-log-analyzer)

PR: #48 — "Update scsi_bsg_mpi3mr.h" (ayukum-hyd)
Source: https://github.com/qualcomm-linux-stg/kernel-config-test/actions/runs/23713793043
Overall CI status:failure (reported back to commit 1d53e785568005804ccff3500117c9758bae78d9)

Checker Result Summary
checkpatch ⏭️ N/A No log present; not triggered for this run
dt-binding-check ⏭️ N/A No log present; not triggered for this run
dtb-check ✅ Pass "No changes in Devicetree" — skipped cleanly
sparse-check ⏭️ N/A No log present; not triggered for this run
check-uapi-headers ❌ Fail ABI-breaking deletion in include/uapi/scsi/scsi_bsg_mpi3mr.h
check-patch-compliance ⏭️ N/A No log present; not triggered for this run
tag-check ⏭️ N/A No log present; not triggered for this run
qcom-next-check ⏭️ N/A No log present; not triggered for this run

❌ check-uapi-headers

Root cause: The PR deletes struct mpi3_driver_info_layout and its embedded member driver_info from include/uapi/scsi/scsi_bsg_mpi3mr.h, shrinking struct mpi3mr_bsg_in_adpinfo from 1344 bits (168 bytes) to 384 bits (48 bytes) — a backwards-incompatible ABI change in a user-facing UAPI header.

Failure details:

==== ABI differences detected in include/scsi/scsi_bsg_mpi3mr.h
     from 9448598b22c5 -> 1d53e7855680 ====
    [C] 'struct mpi3mr_bsg_in_adpinfo' changed:
      type size changed from 1344 to 384 (in bits)
      1 data member deletion:
        'mpi3_driver_info_layout driver_info', at offset 384 (in bits)
        at scsi_bsg_mpi3mr.h:146:1

error - 1/941 UAPI headers compatible with arm64 appear _not_ to be
        backwards compatible

What the patch does:

  • Removes the entire struct mpi3_driver_info_layout definition (11 lines).
  • Removes the struct mpi3_driver_info_layout driver_info member from struct mpi3mr_bsg_in_adpinfo.
  • This is a UAPI ABI break: any userspace binary that was compiled against the old header and reads/writes driver_info will silently access wrong memory or crash.

Fix options:

  1. Do not delete — if the struct is still used by any in-tree or out-of-tree userspace (e.g. mpi3mrctl), keep it and mark it deprecated with a comment instead.
  2. Deprecate, don't remove — add __attribute__((deprecated)) or a /* deprecated */ comment and keep the struct for at least one kernel release cycle.
  3. If removal is intentional — it must go through the proper UAPI deprecation process: announce on the mailing list, give userspace time to adapt, and coordinate with the scsi maintainer. The commit message must explain why the ABI break is justified.

Reproduce locally:

./scripts/check-uapi.sh -b 1d53e785568005804ccff3500117c9758bae78d9 \
                        -p 9448598b22c50c8a5bb77a9103e2d49f134c9578 \
                        -l uapi_errors.log

Verdict

1 blocker to fix: check-uapi-headers fails because the PR makes a backwards-incompatible deletion from a UAPI header (struct mpi3_driver_info_layout + its embedded use in struct mpi3mr_bsg_in_adpinfo). UAPI structs must not be removed without a proper deprecation cycle. The PR is not ready to merge until this ABI break is resolved.

@rahujosh
Copy link
Copy Markdown

PR #48 — validate-patch

PR: #48

Verdict Issues Detailed Report
0 Full report
Verdict: ❌ — click to expand

🔍 Patch Validation

PR: #48 — "Update scsi_bsg_mpi3mr.h"
Upstream commit: N/A — no upstream reference provided; network unavailable to verify independently
Verdict: ❌ FAIL


Commit Message

Check Status Note
Subject matches upstream "Update scsi_bsg_mpi3mr.h" is a non-descriptive filename-only subject; kernel convention requires <subsystem>: <imperative verb phrase>, e.g. scsi: mpi3mr: remove mpi3_driver_info_layout from UAPI header
Body preserves rationale Commit body is completely absent — no explanation of why the struct is removed, no reference to a firmware/spec change, no bug being fixed
Fixes tag present/correct No Fixes: tag; if this is a bug fix (e.g. ABI correction or dead-code removal) the tag is required; if it is a deliberate feature removal, the rationale must be in the body
Authorship preserved ⚠️ From: ayukum-hyd <ayukum@qti.qualcomm.com> — the From name is a machine/username alias (ayukum-hyd), not a real name as required by the DCO and kernel Signed-off-by policy (Documentation/process/submitting-patches.rst §14)
Backport note (if applicable) No Link: tag pointing to an upstream commit; no cherry-picked from note; impossible to verify this is a faithful backport without it

Diff

File Status Notes
include/uapi/scsi/scsi_bsg_mpi3mr.h Removes struct mpi3_driver_info_layout from a UAPI header — this is a userspace ABI-breaking change; once exported, UAPI structs cannot be deleted without a documented upstream justification and ack from the UAPI maintainer
include/uapi/scsi/scsi_bsg_mpi3mr.h Removes driver_info field from struct mpi3mr_bsg_in_adpinfo — changes the binary layout of a UAPI struct, breaking any userspace binary that uses BSG_MPI3MR_OPCODE_ADPINFO
include/uapi/scsi/scsi_bsg_mpi3mr.h ⚠️ Orphaned kdoc comment: the @driver_release_date / @driver_capabilities kernel-doc lines (context lines 16–18 in the patch) remain in the file after the struct they document is deleted, leaving dangling documentation

Issues

  1. UAPI ABI breakage (critical): include/uapi/scsi/scsi_bsg_mpi3mr.h is a userspace-visible header. Removing struct mpi3_driver_info_layout and the driver_info field from mpi3mr_bsg_in_adpinfo silently changes the size and layout of the BSG adapter-info response structure. Any compiled userspace tool (e.g. mpi3mrctl) that was built against the old header will silently misparse the response. This requires an explicit upstream justification, a check-uapi-headers pass, and maintainer sign-off.

  2. Missing commit rationale: There is zero explanation of why these symbols are removed. Is this a dead-code cleanup? A firmware ABI realignment? A security fix? Without a body, reviewers and bisectors cannot assess correctness or regression risk.

  3. Non-real author name: The From: / Signed-off-by: name ayukum-hyd is a username, not a legal name. The DCO (Developer Certificate of Origin) and kernel policy require a real name (first + last). This must be corrected before the patch can be accepted upstream.

  4. No upstream traceability: There is no Link: tag, no cherry picked from commit <sha>, and no reference to a mailing-list thread. It is impossible to determine whether this change has upstream acceptance or is a local-only modification.

  5. Orphaned kdoc: After the deletion, the kernel-doc comment block that documents @driver_release_date and @driver_capabilities is left dangling in the file with no struct to attach to. This will produce kernel-doc warnings and confuse documentation generators.


Verdict

Reject. The patch makes an undocumented, ABI-breaking deletion from a UAPI header with no commit rationale, no upstream traceability, and a non-conforming author name — all three must be corrected before this can be considered for merge.

@rahujosh
Copy link
Copy Markdown

PR #48 — checker-log-analyzer

PR: #48
Checker run: https://github.com/qualcomm-linux-stg/kernel-config-test/actions/runs/23713793043

Checker Result Summary
Checker Result Summary
checkpatch ⏭️ N/A No log present; not triggered for this run
dt-binding-check ⏭️ N/A No log present; not triggered for this run
dtb-check No DT changes detected; skipped cleanly
sparse-check ⏭️ N/A No log present; not triggered for this run
check-uapi-headers ABI-breaking deletion in struct mpi3mr_bsg_in_adpinfo
check-patch-compliance ⏭️ N/A No log present; not triggered for this run
tag-check ⏭️ N/A No log present; not triggered for this run
qcom-next-check ⏭️ N/A No log present; not triggered for this run

Detailed report: Full report

Checker analysis — click to expand

🤖 CI Checker Analysis (checker-log-analyzer)

PR: #48 — "Update scsi_bsg_mpi3mr.h" (ayukum-hyd)
Source: https://github.com/qualcomm-linux-stg/kernel-config-test/actions/runs/23713793043

Checker Result Summary
checkpatch ⏭️ N/A No log present; not triggered for this run
dt-binding-check ⏭️ N/A No log present; not triggered for this run
dtb-check No DT changes detected; skipped cleanly
sparse-check ⏭️ N/A No log present; not triggered for this run
check-uapi-headers ABI-breaking deletion in struct mpi3mr_bsg_in_adpinfo
check-patch-compliance ⏭️ N/A No log present; not triggered for this run
tag-check ⏭️ N/A No log present; not triggered for this run
qcom-next-check ⏭️ N/A No log present; not triggered for this run

❌ check-uapi-headers

Root cause: The patch deletes struct mpi3_driver_info_layout and removes the driver_info member from struct mpi3mr_bsg_in_adpinfo in the UAPI header include/uapi/scsi/scsi_bsg_mpi3mr.h, shrinking the struct from 1344 bits (168 bytes) to 384 bits (48 bytes) — a backwards-incompatible ABI change.

Failure details:

==== ABI differences detected in include/scsi/scsi_bsg_mpi3mr.h
     from 9448598b22c5 -> 1d53e7855680 ====
    [C] 'struct mpi3mr_bsg_in_adpinfo' changed:
      type size changed from 1344 to 384 (in bits)
      1 data member deletion:
        'mpi3_driver_info_layout driver_info', at offset 384 (in bits)
        at scsi_bsg_mpi3mr.h:146:1

error - 1/941 UAPI headers compatible with arm64 appear _not_ to be
        backwards compatible

What the patch does:

  • Deletes the entire struct mpi3_driver_info_layout definition (11 lines removed from include/uapi/scsi/scsi_bsg_mpi3mr.h)
  • Removes the struct mpi3_driver_info_layout driver_info member from struct mpi3mr_bsg_in_adpinfo

Why this is a blocker: UAPI headers are a stable kernel-to-userspace ABI contract. Removing a struct member shrinks the struct size and breaks any existing userspace application (e.g., mpi3mrctl) that was compiled against the old header and passes mpi3mr_bsg_in_adpinfo via ioctl. This is a hard ABI regression.

Fix options:

  1. Preferred — deprecate, don't delete: Mark mpi3_driver_info_layout and the driver_info member as deprecated using a comment or __attribute__((deprecated)), but keep them in the header. Userspace must not be broken.
  2. If the struct was never shipped in a released kernel: Confirm the field was never part of a stable release (check git log --follow on the header). If it was only ever in a staging/vendor tree, document that justification clearly in the commit message and request a checker override.
  3. If removal is intentional and justified: The commit message must explain why the ABI break is acceptable (e.g., the struct was never exposed to real userspace), and the PR author should request an explicit override: kernel-checker from an authorized reviewer (shashim-quic or sgaud-quic).

Reproduce locally:

./scripts/check-uapi.sh -b 1d53e785568005804ccff3500117c9758bae78d9 \
                        -p 9448598b22c50c8a5bb77a9103e2d49f134c9578 \
                        -l uapi_errors.log

Verdict

1 blocker: check-uapi-headers fails due to an ABI-breaking deletion of struct mpi3_driver_info_layout and its embedded member from the UAPI header — either restore the struct/member, or obtain an explicit checker override with justification before merging.

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